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

fake_id0: do not re-translate the chroot path #55

Merged
merged 2 commits into from
Mar 18, 2019

Conversation

lightful
Copy link
Contributor

The call to chroot "/" from UID 0 may fail with "No such file or directory". This call is invoked e.g. by pacman during the post-install.

The culprit is this commit, which added extra calls to translate_path and realpath. These are unnecessary because the function handling chroot, which is invoked from SYSCALL_EXIT_END, already receives the path translated (during the enter syscall, translate_sysarg() ultimately invokes translate_path() and stores it via set_sysarg_path()). The mentioned commit even tries to translate the path returned by get_root(tracee). It was trying to solve some issue which in the end was not solved, as the latest posts in that thread acknowledge.

The duplicated translation try may accidentally be harmless but may cause errors depending on the bindings configuration. I think that this may have not been reported until now because many distros choose to bind paths in /data, usually within the guest root filesystem. When that is not the case (e.g. a "pure" Linux root directory emulation) the substitute_binding() step during the translation fails and ruins the chroot.

I revert here to the original implementation (which of course already supported relative paths).

@lightful
Copy link
Contributor Author

The simplest way to reproduce the issue is to run proot without any bindings:

(Moto G):~$ proot -i 0:0 -r /data/data/com.termux/files/proot/arch/ -w / /usr/bin/env -i /usr/bin/bash -l
[root@localhost /]# chroot /
chroot: cannot change root directory to '/': No such file or directory

@lightful
Copy link
Contributor Author

Well, I see that the original code was also not perfect, because was comparing the non-canonicalized chroot path. The original PR (I just have find it) tried to fix chroot "." errors. And this step is indeed necessary with relative paths:

(chroot "." from root directory):
e.g. get_root(tracee)  vs  non_canonicalized_path
[/data/data/com.termux/files/proot/arch] vs [/data/data/com.termux/files/proot/arch/.]

I have added a second commit. Now not only chroot "/" but also "." (from /) works!

The canonicalization could maybe be systematically performed at translate_path2() in enter.c, perhaps? You'll know better.

The function handling chroot is invoked from SYSCALL_EXIT_END,
and already receives the path translated (during the enter syscall,
translate_sysarg() ultimately invokes translate_path() and stores
it via set_sysarg_path()).

A duplicated translation was tried causing e.g. chroot "/" to fail
in systems not binding paths within the guest root filesystem (e.g.
/data not binded)
@michalbednarski michalbednarski merged commit 2a78bab into termux:master Mar 18, 2019
michalbednarski added a commit to michalbednarski/termux-packages that referenced this pull request Mar 18, 2019
@michalbednarski
Copy link
Collaborator

Thanks, looks nice

fornwall pushed a commit to termux/termux-packages that referenced this pull request Mar 19, 2019
fornwall pushed a commit to termux/termux-packages that referenced this pull request Mar 19, 2019
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.

2 participants