Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: Always use link target for /proc/self/exe #1168

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

s-zeid
Copy link
Contributor

@s-zeid s-zeid commented Jan 28, 2022

When running an AppImage under gcompat (e.g. on Alpine Linux), fopen("/proc/self/exe", "rb") opens the dynamic linker (e.g. /lib/ld-musl-x86_64.so.1) instead of the AppImage itself. However, readlink("/proc/self/exe", ...) does give the path to the AppImage as expected, even under gcompat.

This commit fixes this problem by always using the link target for /proc/self/exe in places that read the AppImage instead of the link itself. Without this commit, running an AppImage under gcompat results in the error message "This doesn't look like a squashfs image." from squashfuse. With this commit, AppImages run as expected under gcompat.

In order to make --appimage-help and --appimage-portable-{home,config} work under gcompat, I also move the calculation of the fullpath variable in main() to earlier in the function and change print_help() and portable_option() to use it instead of calculating the fullpath separately. (When $TARGET_APPIMAGE is set, since realpath() is (already) used instead of readlink() in that case, this change could result in a different path being used in help output and when creating the portable home and config directories with the respective command line options, but fullpath is already being used to find existing portable directories when running an AppImage, so this should not affect existing portable installations.) For consistency, I also rename the fullpath variable in main() and the corresponding arguments in print_help() and portable_option() to appimage_fullpath.

Fixes #1015 on Alpine Linux systems with gcompat installed, for AppImages made with this changeset applied.

Tested on Alpine Linux edge x86_64 and postmarketOS (based on Alpine) edge aarch64.

When running an AppImage under [gcompat][0] (e.g. on Alpine Linux),
`fopen("/proc/self/exe", "rb")` opens the dynamic linker (e.g.
`/lib/ld-musl-x86_64.so.1`) instead of the AppImage itself.  However,
`readlink("/proc/self/exe", ...)` does give the path to the AppImage as
expected, even under gcompat.

This commit fixes this problem by always using the link target for
`/proc/self/exe` in places that read the AppImage instead of the link
itself.  Without this commit, running an AppImage under gcompat results
in [the error message "This doesn't look like a squashfs image." from
squashfuse][1].  With this commit, AppImages run as expected under
gcompat.

In order to make `--appimage-help` and
`--appimage-portable-{home,config}` work under gcompat, I also move the
calculation of the `fullpath` variable in `main()` to earlier in the
function and change `print_help()` and `portable_option()` to use it
instead of calculating the fullpath separately.  (When
`$TARGET_APPIMAGE` is set, since `realpath()` is (already) used instead
of `readlink()` in that case, this change could result in a different
path being used in help output and when _creating_ the portable home and
config directories with the respective command line options, but
`fullpath` is already being used to find existing portable directories
when running an AppImage, so this should not affect existing portable
installations.)  For consistency, I also rename the `fullpath` variable
in `main()` and the corresponding arguments in `print_help()` and
`portable_option()` to `appimage_fullpath`.

Fixes AppImage#1015 on Alpine Linux systems with gcompat installed, for
AppImages made with this commit applied.

Tested on Alpine Linux edge x86_64 and postmarketOS (based on Alpine)
edge aarch64.

[0]: https://git.adelielinux.org/adelie/gcompat/
[1]: https://github.com/vasi/squashfuse/blob/e51978cd6bb5c4d16fae9eee43d0b258f570bb0f/util.c#L81-L82
@probonopd
Copy link
Member

Thanks for pointing this out. I wasn't aware of gcompat, a library which provides glibc-compatible APIs for use on musl libc systems.

Opening readlink("/proc/self/exe", ...) does sound reasonable to me. @TheAssassin wdyt?

@TheAssassin
Copy link
Member

You should open an issue with gcompat, that's clearly a bug there.

@probonopd
Copy link
Member

probonopd commented Jan 28, 2022

Agree, if gcompat is meant to be compatible with glibc, then it should behave like it.
This being said, is there a downside to doing the readlink/realpath on our end?

src/runtime.c Show resolved Hide resolved
src/runtime.c Outdated Show resolved Hide resolved
src/runtime.c Show resolved Hide resolved
src/runtime.c Show resolved Hide resolved
@TheAssassin
Copy link
Member

No. But someone should let them know about it.

@s-zeid
Copy link
Contributor Author

s-zeid commented Feb 1, 2022

I've opened https://git.adelielinux.org/adelie/gcompat/-/issues/349.

Copy link
Member

@TheAssassin TheAssassin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at a first glance. @probonopd should test a few AppImages built with the appimagetool from this PR, he's the bug finding specialist.

@probonopd
Copy link
Member

probonopd commented Feb 5, 2022

Still to be tested using the build from https://github.com/AppImage/AppImageKit/actions/runs/1780621637

@ell1e
Copy link

ell1e commented Jun 29, 2022

When running an AppImage under gcompat (e.g. on Alpine Linux), fopen("/proc/self/exe", "rb") opens the dynamic linker (e.g. /lib/ld-musl-x86_64.so.1) instead of the AppImage itself. However, readlink("/proc/self/exe", ...) does give the path to the AppImage as expected, even under gcompat.

This being said, is there a downside to doing the readlink/realpath on our end?

I think this causes race conditions when the binary is renamed between the reading of the link and the opening of the path that was read. fopen() directly doesn't seem to possibly cause since it uses some black magic to not actually read the path but somehow accesses the inode directly or such, at least if I recall correctly. (Basically renaming the program binary after it is launched in just the right moment will cause breakage with this merge request when it wouldn't have before.) I would therefore suggest this has a notable downside and shouldn't be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make AppImages run on Alpine
4 participants