GNU bug report logs - #39236
[musl] coreutils cp mishandles error return from lchmod

Previous Next

To reply to this bug, email your comments to 39236 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, bug-coreutils <at> gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 15:34:18 +0100

* Rich Felker:

> coreutils should be opting to use the system-provided lchmod, which is
> safe, and correctly handling error returns (silently treating
> EOPNOTSUPP as success) rather than as hard errors.

glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
lchmod is used in coreutils, but I suspect it is not particularly
useful.

Thanks,
Florian

Message #8 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 16:08:26 +0100

* Rich Felker:

> On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
>> * Rich Felker:
>> 
>> > coreutils should be opting to use the system-provided lchmod, which is
>> > safe, and correctly handling error returns (silently treating
>> > EOPNOTSUPP as success) rather than as hard errors.
>> 
>> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
>> lchmod is used in coreutils, but I suspect it is not particularly
>> useful.
>
> When preserving permissions (cp -p, archive extraction, etc.), you
> want lchmod to work correctly just for the purpose of *not* following
> the link and thereby unwantedly changing the permissions of the link
> target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
> is standard, and that's really what coreutils should be using.

I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
if the file is not a symbolic link.  Likewise, fchmodat with
AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.

The reason for this is that the kernel does not provide a suitable
system call to implement this, even though some file systems allow a
mode change for symbolic links.  I think we can do better, although I
should note that each time we implement such emulation in userspace, it
comes back to bite us eventually.

Thanks,
Florian

Message #11 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 16:32:45 +0100

* Rich Felker:

> On Wed, Jan 22, 2020 at 04:08:26PM +0100, Florian Weimer wrote:
>> * Rich Felker:
>> 
>> > On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
>> >> * Rich Felker:
>> >> 
>> >> > coreutils should be opting to use the system-provided lchmod, which is
>> >> > safe, and correctly handling error returns (silently treating
>> >> > EOPNOTSUPP as success) rather than as hard errors.
>> >> 
>> >> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
>> >> lchmod is used in coreutils, but I suspect it is not particularly
>> >> useful.
>> >
>> > When preserving permissions (cp -p, archive extraction, etc.), you
>> > want lchmod to work correctly just for the purpose of *not* following
>> > the link and thereby unwantedly changing the permissions of the link
>> > target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
>> > is standard, and that's really what coreutils should be using.
>> 
>> I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
>> if the file is not a symbolic link.  Likewise, fchmodat with
>> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.
>
> Yes, I understood that. I was going into why there should be a real
> implementation, but didn't make it clear that that was what I was
> doing.

Ah, yes, there should be a real implementation if we can get full
lchmod/AT_SYMLINK_NOFOLLOW behavior on file systems that support it.  If
we can't, I'm not sure if there is a point to it.

>> The reason for this is that the kernel does not provide a suitable
>> system call to implement this, even though some file systems allow a
>> mode change for symbolic links.  I think we can do better, although I
>> should note that each time we implement such emulation in userspace, it
>> comes back to bite us eventually.
>
> Emulations in userspace that are approximate, have race conditions,
> etc. are bad. Ones that are rigorous are good, though.

Is there a reason for the S_ISLNK check in the musl implementation?
With current kernels, chmod on the proc pseudo-file will not traverse
the symbolic link, but I have yet to check if this has always been the
case.

Thanks,
Florian

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fweimer <at> redhat.com>
Cc: musl <at> lists.openwall.com, bug-coreutils <at> gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 09:42:43 -0500

On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> > coreutils should be opting to use the system-provided lchmod, which is
> > safe, and correctly handling error returns (silently treating
> > EOPNOTSUPP as success) rather than as hard errors.
> 
> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
> lchmod is used in coreutils, but I suspect it is not particularly
> useful.

When preserving permissions (cp -p, archive extraction, etc.), you
want lchmod to work correctly just for the purpose of *not* following
the link and thereby unwantedly changing the permissions of the link
target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
is standard, and that's really what coreutils should be using.

Rich

Message #17 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fweimer <at> redhat.com>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 10:15:07 -0500

On Wed, Jan 22, 2020 at 04:08:26PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> > On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
> >> * Rich Felker:
> >> 
> >> > coreutils should be opting to use the system-provided lchmod, which is
> >> > safe, and correctly handling error returns (silently treating
> >> > EOPNOTSUPP as success) rather than as hard errors.
> >> 
> >> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
> >> lchmod is used in coreutils, but I suspect it is not particularly
> >> useful.
> >
> > When preserving permissions (cp -p, archive extraction, etc.), you
> > want lchmod to work correctly just for the purpose of *not* following
> > the link and thereby unwantedly changing the permissions of the link
> > target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
> > is standard, and that's really what coreutils should be using.
> 
> I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
> if the file is not a symbolic link.  Likewise, fchmodat with
> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.

Yes, I understood that. I was going into why there should be a real
implementation, but didn't make it clear that that was what I was
doing.

> The reason for this is that the kernel does not provide a suitable
> system call to implement this, even though some file systems allow a
> mode change for symbolic links.  I think we can do better, although I
> should note that each time we implement such emulation in userspace, it
> comes back to bite us eventually.

Emulations in userspace that are approximate, have race conditions,
etc. are bad. Ones that are rigorous are good, though.

Rich

Message #20 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fweimer <at> redhat.com>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 11:07:43 -0500

On Wed, Jan 22, 2020 at 04:32:45PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> > On Wed, Jan 22, 2020 at 04:08:26PM +0100, Florian Weimer wrote:
> >> * Rich Felker:
> >> 
> >> > On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
> >> >> * Rich Felker:
> >> >> 
> >> >> > coreutils should be opting to use the system-provided lchmod, which is
> >> >> > safe, and correctly handling error returns (silently treating
> >> >> > EOPNOTSUPP as success) rather than as hard errors.
> >> >> 
> >> >> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
> >> >> lchmod is used in coreutils, but I suspect it is not particularly
> >> >> useful.
> >> >
> >> > When preserving permissions (cp -p, archive extraction, etc.), you
> >> > want lchmod to work correctly just for the purpose of *not* following
> >> > the link and thereby unwantedly changing the permissions of the link
> >> > target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
> >> > is standard, and that's really what coreutils should be using.
> >> 
> >> I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
> >> if the file is not a symbolic link.  Likewise, fchmodat with
> >> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.
> >
> > Yes, I understood that. I was going into why there should be a real
> > implementation, but didn't make it clear that that was what I was
> > doing.
> 
> Ah, yes, there should be a real implementation if we can get full
> lchmod/AT_SYMLINK_NOFOLLOW behavior on file systems that support it.  If
> we can't, I'm not sure if there is a point to it.

The point is to fail when the target is a symlink, rather than
(erroneously and possibly dangerously) applying the chmod to the link
target. Actually supporting link modes is useless. It's the "not
modifying the target" that's important.

> >> The reason for this is that the kernel does not provide a suitable
> >> system call to implement this, even though some file systems allow a
> >> mode change for symbolic links.  I think we can do better, although I
> >> should note that each time we implement such emulation in userspace, it
> >> comes back to bite us eventually.
> >
> > Emulations in userspace that are approximate, have race conditions,
> > etc. are bad. Ones that are rigorous are good, though.
> 
> Is there a reason for the S_ISLNK check in the musl implementation?
> With current kernels, chmod on the proc pseudo-file will not traverse
> the symbolic link, but I have yet to check if this has always been the
> case.

It's explained in the bz you just replied on,
https://sourceware.org/bugzilla/show_bug.cgi?id=14578

The point of the S_ISLNK check is to fail out early with the ENOTSUPP,
which the caller should treat as "success-like", in the non-racing
condition, without the need to open a fd (which may fail with
ENFILE/EMFILE) and without the need for /proc to be mounted.
Otherwise, a different error will be produced when one of those cases
is hit, and the caller will treat it as a real error.

Rich

Message #23 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 17:19:05 +0100

* Rich Felker:

> On Wed, Jan 22, 2020 at 04:32:45PM +0100, Florian Weimer wrote:
>> * Rich Felker:
>> 
>> > On Wed, Jan 22, 2020 at 04:08:26PM +0100, Florian Weimer wrote:
>> >> * Rich Felker:
>> >> 
>> >> > On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
>> >> >> * Rich Felker:
>> >> >> 
>> >> >> > coreutils should be opting to use the system-provided lchmod, which is
>> >> >> > safe, and correctly handling error returns (silently treating
>> >> >> > EOPNOTSUPP as success) rather than as hard errors.
>> >> >> 
>> >> >> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
>> >> >> lchmod is used in coreutils, but I suspect it is not particularly
>> >> >> useful.
>> >> >
>> >> > When preserving permissions (cp -p, archive extraction, etc.), you
>> >> > want lchmod to work correctly just for the purpose of *not* following
>> >> > the link and thereby unwantedly changing the permissions of the link
>> >> > target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
>> >> > is standard, and that's really what coreutils should be using.
>> >> 
>> >> I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
>> >> if the file is not a symbolic link.  Likewise, fchmodat with
>> >> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.
>> >
>> > Yes, I understood that. I was going into why there should be a real
>> > implementation, but didn't make it clear that that was what I was
>> > doing.
>> 
>> Ah, yes, there should be a real implementation if we can get full
>> lchmod/AT_SYMLINK_NOFOLLOW behavior on file systems that support it.  If
>> we can't, I'm not sure if there is a point to it.
>
> The point is to fail when the target is a symlink, rather than
> (erroneously and possibly dangerously) applying the chmod to the link
> target. Actually supporting link modes is useless. It's the "not
> modifying the target" that's important.

The kernel supports it on some file systems, though:

$ ls -l /tmp/x
l---------. 1 fweimer fweimer 6 Jan 22 15:27 /tmp/x -> /tmp/x

Although mode 0 curiously does not prevent readlink calls.

> It's explained in the bz you just replied on,
> https://sourceware.org/bugzilla/show_bug.cgi?id=14578
>
> The point of the S_ISLNK check is to fail out early with the ENOTSUPP,
> which the caller should treat as "success-like", in the non-racing
> condition, without the need to open a fd (which may fail with
> ENFILE/EMFILE) and without the need for /proc to be mounted.
> Otherwise, a different error will be produced when one of those cases
> is hit, and the caller will treat it as a real error.

Hmm.  The way I read the musl code, the O_PATH descriptor already
exists.  At this point, you can just chmod the O_PATH descriptor, and
have the kernel report EOPNOTSUPP if the file system does not support
that.

Thanks,
Florian

Message #26 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fweimer <at> redhat.com>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 12:15:08 -0500

On Wed, Jan 22, 2020 at 05:19:05PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> > On Wed, Jan 22, 2020 at 04:32:45PM +0100, Florian Weimer wrote:
> >> * Rich Felker:
> >> 
> >> > On Wed, Jan 22, 2020 at 04:08:26PM +0100, Florian Weimer wrote:
> >> >> * Rich Felker:
> >> >> 
> >> >> > On Wed, Jan 22, 2020 at 03:34:18PM +0100, Florian Weimer wrote:
> >> >> >> * Rich Felker:
> >> >> >> 
> >> >> >> > coreutils should be opting to use the system-provided lchmod, which is
> >> >> >> > safe, and correctly handling error returns (silently treating
> >> >> >> > EOPNOTSUPP as success) rather than as hard errors.
> >> >> >> 
> >> >> >> glibc's lchmod always returns ENOSYS (except on Hurd).  I don't know how
> >> >> >> lchmod is used in coreutils, but I suspect it is not particularly
> >> >> >> useful.
> >> >> >
> >> >> > When preserving permissions (cp -p, archive extraction, etc.), you
> >> >> > want lchmod to work correctly just for the purpose of *not* following
> >> >> > the link and thereby unwantedly changing the permissions of the link
> >> >> > target. But, fchmodat with AT_SYMLINK_NOFOLLOW works just as well and
> >> >> > is standard, and that's really what coreutils should be using.
> >> >> 
> >> >> I think you misread what I wrote: lchmod *always* returns ENOSYS.  Even
> >> >> if the file is not a symbolic link.  Likewise, fchmodat with
> >> >> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.
> >> >
> >> > Yes, I understood that. I was going into why there should be a real
> >> > implementation, but didn't make it clear that that was what I was
> >> > doing.
> >> 
> >> Ah, yes, there should be a real implementation if we can get full
> >> lchmod/AT_SYMLINK_NOFOLLOW behavior on file systems that support it.  If
> >> we can't, I'm not sure if there is a point to it.
> >
> > The point is to fail when the target is a symlink, rather than
> > (erroneously and possibly dangerously) applying the chmod to the link
> > target. Actually supporting link modes is useless. It's the "not
> > modifying the target" that's important.
> 
> The kernel supports it on some file systems, though:
> 
> $ ls -l /tmp/x
> l---------. 1 fweimer fweimer 6 Jan 22 15:27 /tmp/x -> /tmp/x
> 
> Although mode 0 curiously does not prevent readlink calls.
> 
> > It's explained in the bz you just replied on,
> > https://sourceware.org/bugzilla/show_bug.cgi?id=14578
> >
> > The point of the S_ISLNK check is to fail out early with the ENOTSUPP,
> > which the caller should treat as "success-like", in the non-racing
> > condition, without the need to open a fd (which may fail with
> > ENFILE/EMFILE) and without the need for /proc to be mounted.
> > Otherwise, a different error will be produced when one of those cases
> > is hit, and the caller will treat it as a real error.
> 
> Hmm.  The way I read the musl code, the O_PATH descriptor already
> exists.  At this point, you can just chmod the O_PATH descriptor, and
> have the kernel report EOPNOTSUPP if the file system does not support
> that.

Oh, you mean the second one after it's already open? Maybe that's ok.
I was concerned it might follow the link and chmod the target at that
point. I thought you were asking about the ealier check before the
O_PATH open.

Rich

Message #29 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 21:48:14 +0100

* Rich Felker:

>> Hmm.  The way I read the musl code, the O_PATH descriptor already
>> exists.  At this point, you can just chmod the O_PATH descriptor, and
>> have the kernel report EOPNOTSUPP if the file system does not support
>> that.
>
> Oh, you mean the second one after it's already open? Maybe that's ok.

Yes, that's what I meant.

> I was concerned it might follow the link and chmod the target at that
> point.

In my tests, it works.  I think it's also documented behavior for chown
on these pseudo-files.

I also verified that closing an O_PATH descriptor does not release POSIX
advisory locks for the same file.  But I'm wondering if there's still
something we are missing.

Thanks,
Florian

Message #32 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fweimer <at> redhat.com>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 15:56:05 -0500

On Wed, Jan 22, 2020 at 09:48:14PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> >> Hmm.  The way I read the musl code, the O_PATH descriptor already
> >> exists.  At this point, you can just chmod the O_PATH descriptor, and
> >> have the kernel report EOPNOTSUPP if the file system does not support
> >> that.
> >
> > Oh, you mean the second one after it's already open? Maybe that's ok.
> 
> Yes, that's what I meant.
> 
> > I was concerned it might follow the link and chmod the target at that
> > point.
> 
> In my tests, it works.  I think it's also documented behavior for chown
> on these pseudo-files.

Do you know where we might find that documentation?

> I also verified that closing an O_PATH descriptor does not release POSIX
> advisory locks for the same file.  But I'm wondering if there's still
> something we are missing.

Thanks, I hadn't thought to check that, but wouldn't have expected it
to be a problem since O_PATH is not actually open to the file.

Rich

Message #35 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fweimer <at> redhat.com>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 22:05:52 +0100

* Rich Felker:

> On Wed, Jan 22, 2020 at 09:48:14PM +0100, Florian Weimer wrote:
>> * Rich Felker:
>> 
>> >> Hmm.  The way I read the musl code, the O_PATH descriptor already
>> >> exists.  At this point, you can just chmod the O_PATH descriptor, and
>> >> have the kernel report EOPNOTSUPP if the file system does not support
>> >> that.
>> >
>> > Oh, you mean the second one after it's already open? Maybe that's ok.
>> 
>> Yes, that's what I meant.
>> 
>> > I was concerned it might follow the link and chmod the target at that
>> > point.
>> 
>> In my tests, it works.  I think it's also documented behavior for chown
>> on these pseudo-files.
>
> Do you know where we might find that documentation?

Ugh.  I'm probably misremembering.  It may have been a rejection of
patch to implement another f*at system call with AT_EMPTY_PATH support.
(I did find your 2013 message describing the chmod and chown behavior.)

Thanks,
Florian

Message #38 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Florian Weimer <fweimer <at> redhat.com>, Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 13:55:57 -0800

On 1/22/20 7:08 AM, Florian Weimer wrote:
> I think you misread what I wrote: lchmod*always*  returns ENOSYS.  Even
> if the file is not a symbolic link.  Likewise, fchmodat with
> AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.

That's too bad, because coreutils (and many other applications, I 
expect) assume that lchmod (and fchmodat with AT_SYMLINK_NOFOLLOW) to 
act like chmod except not follow symlinks, in order to make it less 
likely that the application will run afoul of a symlink race and chmod 
the wrong file. Isn't that how the Linux fstatat call behaves? And if 
so, why does glibc fstatat refuse to support this behavior?

To work around this bug, I suppose coreutils etc. should do something 
like the following:

1. Never use lchmod since the porting nightmare is bad enough without it.

2. On non-glibc systems (or glibc systems where the bug is fixed), use 
fchmodat with AT_SYMLINK_NOFOLLOW.

3. On glibc systems with the bug, use openat with AT_SYMLINK_NOFOLLOW 
and O_PATH, and then fchmod the resulting file descriptor.

Does this sound right? Or is there some O_PATH gotcha that I haven't 
thought about?

Come to think of it, perhaps the best thing would be to change Gnulib's 
lchmod and fchmodat modules so that they do what applications expect, 
even on buggy glibc systems. (Which would be ironic, since Gnulib's main 
goal is to put wrappers around other libraries so that they look more 
like glibc.)

Message #41 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Florian Weimer <fweimer <at> redhat.com>, musl <at> lists.openwall.com,
 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 22 Jan 2020 17:05:15 -0500

On Wed, Jan 22, 2020 at 01:55:57PM -0800, Paul Eggert wrote:
> On 1/22/20 7:08 AM, Florian Weimer wrote:
> >I think you misread what I wrote: lchmod*always*  returns ENOSYS.  Even
> >if the file is not a symbolic link.  Likewise, fchmodat with
> >AT_SYMLINK_NOFOLLOW *always* returns ENOTSUP.
> 
> That's too bad, because coreutils (and many other applications, I
> expect) assume that lchmod (and fchmodat with AT_SYMLINK_NOFOLLOW)
> to act like chmod except not follow symlinks, in order to make it
> less likely that the application will run afoul of a symlink race
> and chmod the wrong file. Isn't that how the Linux fstatat call
> behaves? And if so, why does glibc fstatat refuse to support this
> behavior?

I think you're confusing fchmodat with fstatat. The Linux fchmodat
syscall lacks a flags argument and thus doesn't suffice to implement
fchmodat. The fstatat syscall does work.

> To work around this bug, I suppose coreutils etc. should do
> something like the following:
> 
> 1. Never use lchmod since the porting nightmare is bad enough without it.
> 
> 2. On non-glibc systems (or glibc systems where the bug is fixed),
> use fchmodat with AT_SYMLINK_NOFOLLOW.
> 
> 3. On glibc systems with the bug, use openat with
> AT_SYMLINK_NOFOLLOW and O_PATH, and then fchmod the resulting file
> descriptor.
> 
> Does this sound right? Or is there some O_PATH gotcha that I haven't
> thought about?

I think fchmod historically did not work on O_PATH file descriptors,
which is why musl is using chmod on the procfs magic symlink. However,
fchmodat might work too with an empty pathname; I'm not sure.

I think these fixes are better encapsulated as a replacement for
missing/broken fchmodat, rather than putting the logic in individual
utilities or coreutils-specific library code.

Also, note that if you want to skip checking stat to make sure you
didn't open a symlink with O_PATH, that depends on confirming
Florian's claim that the kernel documents it will not follow the
symlink.

> Come to think of it, perhaps the best thing would be to change
> Gnulib's lchmod and fchmodat modules so that they do what
> applications expect, even on buggy glibc systems. (Which would be
> ironic, since Gnulib's main goal is to put wrappers around other
> libraries so that they look more like glibc.)

I think we're approaching a consensus that glibc should fix this too,
so then it would just be gnulib matching the fix.

Rich

Message #44 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Rich Felker <dalias <at> libc.org>
Cc: Florian Weimer <fweimer <at> redhat.com>, Gnulib bugs <bug-gnulib <at> gnu.org>,
 musl <at> lists.openwall.com, 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Fri, 7 Feb 2020 16:37:56 -0800

[Message part 1 (text/plain, inline)]

On 1/22/20 2:05 PM, Rich Felker wrote:
> I think we're approaching a consensus that glibc should fix this too,
> so then it would just be gnulib matching the fix.

I installed the attached patch to Gnulib in preparation for the upcoming 
glibc fix. The patch causes fchmodat with AT_SYMLINK_NOFOLLOW to work on 
non-symlinks, and similarly for lchmod on non-symlinks. The idea is to 
avoid this sort of problem in the future, and to let Coreutils etc. work 
on older platforms as if glibc 2.32 (or whatever) is already in place.

[0001-fchmodat-AT_SYMLINK_NOFOLLOW-fix-for-non-symlinks.patch (text/x-patch, attachment)]

Message #47 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fw <at> deneb.enyo.de>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Florian Weimer <fweimer <at> redhat.com>, musl <at> lists.openwall.com,
 Rich Felker <dalias <at> libc.org>, Gnulib bugs <bug-gnulib <at> gnu.org>,
 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 12 Feb 2020 12:50:19 +0100

* Paul Eggert:

> On 1/22/20 2:05 PM, Rich Felker wrote:
>> I think we're approaching a consensus that glibc should fix this too,
>> so then it would just be gnulib matching the fix.
>
> I installed the attached patch to Gnulib in preparation for the upcoming 
> glibc fix. The patch causes fchmodat with AT_SYMLINK_NOFOLLOW to work on 
> non-symlinks, and similarly for lchmod on non-symlinks. The idea is to 
> avoid this sort of problem in the future, and to let Coreutils etc. work 
> on older platforms as if glibc 2.32 (or whatever) is already in place.

The lchmod implementation based on /proc tickles an XFS bug:

  <https://sourceware.org/ml/libc-alpha/2020-02/msg00467.html>

Message #50 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fw <at> deneb.enyo.de>
Cc: Florian Weimer <fweimer <at> redhat.com>, musl <at> lists.openwall.com,
 Paul Eggert <eggert <at> cs.ucla.edu>, Gnulib bugs <bug-gnulib <at> gnu.org>,
 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 12 Feb 2020 08:05:55 -0500

On Wed, Feb 12, 2020 at 12:50:19PM +0100, Florian Weimer wrote:
> * Paul Eggert:
> 
> > On 1/22/20 2:05 PM, Rich Felker wrote:
> >> I think we're approaching a consensus that glibc should fix this too,
> >> so then it would just be gnulib matching the fix.
> >
> > I installed the attached patch to Gnulib in preparation for the upcoming 
> > glibc fix. The patch causes fchmodat with AT_SYMLINK_NOFOLLOW to work on 
> > non-symlinks, and similarly for lchmod on non-symlinks. The idea is to 
> > avoid this sort of problem in the future, and to let Coreutils etc. work 
> > on older platforms as if glibc 2.32 (or whatever) is already in place.
> 
> The lchmod implementation based on /proc tickles an XFS bug:
> 
>   <https://sourceware.org/ml/libc-alpha/2020-02/msg00467.html>

Uhg, why does Linux even let the fs driver see whether the chmod is
being performed via a filename, O_PATH fd, or magic symlink in /proc?
It should just be an operation on the inode.

Rich

Message #53 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: Florian Weimer <fw <at> deneb.enyo.de>
Cc: Florian Weimer <fweimer <at> redhat.com>, musl <at> lists.openwall.com,
 Paul Eggert <eggert <at> cs.ucla.edu>, Gnulib bugs <bug-gnulib <at> gnu.org>,
 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 12 Feb 2020 14:07:42 -0500

On Wed, Feb 12, 2020 at 08:05:55AM -0500, Rich Felker wrote:
> On Wed, Feb 12, 2020 at 12:50:19PM +0100, Florian Weimer wrote:
> > * Paul Eggert:
> > 
> > > On 1/22/20 2:05 PM, Rich Felker wrote:
> > >> I think we're approaching a consensus that glibc should fix this too,
> > >> so then it would just be gnulib matching the fix.
> > >
> > > I installed the attached patch to Gnulib in preparation for the upcoming 
> > > glibc fix. The patch causes fchmodat with AT_SYMLINK_NOFOLLOW to work on 
> > > non-symlinks, and similarly for lchmod on non-symlinks. The idea is to 
> > > avoid this sort of problem in the future, and to let Coreutils etc. work 
> > > on older platforms as if glibc 2.32 (or whatever) is already in place.
> > 
> > The lchmod implementation based on /proc tickles an XFS bug:
> > 
> >   <https://sourceware.org/ml/libc-alpha/2020-02/msg00467.html>
> 
> Uhg, why does Linux even let the fs driver see whether the chmod is
> being performed via a filename, O_PATH fd, or magic symlink in /proc?
> It should just be an operation on the inode.

OK, I don't think it's actually clear from the test that the use of
the magic symlink is the cause. It's plausible that XFS just always
returns failure on success for this operation, and I don't have XFS to
test with.

Note that in any case, musl's lchmod/fchmodat is not affected since it
always refuses to change symlink modes; I did this because I was
worried that chmod on the magic symlink in /proc might pass through
not just to the symlink it refers to, but to the symlink target if one
exists. With current kernel versions it seems that does not happen; is
it safe to assume it doesn't?

Further, I've found some inconsistent behavior with ext4: chmod on the
magic symlink fails with EOPNOTSUPP as in Florian's test, but fchmod
on the O_PATH fd succeeds and changes the symlink mode. This is with
5.4. Cany anyone else confirm this? Is it a problem?

Rich

Message #56 received at 39236 <at> debbugs.gnu.org (full text, mbox):

From: Florian Weimer <fw <at> deneb.enyo.de>
To: Rich Felker <dalias <at> libc.org>
Cc: musl <at> lists.openwall.com, Paul Eggert <eggert <at> cs.ucla.edu>,
 Gnulib bugs <bug-gnulib <at> gnu.org>, 39236 <at> debbugs.gnu.org
Subject: Re: bug#39236: [musl] coreutils cp mishandles error return from lchmod
Date: Wed, 12 Feb 2020 20:13:38 +0100

* Rich Felker:

> Note that in any case, musl's lchmod/fchmodat is not affected since it
> always refuses to change symlink modes; I did this because I was
> worried that chmod on the magic symlink in /proc might pass through
> not just to the symlink it refers to, but to the symlink target if one
> exists. With current kernel versions it seems that does not happen; is
> it safe to assume it doesn't?

I saw it happen with sshfs over FUSE. 8-/

Yet another reason to put in a check before performing the chmod.

> Further, I've found some inconsistent behavior with ext4: chmod on the
> magic symlink fails with EOPNOTSUPP as in Florian's test, but fchmod
> on the O_PATH fd succeeds and changes the symlink mode. This is with
> 5.4. Cany anyone else confirm this? Is it a problem?

Interesting. Let me update the other thread.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.