Replace realpath to BSD implementation

[yujincheng08]

Fix #5530

The thing is, ndk changes its realpath implementation to a new one that does not support kernel with version < 3.6. So, on some old devices or TWRP, readlink -f will fail. This replaces the realpath implementation to support older kernel versions.

@osm0sis Could you help me make this change to ndk-kitchen and co-author me? Thx.

[yujincheng08]

This issue is introduced by NDK, see: android/ndk#1260

And NDK seems not going to fix it. So sad.

[osm0sis]

Not a ton of time with a 1-month old here; basically limited to stuff I can do on my phone in between/during feedings. 🥲😴😅

[osm0sis]

Might be best to do this in the busybox source, similar to this: https://github.com/osm0sis/android-busybox-ndk/blob/master/patches/024-a-hush-platform.patch

It's how busybox handles broken/incomplete platforms.

[yujincheng08]

Might be best to do this in the busybox source, similar to this: https://github.com/osm0sis/android-busybox-ndk/blob/master/patches/024-a-hush-platform.patch

It's how busybox handles broken/incomplete platforms.

No. Your patch is for undefined symbols but mine is for replacing the bionic implementation. So doing something like your patch will raise duplicated symbols when linking.

[osm0sis]

platform.h/.c can be used to wrap and add various functions and calls, sometimes replacing ones from the libc used on a particular platform by defining a bb_ version instead.

Yours could be added in code as bb_realpath in platform.c, then defined as a replacement in platform.h, I think I've seen that done in busybox before. 🤷‍♂️

[yujincheng08]

https://github.com/mirror/busybox/blob/fc7868602ecf0d761a9a877141add4a9b6918d02/include/platform.h#L600
you mean this?

[osm0sis]

Sort of? I might be also thinking about how I took the bb_ functions for ttyname and used them in my nano builds by defining it as the replacement for the useless NDK stub version of it: https://forum.xda-developers.com/t/tools-zips-scripts-osm0sis-odds-and-ends-multiple-devices-platforms.2239421/page-34#post-66486572

But busybox platform.h/.c should allow for something similar I thought.

[yujincheng08]

Ok, I see now. Instead of -Wl,--wrap them, busybox prefers to use macro #define bb_x x to replace x function. Well, as long as it can cover all source code, it's okay. I use wrap because I am afraid some source files don't include platform.h and use the original function from libc.a.

[osm0sis]

Bingo! And nah, in busybox platform is in all, since it has all the platform hacks needed everywhere. 👍

[yujincheng08]

Then it's safe to use bb_ trick. But I just let it go since this PR is only for testing and demonstration because I don't want to mess up with the busybox kitchen. It's up to the one who actually makes this change to the busybox kitchen. Maybe you, or Jhon.

[osm0sis]

Sounds good. Can you let me know the first SDK version or NDK version where this issue started? It worked last with Magisk 24.1 I believe. I'll look into guarding it in a patch to platform.h with that, and upstream it in busybox if they'll accept it.

[yujincheng08]

ndk 21 and ndk 22 have no problem. ndk 23 introduces the problem.

[osm0sis]

Could this regression break anything else in Magisk? Should the used NDK perhaps be downgraded for now? I know @topjohnwu has done that in the past.

[yujincheng08]

Only busybox is affected.

[topjohnwu]

@osm0sis we shouldn't upstream this to BusyBox, as this is an issue with Bionic updating realpath to an implementation that requires higher Linux versions. Bionic does not really support static executables, so no considerations has been made when this Bionic change is pushed. If we build busybox as a dynamically linked executable (which 99.9% of people should, we build busybox statically for no reason other than supporting recovery installations), there would be no issues at all.

[osm0sis]

I mean, busybox is about supporting broken platforms in places so it's not entirely outside their usual, but 👍