Discussion:
bug#32796: please consider using renameat2 from glibc if available
(too old to reply)
Johannes Schauer
2018-09-21 06:44:09 UTC
Permalink
Hi,

tools like `mv` currently perform a direct renameat2 syscall. This is
problematic for software like fakechroot which allows one to perform a fake
chroot call without being root by intercepting and overriding libc functions
using LD_PRELOAD. It is currently not possible to perform a `mv` with recent
versions of coreutils inside a fakechroot environment because coreutils uses
the renameat2 syscall directly instead of using a libc function.

This problem could be solved by coreutils using the glibc renameat2 function in
glibc version 2.28 and newer. If glibc is older, coreutils could fall back to
performing a plain syscall instead to keep backwards compatibility.

Thanks!

cheers, josch
Paul Eggert
2018-09-21 23:42:37 UTC
Permalink
Post by Johannes Schauer
This problem could be solved by coreutils using the glibc renameat2 function in
glibc version 2.28 and newer.
Yes, that's on my list of things to do. It's not high priority, though, so if
you want it done faster you can write up and test the code and submit a patch
(hint, hint).
Andreas Henriksson
2018-10-06 18:40:56 UTC
Permalink
Hi,
Post by Paul Eggert
Post by Johannes Schauer
This problem could be solved by coreutils using the glibc renameat2 function in
glibc version 2.28 and newer.
Yes, that's on my list of things to do. It's not high priority, though, so
if you want it done faster you can write up and test the code and submit a
patch (hint, hint).
Probably going to embarras myself here with no prior coreutils (or
gnulib) experience and including an untested patch, but oh well.

I tried to look at this issue and AIUI this is basically a gnulib
issue rather than coreutils. There's even been some relevant changes
done since (the gnulib version included in) coreutils 8.30:
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=2522322e5304e7d86c63e607e2bc83c8d8b0a889

Since I have no idea how to bundle a new gnulib version with coreutils
to build a test version (and I don't even have glibc 2.28 yet either),
I haven't been able to test but wouldn't a simple patch like the
attached one do the trick?
(Note: stdio.h is already included so no addition for it needed. The
fallback discussed in [1] should be the same for HAVE_RENAMEAT2 and
SYS_renameat2 codepaths.)

[1] https://sourceware.org/ml/libc-alpha/2018-07/msg00064.html

I assume if it was this simple you'd have already delt with it, so
I'm probably way too naive here.

Regards,
Andreas Henriksson
Paul Eggert
2018-10-07 00:54:09 UTC
Permalink
wouldn't a simple patch like the attached one do the trick?
Most likely yes, but I'd like it tested before installing. This is because the
patch shouldn't affect behavior on any platform, and it's better to sanity-check
a clerical patch like this (by actually using it) rather than hoping that a typo
didn't inadvertently introduce a bug.
Johannes Schauer
2018-10-07 17:18:44 UTC
Permalink
My last mail seems to have been dropped. Maybe because of the attachment size?
I now gzipped the attachments.

Hi,

Quoting Paul Eggert (2018-10-07 02:54:09)
Post by Paul Eggert
wouldn't a simple patch like the attached one do the trick?
Most likely yes, but I'd like it tested before installing. This is because the
patch shouldn't affect behavior on any platform, and it's better to sanity-check
a clerical patch like this (by actually using it) rather than hoping that a typo
didn't inadvertently introduce a bug.
I just did a naive test on Debian unstable.

First, I built glibc because Debian still ships 2.27 but renameat2 is only in
2.28. Then, I applied Andreas' patch on coreutils 8.30. The only changes to
Andreas' patch I had to do were to rename the renameat2 function from coreutils
to renameatu as it's already done in the coreutils git. Otherwise, the code
from coreutils 8.30 would perform an infinite recursion. Then, I then rebuilt
fakechroot with the new glibc and a patch that adds support for glibc's
renameat2 function. Lastly, I created a Debian chroot with the new glibc,
coreutils and fakechroot and then, within a nested chroot tried to execute:

$ mv /etc/fstab /etc/blub

To prove that this now works I attached the strace output from before and after
I patched everything. The file fakechroot.old shows that renameat2 tries to
rename the real /etc/fstab for which it doesn't have permission. The file
fakechroot.new shows that the path to the /etc/fstab inside the chroot is
properly translated and thus does not anymore yield an EACCESS.

Which other tests should I perform?

Thanks!

cheers, josch
Johannes Schauer
2018-10-07 20:17:35 UTC
Permalink
Quoting Paul Eggert (2018-10-07 22:14:11)
Post by Johannes Schauer
Which other tests should I perform?
I'd like to see a test using glibc without a fake root, as that's the most
common way this code will be used.
Which test result would you like to see?

I have coreutils with that patch running on my system and in the chroot and mv
seems to be working fine as far as I can see.
Paul Eggert
2018-10-07 20:21:12 UTC
Permalink
Post by Johannes Schauer
Which test result would you like to see?
I have coreutils with that patch running on my system and in the chroot and mv
seems to be working fine as far as I can see.
The strace results you sent were of the old code and the new code, apparently
running in a chrooted environment. I'd like to see what happens if the test is
run in a non-chrooted environment; just the basic environment that people get
when they log into a traditional GNU/Linux setup.
Johannes Schauer
2018-10-07 20:30:31 UTC
Permalink
Quoting Paul Eggert (2018-10-07 22:21:12)
Post by Paul Eggert
The strace results you sent were of the old code and the new code, apparently
running in a chrooted environment. I'd like to see what happens if the test
is run in a non-chrooted environment; just the basic environment that people
get when they log into a traditional GNU/Linux setup.
Attached a gzipped log file.

If you want to play with a chroot that contains the patched coreutils as well
as glibc 2.28, then you can find a 20 MB tarball here for the next 24 hours:

https://a.uguu.se/MAHWcJ0uVW4w_debian-sid.tar.xz

Thanks!

cheers, josch
Paul Eggert
2018-10-08 06:27:34 UTC
Permalink
Thanks for checking. I installed the attached into Gnulib master. The "(tiny
change)" is because the patch is small enough that we don't need to worry about
copyright papers.
Assaf Gordon
2018-10-30 04:01:17 UTC
Permalink
tags 32796 fixed
close 32796
stop
Post by Paul Eggert
Thanks for checking. I installed the attached into Gnulib master. The
"(tiny change)" is because the patch is small enough that we don't need
to worry about copyright papers.
Pushed here:
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c50cf67bd7ff70525f3cb4074f0d9cc1f5c6cf9c

Closing as "fixed".

-assaf

Johannes Schauer
2018-10-07 15:43:00 UTC
Permalink
Hi,

Quoting Paul Eggert (2018-10-07 02:54:09)
Post by Paul Eggert
wouldn't a simple patch like the attached one do the trick?
Most likely yes, but I'd like it tested before installing. This is because the
patch shouldn't affect behavior on any platform, and it's better to sanity-check
a clerical patch like this (by actually using it) rather than hoping that a typo
didn't inadvertently introduce a bug.
I just did a naive test on Debian unstable.

First, I built glibc because Debian still ships 2.27 but renameat2 is only in
2.28. Then, I applied Andreas' patch on coreutils 8.30. The only changes to
Andreas' patch I had to do were to rename the renameat2 function from coreutils
to renameatu as it's already done in the coreutils git. Otherwise, the code
from coreutils 8.30 would perform an infinite recursion. Then, I then rebuilt
fakechroot with the new glibc and a patch that adds support for glibc's
renameat2 function. Lastly, I created a Debian chroot with the new glibc,
coreutils and fakechroot and then, within a nested chroot tried to execute:

$ mv /etc/fstab /etc/blub

To prove that this now works I attached the strace output from before and after
I patched everything. The file fakechroot.old shows that renameat2 tries to
rename the real /etc/fstab for which it doesn't have permission. The file
fakechroot.new shows that the path to the /etc/fstab inside the chroot is
properly translated and thus does not anymore yield an EACCESS.

Which other tests should I perform?

Thanks!

cheers, josch
Loading...