Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Bug#915559: coreutils: Use renameat2 from glibc instead of syscall
70 views
Skip to first unread message
Johannes 'josch' Schauer
Dec 4, 2018, 3:20:02 PM12/4/18
to
Package: coreutils
Version: 8.30-1
Severity: normal
Tags: patch
Control: block #909612 by -1
Hi,
recently (2018-11-29), glibc 2.28 was accepted in unstable. It adds a
wrapper for the renameat2 syscall. That wrapper is necessary for
fakechroot because fakechroot cannot intercept system calls but uses a
preloaded library to intercept library calls.
Up to coreutils 8.30, mv(1) uses the renameat2 syscall which makes mv(1)
fail under fakechroot. See #909612.
Now that glibc 2.28 offers the renameat2 library function as a wrapper
to the syscall, coreutils added support for choosing the library
function instead of the syscall if the former is available:
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c50cf67bd7ff70525f3cb4074f0d9cc1f5c6cf9c
I don't know if another coreutils release is likely to happen before the
freeze but if it isn't please consider the attached patch which
backports the commit from the gnulib git to coreutils 8.30.
Without this patch, fakechroot is currently not very useful because the
mv(1) tool is unusable inside fakechroot. Most prominently, apt uses
mv(1) inside its postinst script, so its impossible to install apt
inside fakechroot and thus one cannot setup a sensible system.
Thanks!
cheers, josch
Version: 8.30-1
Severity: normal
Tags: patch
Control: block #909612 by -1
Hi,
recently (2018-11-29), glibc 2.28 was accepted in unstable. It adds a
wrapper for the renameat2 syscall. That wrapper is necessary for
fakechroot because fakechroot cannot intercept system calls but uses a
preloaded library to intercept library calls.
Up to coreutils 8.30, mv(1) uses the renameat2 syscall which makes mv(1)
fail under fakechroot. See #909612.
Now that glibc 2.28 offers the renameat2 library function as a wrapper
to the syscall, coreutils added support for choosing the library
function instead of the syscall if the former is available:
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c50cf67bd7ff70525f3cb4074f0d9cc1f5c6cf9c
I don't know if another coreutils release is likely to happen before the
freeze but if it isn't please consider the attached patch which
backports the commit from the gnulib git to coreutils 8.30.
Without this patch, fakechroot is currently not very useful because the
mv(1) tool is unusable inside fakechroot. Most prominently, apt uses
mv(1) inside its postinst script, so its impossible to install apt
inside fakechroot and thus one cannot setup a sensible system.
Thanks!
cheers, josch
Johannes Schauer
Dec 23, 2018, 1:00:03 PM12/23/18
to
Hi,
what is the status of this bug? Without this patch, the functionality of
fakechroot and mmdebstrap in the next stable release will be hampered. If you
don't have time, I could also NMU coreutils with the attached patch.
What do you think?
Thanks!
cheers, josch
fakechroot and mmdebstrap in the next stable release will be hampered. If you
don't have time, I could also NMU coreutils with the attached patch.
What do you think?
Thanks!
cheers, josch
Michael Stone
Dec 23, 2018, 1:10:02 PM12/23/18
to
--
Michael Stone
(From phone, please excuse typos)
Johannes Schauer
Dec 28, 2018, 3:30:03 AM12/28/18
to
On Sun, 23 Dec 2018 12:56:55 -0500 Michael Stone <mst...@mathom.us> wrote:
> Please just wait
I take that means that you intend to apply the fix I proposed?
The problem now became more dire for fakechroot because mv(1) is now used in
the maintainer script of dash (during the diversion process). So as of a few
days ago it is now impossible to use debootstrap with fakechroot.
Thanks!
cheers, josch
> Please just wait
I take that means that you intend to apply the fix I proposed?
The problem now became more dire for fakechroot because mv(1) is now used in
the maintainer script of dash (during the diversion process). So as of a few
days ago it is now impossible to use debootstrap with fakechroot.
Thanks!
cheers, josch
Johannes Schauer
Dec 31, 2018, 5:10:02 AM12/31/18
to
Hi,
I'm sorry, but my attached patch was apparently not properly tested by me. :(
More work is needed to backport the patch from upstream to version 8.30 because
just calling renameat2 as proposed in the patch I attached in my first message
will just result in an infinite recursion loop. I guess I only didn't notice
because the initial patch never worked because HAVE_RENAMEAT2 never got defined
as the following hunk was missing from it:
--- coreutils-8.30.orig/lib/config.hin
+++ coreutils-8.30/lib/config.hin
@@ -2069,6 +2069,9 @@
/* Define to 1 if you have the `renameat' function. */
#undef HAVE_RENAMEAT
+/* Define to 1 if you have the `renameat2' function. */
+#undef HAVE_RENAMEAT2
+
/* Define to 1 if you have the `rewinddir' function. */
#undef HAVE_REWINDDIR
I only now started to understand how the Debian package includes the gnulib
submodule and ends up shipping a number of autogenerated files as well.
Upstream doesn't have the problem of infinite recursion because they renamed
renameat2 to renameatu. So one way of solving this would be to also backport
that change.
Would you be okay with that or would you like to see another solution?
How much time do I have to present a working patch?
Thanks!
cheers, josch
More work is needed to backport the patch from upstream to version 8.30 because
just calling renameat2 as proposed in the patch I attached in my first message
will just result in an infinite recursion loop. I guess I only didn't notice
because the initial patch never worked because HAVE_RENAMEAT2 never got defined
as the following hunk was missing from it:
--- coreutils-8.30.orig/lib/config.hin
+++ coreutils-8.30/lib/config.hin
@@ -2069,6 +2069,9 @@
/* Define to 1 if you have the `renameat' function. */
#undef HAVE_RENAMEAT
+/* Define to 1 if you have the `renameat2' function. */
+#undef HAVE_RENAMEAT2
+
/* Define to 1 if you have the `rewinddir' function. */
#undef HAVE_REWINDDIR
I only now started to understand how the Debian package includes the gnulib
submodule and ends up shipping a number of autogenerated files as well.
Upstream doesn't have the problem of infinite recursion because they renamed
renameat2 to renameatu. So one way of solving this would be to also backport
that change.
Would you be okay with that or would you like to see another solution?
How much time do I have to present a working patch?
Thanks!
cheers, josch
Johannes Schauer
Dec 31, 2018, 10:50:03 AM12/31/18
to
Hello again,
Quoting Johannes Schauer (2018-12-31 10:58:18)
the renameat2 function to renameatu (as it was done upstream) and the other
adds support for using glibc renameat2 instead of the systemcall (and also
includes the missing hunk from lib/config.hin).
The renameatu patch is very small because the Debian package does not seem to
run the testsuite? At least it compiles fine and seems to work fine together
with my patch to fakechroot in #909612.
Sorry to not have gotten it right when I reported the bug.
Is there anything else you'd like me to do for this bug?
Thanks!
cheers, josch
Quoting Johannes Schauer (2018-12-31 10:58:18)
> I'm sorry, but my attached patch was apparently not properly tested by me. :(
>
> More work is needed to backport the patch from upstream to version 8.30 because
> just calling renameat2 as proposed in the patch I attached in my first message
> will just result in an infinite recursion loop. I guess I only didn't notice
> because the initial patch never worked because HAVE_RENAMEAT2 never got defined
> as the following hunk was missing from it:
>
> --- coreutils-8.30.orig/lib/config.hin
> +++ coreutils-8.30/lib/config.hin
> @@ -2069,6 +2069,9 @@
> /* Define to 1 if you have the `renameat' function. */
> #undef HAVE_RENAMEAT
>
> +/* Define to 1 if you have the `renameat2' function. */
> +#undef HAVE_RENAMEAT2
> +
> /* Define to 1 if you have the `rewinddir' function. */
> #undef HAVE_REWINDDIR
>
>
> I only now started to understand how the Debian package includes the gnulib
> submodule and ends up shipping a number of autogenerated files as well.
>
> Upstream doesn't have the problem of infinite recursion because they renamed
> renameat2 to renameatu. So one way of solving this would be to also backport
> that change.
>
> Would you be okay with that or would you like to see another solution?
>
> How much time do I have to present a working patch?
I think I've got it this time. Please find attached two patches. One renames
>
> More work is needed to backport the patch from upstream to version 8.30 because
> just calling renameat2 as proposed in the patch I attached in my first message
> will just result in an infinite recursion loop. I guess I only didn't notice
> because the initial patch never worked because HAVE_RENAMEAT2 never got defined
> as the following hunk was missing from it:
>
> --- coreutils-8.30.orig/lib/config.hin
> +++ coreutils-8.30/lib/config.hin
> @@ -2069,6 +2069,9 @@
> /* Define to 1 if you have the `renameat' function. */
> #undef HAVE_RENAMEAT
>
> +/* Define to 1 if you have the `renameat2' function. */
> +#undef HAVE_RENAMEAT2
> +
> /* Define to 1 if you have the `rewinddir' function. */
> #undef HAVE_REWINDDIR
>
>
> I only now started to understand how the Debian package includes the gnulib
> submodule and ends up shipping a number of autogenerated files as well.
>
> Upstream doesn't have the problem of infinite recursion because they renamed
> renameat2 to renameatu. So one way of solving this would be to also backport
> that change.
>
> Would you be okay with that or would you like to see another solution?
>
> How much time do I have to present a working patch?
the renameat2 function to renameatu (as it was done upstream) and the other
adds support for using glibc renameat2 instead of the systemcall (and also
includes the missing hunk from lib/config.hin).
The renameatu patch is very small because the Debian package does not seem to
run the testsuite? At least it compiles fine and seems to work fine together
with my patch to fakechroot in #909612.
Sorry to not have gotten it right when I reported the bug.
Is there anything else you'd like me to do for this bug?
Thanks!
cheers, josch
Jonas Smedegaard
Jan 30, 2019, 6:10:03 AM1/30/19
to
Dear Michael
Jus a friendly nudge: It would be great if this bug was fixed in time
for Buster.
Do you think you can find the time to have a look at the patches
provided by Josch?
Do you need some help?
Kind regards,
- Jonas
--
* Jonas Smedegaard - idealist & Internet-arkitekt
* Tlf.: +45 40843136 Website: http://dr.jones.dk/
[x] quote me freely [ ] ask before reusing [ ] keep private
Jus a friendly nudge: It would be great if this bug was fixed in time
for Buster.
Do you think you can find the time to have a look at the patches
provided by Josch?
Do you need some help?
Kind regards,
- Jonas
--
* Jonas Smedegaard - idealist & Internet-arkitekt
* Tlf.: +45 40843136 Website: http://dr.jones.dk/
[x] quote me freely [ ] ask before reusing [ ] keep private
Johannes Schauer
Feb 6, 2019, 5:00:03 AM2/6/19
to
Hi Michael,
On Wed, 30 Jan 2019 12:07:37 +0100 Jonas Smedegaard <d...@jones.dk> wrote:
> Jus a friendly nudge: It would be great if this bug was fixed in time for
> Buster.
>
> Do you think you can find the time to have a look at the patches
> provided by Josch?
>
> Do you need some help?
another week passed since Jonas' email. And more than seven weeks since your
last activity in this bug. Unfortunately, fakechroot is not really usable
without mv(1). The mv(1) tool is used in apt and dash postinst maintainer
scripts, so this also affects debootstrap and mmdebstrap and thus in turn also
packages using these tools.
I thus hereby announce my intention to NMU coreutils with my two patches in one
week (February 13) and a delay of 10 days (then in unstable by February 23).
Please speak up if you are not okay with that plan.
Thanks!
cheers, josch
On Wed, 30 Jan 2019 12:07:37 +0100 Jonas Smedegaard <d...@jones.dk> wrote:
> Jus a friendly nudge: It would be great if this bug was fixed in time for
> Buster.
>
> Do you think you can find the time to have a look at the patches
> provided by Josch?
>
> Do you need some help?
last activity in this bug. Unfortunately, fakechroot is not really usable
without mv(1). The mv(1) tool is used in apt and dash postinst maintainer
scripts, so this also affects debootstrap and mmdebstrap and thus in turn also
packages using these tools.
I thus hereby announce my intention to NMU coreutils with my two patches in one
week (February 13) and a delay of 10 days (then in unstable by February 23).
Please speak up if you are not okay with that plan.
Thanks!
cheers, josch
Michael Stone
Feb 6, 2019, 8:40:02 AM2/6/19
to
On Wed, Feb 06, 2019 at 10:47:12AM +0100, Johannes Schauer wrote:
Johannes Schauer
Feb 14, 2019, 4:10:03 AM2/14/19
to
Control: severity -1 critical
Hi Michael,
Again, more than a week passed since my last message. You manage to reply
within hours when it's about dismissing my offers to fix this bug in coreutils
but when it is about explaining your reasoning you stay silent for weeks. Maybe
my patch has a fatal flaw in it, maybe you want us to work together
differently, maybe you want to fix the problem yourself (as indicated by your
message "please just wait") or maybe I was missing something essential which
makes my patch completely unacceptable. But so far you only left 9 words of
text in this bug report offering no explanation at all. This is honestly very
frustrating for me. :(
I'm now raising the severity of this bug to "critical" because this bug "makes
unrelated software on the system break" -- in this case it directly breaks
fakechroot and it indirectly breaks the software using fakechroot.
In case you do not agree with this severity level, please explain why you think
that this bug is not RC even though it leads to another package becoming nearly
unusable.
Thanks!
cheers, josch
Hi Michael,
within hours when it's about dismissing my offers to fix this bug in coreutils
but when it is about explaining your reasoning you stay silent for weeks. Maybe
my patch has a fatal flaw in it, maybe you want us to work together
differently, maybe you want to fix the problem yourself (as indicated by your
message "please just wait") or maybe I was missing something essential which
makes my patch completely unacceptable. But so far you only left 9 words of
text in this bug report offering no explanation at all. This is honestly very
frustrating for me. :(
I'm now raising the severity of this bug to "critical" because this bug "makes
unrelated software on the system break" -- in this case it directly breaks
fakechroot and it indirectly breaks the software using fakechroot.
In case you do not agree with this severity level, please explain why you think
that this bug is not RC even though it leads to another package becoming nearly
unusable.
Thanks!
cheers, josch
Per Sandberg
Feb 24, 2019, 2:10:03 PM2/24/19
to
Dear maintainer This bug is probably very the root-cause to the following bug chain: ThisBug#915559: coreutils: Use renameat2 from glibc instead of syscall Leading to: Bug#921924: fakechroot: mv(1) cannot be used inside fakechroot Leading to: Bug#922572: debootstrap variant fakeroot fails due to dash diversion. And maybe a heap load of others so is it possible to get it fixed. /Regards /P
Pádraig Brady
Feb 28, 2019, 2:10:02 AM2/28/19
to
On 27/02/19 14:20, Derek Dongray wrote:
> When trying to use mv to rename a file on an external drive using coreutils
> 8.30-2 on a debian system (Linux version 4.19.0-3-amd64), the rename fails
> with the error message:
>
> mv: cannot move 'file1' to a subdirectory of itself 'file2'
>
> Downgrading to coreutils 8.30-1, the rename executes as expected.
>
> The following is the result of running a test script. The folder '/backup'
> is an external drive using the ZFS fileystems (zfs-zed 0.7.12-3), but I
> have seen a report on superuser.com (
> https://superuser.com/questions/1409618/renaming-a-file-with-mv-cannot-move-to-a-subdirectory-of-itself)
> that this also happens with NTFS external drives.
>
> root@capella:~# ./mv-bug
> + apt install -y --allow-downgrades coreutils=8.30-1
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> coreutils is already the newest version (8.30-1).
> 0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
> + cd /backup
> + touch t
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + mv t s
> + ls s t
> ls: cannot access 't': No such file or directory
> s
> + rm s t
> rm: cannot remove 't': No such file or directory
> + cd /root
> + apt install -y coreutils=8.30-2
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> The following packages will be upgraded:
> coreutils
> 1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
> Need to get 2,707 kB of archives.
> After this operation, 4,096 B disk space will be freed.
> Get:1 http://ftp.uk.debian.org/debian sid/main amd64 coreutils amd64 8.30-2
> [2,707 kB]
> Fetched 2,707 kB in 1s (2,729 kB/s)
> apt-listchanges: Reading changelogs...
> (Reading database ... 226704 files and directories currently installed.)
> Preparing to unpack .../coreutils_8.30-2_amd64.deb ...
> Unpacking coreutils (8.30-2) over (8.30-1) ...
> Setting up coreutils (8.30-2) ...
> Processing triggers for install-info (6.5.0.dfsg.1-4+b1) ...
> Processing triggers for man-db (2.8.5-2) ...
> + cd /backup
> + touch t
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + mv t s
> mv: cannot move 't' to a subdirectory of itself, 's'
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + rm s t
> rm: cannot remove 's': No such file or directory
> root@capella:~#
That sounds like unsupported renameat2()
is not falling back to rename()
The only change in debian is:
coreutils (8.30-2) unstable; urgency=medium
* Use renameat glibc function that can be intercepted by fakechroot
(Closes: https://bugs.debian.org/915559 )
* Above requires autoreconf turned on again
A quick scan of the patches shows that the name was changed
to renameatu() in gnulib, but copy.c still calls renameat2()
and thus now doesn't have the fallback.
To be clear this seems to only be an issue in the debian patch.
cheers,
Pádraig
> When trying to use mv to rename a file on an external drive using coreutils
> 8.30-2 on a debian system (Linux version 4.19.0-3-amd64), the rename fails
> with the error message:
>
> mv: cannot move 'file1' to a subdirectory of itself 'file2'
>
> Downgrading to coreutils 8.30-1, the rename executes as expected.
>
> The following is the result of running a test script. The folder '/backup'
> is an external drive using the ZFS fileystems (zfs-zed 0.7.12-3), but I
> have seen a report on superuser.com (
> https://superuser.com/questions/1409618/renaming-a-file-with-mv-cannot-move-to-a-subdirectory-of-itself)
> that this also happens with NTFS external drives.
>
> root@capella:~# ./mv-bug
> + apt install -y --allow-downgrades coreutils=8.30-1
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> coreutils is already the newest version (8.30-1).
> 0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
> + cd /backup
> + touch t
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + mv t s
> + ls s t
> ls: cannot access 't': No such file or directory
> s
> + rm s t
> rm: cannot remove 't': No such file or directory
> + cd /root
> + apt install -y coreutils=8.30-2
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> The following packages will be upgraded:
> coreutils
> 1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
> Need to get 2,707 kB of archives.
> After this operation, 4,096 B disk space will be freed.
> Get:1 http://ftp.uk.debian.org/debian sid/main amd64 coreutils amd64 8.30-2
> [2,707 kB]
> Fetched 2,707 kB in 1s (2,729 kB/s)
> apt-listchanges: Reading changelogs...
> (Reading database ... 226704 files and directories currently installed.)
> Preparing to unpack .../coreutils_8.30-2_amd64.deb ...
> Unpacking coreutils (8.30-2) over (8.30-1) ...
> Setting up coreutils (8.30-2) ...
> Processing triggers for install-info (6.5.0.dfsg.1-4+b1) ...
> Processing triggers for man-db (2.8.5-2) ...
> + cd /backup
> + touch t
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + mv t s
> mv: cannot move 't' to a subdirectory of itself, 's'
> + ls s t
> ls: cannot access 's': No such file or directory
> t
> + rm s t
> rm: cannot remove 's': No such file or directory
> root@capella:~#
That sounds like unsupported renameat2()
is not falling back to rename()
The only change in debian is:
coreutils (8.30-2) unstable; urgency=medium
* Use renameat glibc function that can be intercepted by fakechroot
(Closes: https://bugs.debian.org/915559 )
* Above requires autoreconf turned on again
A quick scan of the patches shows that the name was changed
to renameatu() in gnulib, but copy.c still calls renameat2()
and thus now doesn't have the fallback.
To be clear this seems to only be an issue in the debian patch.
cheers,
Pádraig
0 new messages