Have RESOLVE_* flags superseded AT_* flags for new syscalls?

Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

	
I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
should be used instead of the equivalent AT_* flags for new system calls.  Is
this the case?

If so, should we comment them as being deprecated in the header file?  And
should they be in linux/fcntl.h rather than linux/openat2.h?

Also:

 (*) It should be noted that the RESOLVE_* flags are not a superset of the
     AT_* flags (there's no equivalent of AT_NO_AUTOMOUNT for example).

 (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
     only RESOLVE_NO_SYMLINKS exists.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

[Cc Florian since that ends up on libc's table sooner or later...]

On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> 	
> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> should be used instead of the equivalent AT_* flags for new system calls.  Is
> this the case?

Imho, it would make sense to use RESOLVE_* flags for new system calls
and afair this was the original intention.
The alternative is that RESOLVE_* flags are special to openat2(). But
that seems strange, imho. The semantics openat2() has might be very
useful for new system calls as well which might also want to support
parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
flags mirroring the new RESOLVE_* flags or we end up adding new
RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
possibility I vote for RESOLVE_* flags going forward. The have better
naming too imho.

An argument against this could be that we might end up causing more
confusion for userspace due to yet another set of flags. But maybe this
isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
When we introduce a new syscall userspace will have to add support for
it anyway.

> 
> If so, should we comment them as being deprecated in the header file?  And
> should they be in linux/fcntl.h rather than linux/openat2.h?
> 
> Also:
> 
>  (*) It should be noted that the RESOLVE_* flags are not a superset of the
>      AT_* flags (there's no equivalent of AT_NO_AUTOMOUNT for example).

That's true but it seems we could just add e.g. RESOLVE_NO_AUTOMOUNT as
soon as we have a new syscall showing up that needs it or we have an
existing syscall (e.g. openat2()) that already uses RESOLVE_* flags and
needs it?

> 
>  (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
>      only RESOLVE_NO_SYMLINKS exists.

I'd be very much in favor of not following symlinks being the default.
That's usually a source of a lot of security issues.
And since no kernel with openat2() has been released there's still time
to switch it and with openat2() being a new syscall it won't hurt if it
has new semantics; I mean it deviates from openat() - intentionally -
already.

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 4999 bytes --]

On 2020-02-28, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> [Cc Florian since that ends up on libc's table sooner or later...]
> 
> On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > 	
> > I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > should be used instead of the equivalent AT_* flags for new system calls.  Is
> > this the case?
> 
> Imho, it would make sense to use RESOLVE_* flags for new system calls
> and afair this was the original intention.

Yes, RESOLVE_ flags would ideally be usable with all new system calls
(though only where it makes sense, obviously). This would make it much
easier for userspace to safely resolve paths without having to go
through several levels of O_PATH fuckery.

The "openat2.h" name was honestly a completely arbitrary decision.

> So we either end up adding new AT_* flags mirroring the new RESOLVE_*
> flags or we end up adding new RESOLVE_* flags mirroring parts of AT_*
> flags. And if that's a possibility I vote for RESOLVE_* flags going
> forward. The have better naming too imho.

I can see the argument for merging AT_ flags into RESOLVE_ flags (fewer
flag arguments for syscalls is usually a good thing) ... but I don't
really like it. There are a couple of problems right off the bat:

 * The prefix RESOLVE_ implies that the flag is specifically about path
   resolution. While you could argue that AT_EMPTY_PATH is at least
   *related* to path resolution, flags like AT_REMOVEDIR and
   AT_RECURSIVE aren't.

 * That point touches on something I see as a more fundamental problem
   in the AT_ flags -- they were intended to be generic flags for all of
   the ...at(2) syscalls. But then AT_ grew things like AT_STATX_ and
   AT_REMOVEDIR (both of which are necessary features to have for their
   respective syscalls, but now those flag bits are dead for other
   syscalls -- not to mention the whole AT_SYMLINK_{NO,}FOLLOW thing).

 * While the above might be seen as minor quibbles, the really big
   issue is that even the flags which are "similar" (AT_SYMLINK_NOFOLLOW
   and RESOLVE_NO_SYMLINKS) have different semantics (by design -- in my
   view, AT_SYMLINK_{NO,}FOLLOW / O_NOFOLLOW / lstat(2) has always had
   the wrong semantics if the intention was to be a way to safely avoid
   resolving symlinks).

But maybe I'm just overthinking what a merge of AT_ and RESOLVE_ would
look like -- would it on.

> An argument against this could be that we might end up causing more
> confusion for userspace due to yet another set of flags. But maybe this
> isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> When we introduce a new syscall userspace will have to add support for
> it anyway.
> 
> > 
> > If so, should we comment them as being deprecated in the header file?  And
> > should they be in linux/fcntl.h rather than linux/openat2.h?
> > 
> > Also:
> > 
> >  (*) It should be noted that the RESOLVE_* flags are not a superset of the
> >      AT_* flags (there's no equivalent of AT_NO_AUTOMOUNT for example).
> 
> That's true but it seems we could just add e.g. RESOLVE_NO_AUTOMOUNT as
> soon as we have a new syscall showing up that needs it or we have an
> existing syscall (e.g. openat2()) that already uses RESOLVE_* flags and
> needs it?

RESOLVE_NO_AUTOMOUNT is on the roadmap for openat2() -- I mentioned it
as future work in the cover letter. :P

But see my above concerns about merging AT_ and RESOLVE_ flags. The
semantic disconnect between AT_ and RESOLVE_ (which is most obvious with
AT_SYMLINK_NOFOLLOW) also exists for AT_NO_AUTOMOUNT.

> >  (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
> >      only RESOLVE_NO_SYMLINKS exists.
> 
> I'd be very much in favor of not following symlinks being the default.
> That's usually a source of a lot of security issues.
> And since no kernel with openat2() has been released there's still time
> to switch it and with openat2() being a new syscall it won't hurt if it
> has new semantics; I mean it deviates from openat() - intentionally -
> already.

I agree in principle, but the problem is that if we want to add new
RESOLVE_ flags you end up with half (or fewer) of the flags being opt-in
with the rest necessarily being opt-out (since the flag not being set
needs to be the old behaviour).

There's also a slight ugliness with RESOLVE_SYMLINKS|RESOLVE_MAGICLINKS
-- should you have to specify both or should RESOLVE_MAGICLINKS imply
RESOLVE_SYMLINKS but only for magic-links. (Is allowing magic-links but
not symlinks even a sane thing to do?)

Also I have a very strong feeling people won't like RESOLVE_XDEV nor
RESOLVE_SYMLINKS being opt-in -- lots of systems use bind-mounts and
symlinks in system paths and developers might not be aware of this.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

On 2020-03-01, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2020-02-28, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > So we either end up adding new AT_* flags mirroring the new RESOLVE_*
> > flags or we end up adding new RESOLVE_* flags mirroring parts of AT_*
> > flags. And if that's a possibility I vote for RESOLVE_* flags going
> > forward. The have better naming too imho.
> 
> I can see the argument for merging AT_ flags into RESOLVE_ flags (fewer
> flag arguments for syscalls is usually a good thing) ... but I don't
> really like it. There are a couple of problems right off the bat:
> 
>  * The prefix RESOLVE_ implies that the flag is specifically about path
>    resolution. While you could argue that AT_EMPTY_PATH is at least
>    *related* to path resolution, flags like AT_REMOVEDIR and
>    AT_RECURSIVE aren't.
> 
>  * That point touches on something I see as a more fundamental problem
>    in the AT_ flags -- they were intended to be generic flags for all of
>    the ...at(2) syscalls. But then AT_ grew things like AT_STATX_ and
>    AT_REMOVEDIR (both of which are necessary features to have for their
>    respective syscalls, but now those flag bits are dead for other
>    syscalls -- not to mention the whole AT_SYMLINK_{NO,}FOLLOW thing).
> 
>  * While the above might be seen as minor quibbles, the really big
>    issue is that even the flags which are "similar" (AT_SYMLINK_NOFOLLOW
>    and RESOLVE_NO_SYMLINKS) have different semantics (by design -- in my
>    view, AT_SYMLINK_{NO,}FOLLOW / O_NOFOLLOW / lstat(2) has always had
>    the wrong semantics if the intention was to be a way to safely avoid
>    resolving symlinks).
> 
> But maybe I'm just overthinking what a merge of AT_ and RESOLVE_ would
> look like -- would it on.

Eugh, dropped the rest of that sentence:

... would it only be the few AT_ flags which are strictly related to
path resolution (such as AT_EMPTY_PATH)? If so wouldn't that just mean
we end up with two flag arguments for new syscalls?

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Sun, Mar 01, 2020 at 02:26:56AM +1100, Aleksa Sarai wrote:
> On 2020-02-28, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > [Cc Florian since that ends up on libc's table sooner or later...]
> > 
> > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > > 	
> > > I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > > should be used instead of the equivalent AT_* flags for new system calls.  Is
> > > this the case?
> > 
> > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > and afair this was the original intention.
> 
> Yes, RESOLVE_ flags would ideally be usable with all new system calls
> (though only where it makes sense, obviously). This would make it much
> easier for userspace to safely resolve paths without having to go
> through several levels of O_PATH fuckery.
> 
> The "openat2.h" name was honestly a completely arbitrary decision.
> 
> > So we either end up adding new AT_* flags mirroring the new RESOLVE_*
> > flags or we end up adding new RESOLVE_* flags mirroring parts of AT_*
> > flags. And if that's a possibility I vote for RESOLVE_* flags going
> > forward. The have better naming too imho.
> 
> I can see the argument for merging AT_ flags into RESOLVE_ flags (fewer
> flag arguments for syscalls is usually a good thing) ... but I don't
> really like it. There are a couple of problems right off the bat:

Sorry, I didn't want to suggest that we simply merge them or make one a
superset of the other (Though I think that's what David first had in
mind.). I rather meant it like: If we need a flag in the RESOLVE_*
namespace that already corresponds to a flag present in the AT_*
namespace we should not use the AT_* flag but rather introduce a new
RESOLVE_* flag. This way we don't end up mixing AT_* and RESOLVE_*
flags. That obviously is optimistic about not ending up with a scenario
where an old syscall suddenly wants a RESOLVE_* flag. That still seems
better than a new syscall wanting RESOLVE_* and AT_*. But maybe I'm
overthinking this.

I think you did a good job by keeping the values apart for now for
RESOLVE_* and AT_*, right? Mixing them would thus be kinda ok but still
feels very messy.

> 
>  * The prefix RESOLVE_ implies that the flag is specifically about path
>    resolution. While you could argue that AT_EMPTY_PATH is at least
>    *related* to path resolution, flags like AT_REMOVEDIR and
>    AT_RECURSIVE aren't.
> 
>  * That point touches on something I see as a more fundamental problem
>    in the AT_ flags -- they were intended to be generic flags for all of
>    the ...at(2) syscalls. But then AT_ grew things like AT_STATX_ and
>    AT_REMOVEDIR (both of which are necessary features to have for their
>    respective syscalls, but now those flag bits are dead for other
>    syscalls -- not to mention the whole AT_SYMLINK_{NO,}FOLLOW thing).

Right, basically why we ended up with RESOLVE_*.

> 
>  * While the above might be seen as minor quibbles, the really big
>    issue is that even the flags which are "similar" (AT_SYMLINK_NOFOLLOW
>    and RESOLVE_NO_SYMLINKS) have different semantics (by design -- in my
>    view, AT_SYMLINK_{NO,}FOLLOW / O_NOFOLLOW / lstat(2) has always had
>    the wrong semantics if the intention was to be a way to safely avoid
>    resolving symlinks).
> 
> But maybe I'm just overthinking what a merge of AT_ and RESOLVE_ would
> look like -- would it on.
> 
> > An argument against this could be that we might end up causing more
> > confusion for userspace due to yet another set of flags. But maybe this
> > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > When we introduce a new syscall userspace will have to add support for
> > it anyway.
> > 
> > > 
> > > If so, should we comment them as being deprecated in the header file?  And
> > > should they be in linux/fcntl.h rather than linux/openat2.h?
> > > 
> > > Also:
> > > 
> > >  (*) It should be noted that the RESOLVE_* flags are not a superset of the
> > >      AT_* flags (there's no equivalent of AT_NO_AUTOMOUNT for example).
> > 
> > That's true but it seems we could just add e.g. RESOLVE_NO_AUTOMOUNT as
> > soon as we have a new syscall showing up that needs it or we have an
> > existing syscall (e.g. openat2()) that already uses RESOLVE_* flags and
> > needs it?
> 
> RESOLVE_NO_AUTOMOUNT is on the roadmap for openat2() -- I mentioned it
> as future work in the cover letter. :P
> 
> But see my above concerns about merging AT_ and RESOLVE_ flags. The
> semantic disconnect between AT_ and RESOLVE_ (which is most obvious with
> AT_SYMLINK_NOFOLLOW) also exists for AT_NO_AUTOMOUNT.
> 
> > >  (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
> > >      only RESOLVE_NO_SYMLINKS exists.
> > 
> > I'd be very much in favor of not following symlinks being the default.
> > That's usually a source of a lot of security issues.
> > And since no kernel with openat2() has been released there's still time
> > to switch it and with openat2() being a new syscall it won't hurt if it
> > has new semantics; I mean it deviates from openat() - intentionally -
> > already.
> 
> I agree in principle, but the problem is that if we want to add new
> RESOLVE_ flags you end up with half (or fewer) of the flags being opt-in
> with the rest necessarily being opt-out (since the flag not being set
> needs to be the old behaviour).

(This also reminds me of the discussion we had with new fd types
being cloexec by default going forward or not. :))
Fair, but that's true for any new flag argument that introduces a
hardening relevant feature that wasn't blocked by default for any
syscall (looking at you mmap(MAP_FIXED/MAP_FIXED_NOREPLACE) :).) I'd
also point out that with symlink resolution we've over time kinda
figured out that it should probably be opt-in rather than opt-out. I
feel that's not necessarily true for something like xdev.

> 
> There's also a slight ugliness with RESOLVE_SYMLINKS|RESOLVE_MAGICLINKS
> -- should you have to specify both or should RESOLVE_MAGICLINKS imply
> RESOLVE_SYMLINKS but only for magic-links. (Is allowing magic-links but
> not symlinks even a sane thing to do?)

Tricky but you have the inverse problem right now, no? Meaning, what
happens if you specify RESOLVE_NO_SYMLINKS but don't specify
RESOLVE_MAGICLINKS? Seems like you'd end up allowing magic links but no
symlinks, no?

> 
> Also I have a very strong feeling people won't like RESOLVE_XDEV nor
> RESOLVE_SYMLINKS being opt-in -- lots of systems use bind-mounts and
> symlinks in system paths and developers might not be aware of this.

I don't see that as a problem because openat2() is a new syscall anyway.
So any application that wants to make use of it needs to get used to
new semantics already. It's not that we're switching an old syscall to
new behavior.

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Sun, Mar 01, 2020 at 02:54:11AM +1100, Aleksa Sarai wrote:
> On 2020-03-01, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > On 2020-02-28, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > > So we either end up adding new AT_* flags mirroring the new RESOLVE_*
> > > flags or we end up adding new RESOLVE_* flags mirroring parts of AT_*
> > > flags. And if that's a possibility I vote for RESOLVE_* flags going
> > > forward. The have better naming too imho.
> > 
> > I can see the argument for merging AT_ flags into RESOLVE_ flags (fewer
> > flag arguments for syscalls is usually a good thing) ... but I don't
> > really like it. There are a couple of problems right off the bat:
> > 
> >  * The prefix RESOLVE_ implies that the flag is specifically about path
> >    resolution. While you could argue that AT_EMPTY_PATH is at least
> >    *related* to path resolution, flags like AT_REMOVEDIR and
> >    AT_RECURSIVE aren't.
> > 
> >  * That point touches on something I see as a more fundamental problem
> >    in the AT_ flags -- they were intended to be generic flags for all of
> >    the ...at(2) syscalls. But then AT_ grew things like AT_STATX_ and
> >    AT_REMOVEDIR (both of which are necessary features to have for their
> >    respective syscalls, but now those flag bits are dead for other
> >    syscalls -- not to mention the whole AT_SYMLINK_{NO,}FOLLOW thing).
> > 
> >  * While the above might be seen as minor quibbles, the really big
> >    issue is that even the flags which are "similar" (AT_SYMLINK_NOFOLLOW
> >    and RESOLVE_NO_SYMLINKS) have different semantics (by design -- in my
> >    view, AT_SYMLINK_{NO,}FOLLOW / O_NOFOLLOW / lstat(2) has always had
> >    the wrong semantics if the intention was to be a way to safely avoid
> >    resolving symlinks).
> > 
> > But maybe I'm just overthinking what a merge of AT_ and RESOLVE_ would
> > look like -- would it on.
> 
> Eugh, dropped the rest of that sentence:
> 
> ... would it only be the few AT_ flags which are strictly related to
> path resolution (such as AT_EMPTY_PATH)? If so wouldn't that just mean
> we end up with two flag arguments for new syscalls?

That's a good question that we kinda ran into right once we
accepted the RESOLVE_* namespace implicitly? This smells like the same
problem we have in e.g. waitid() with WEXITED/WSTOPPED/WCONTINUED and
WNOHANG/WNOWAIT...I think one answer could be one flag argument,
different prefixes? i.e. RESOLVE_* and then e.g. simply REMOVE_DIR instead of
AT_REMOVEDIR. This way we don't duplicate the problem the AT_*
namespace had (e.g. AT_REMOVEDIR and AT_SYMLINK_NOFOLLOW being about two
separate things). Maybe that's crazy and doesn't really make things
better?

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Florian Weimer]

* Christian Brauner:

> [Cc Florian since that ends up on libc's table sooner or later...]

I'm not sure what you are after here …

> On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
>> 	
>> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
>> should be used instead of the equivalent AT_* flags for new system calls.  Is
>> this the case?
>
> Imho, it would make sense to use RESOLVE_* flags for new system calls
> and afair this was the original intention.
> The alternative is that RESOLVE_* flags are special to openat2(). But
> that seems strange, imho. The semantics openat2() has might be very
> useful for new system calls as well which might also want to support
> parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> flags mirroring the new RESOLVE_* flags or we end up adding new
> RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> possibility I vote for RESOLVE_* flags going forward. The have better
> naming too imho.
>
> An argument against this could be that we might end up causing more
> confusion for userspace due to yet another set of flags. But maybe this
> isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> When we introduce a new syscall userspace will have to add support for
> it anyway.

I missed the start of the dicussion and what this is about, sorry.

Regarding open flags, I think the key point for future APIs is to avoid
using the set of flags for both control of the operation itself
(O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
help code that has to re-create an equivalent descriptor.  The operation
flags are largely irrelevant to that if you can get the descriptor by
other means.

>>  (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
>>      only RESOLVE_NO_SYMLINKS exists.
>
> I'd be very much in favor of not following symlinks being the default.
> That's usually a source of a lot of security issues.

But that's inconsistent with the rest of the system.  And for example,
if you make /etc/resolv.conf a symbolic link, a program which uses a new
I/O library (with the new interfaces) will not be able to read it.

AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
so it's relatively little protection.

Thanks,
Florian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 12:30:47PM +0100, Florian Weimer wrote:
> * Christian Brauner:
> 
> > [Cc Florian since that ends up on libc's table sooner or later...]
> 
> I'm not sure what you are after here …

Exactly what you've commented below. Input on whether any of these
changes would be either problematic if you e.g. were to implement
openat() on top of openat2() in the future or if it would be problematic
if we e.g. were to really deprecate AT_* flags for new syscalls.

> 
> > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> >> 	
> >> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> >> should be used instead of the equivalent AT_* flags for new system calls.  Is
> >> this the case?
> >
> > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > and afair this was the original intention.
> > The alternative is that RESOLVE_* flags are special to openat2(). But
> > that seems strange, imho. The semantics openat2() has might be very
> > useful for new system calls as well which might also want to support
> > parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> > flags mirroring the new RESOLVE_* flags or we end up adding new
> > RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> > possibility I vote for RESOLVE_* flags going forward. The have better
> > naming too imho.
> >
> > An argument against this could be that we might end up causing more
> > confusion for userspace due to yet another set of flags. But maybe this
> > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > When we introduce a new syscall userspace will have to add support for
> > it anyway.
> 
> I missed the start of the dicussion and what this is about, sorry.
> 
> Regarding open flags, I think the key point for future APIs is to avoid
> using the set of flags for both control of the operation itself
> (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> help code that has to re-create an equivalent descriptor.  The operation
> flags are largely irrelevant to that if you can get the descriptor by
> other means.
> 
> >>  (*) It has been suggested that AT_SYMLINK_NOFOLLOW should be the default, but
> >>      only RESOLVE_NO_SYMLINKS exists.
> >
> > I'd be very much in favor of not following symlinks being the default.
> > That's usually a source of a lot of security issues.
> 
> But that's inconsistent with the rest of the system.  And for example,
> if you make /etc/resolv.conf a symbolic link, a program which uses a new
> I/O library (with the new interfaces) will not be able to read it.

Fair, but I expect that e.g. a C library would simply implement openat()
on top of openat2() if the latter is available and thus could simply
pass RESOLVE_SYMLINKS so any new I/O library not making use of the
syscall directly would simply get the old behavior. For anyone using the
syscall directly they need to know about its exact semantics anyway. But
again, maybe just having it opt-in is fine.

> 
> AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
> so it's relatively little protection.

So this is partially why I think it's at least worth considerings: the
new RESOLVE_NO_SYMLINKS flag does block all symlink resolution, not just
for the last component in contrast to AT_SYMLINK_NOFOLLOW. This is
278121417a72d87fb29dd8c48801f80821e8f75a

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 12:52:39PM +0100, Christian Brauner wrote:
> On Mon, Mar 02, 2020 at 12:30:47PM +0100, Florian Weimer wrote:
> > * Christian Brauner:
> > 
> > > [Cc Florian since that ends up on libc's table sooner or later...]
> > 
> > I'm not sure what you are after here …
> 
> Exactly what you've commented below. Input on whether any of these
> changes would be either problematic if you e.g. were to implement
> openat() on top of openat2() in the future or if it would be problematic
> if we e.g. were to really deprecate AT_* flags for new syscalls.
> 
> > 
> > > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > >> 	
> > >> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > >> should be used instead of the equivalent AT_* flags for new system calls.  Is
> > >> this the case?
> > >
> > > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > > and afair this was the original intention.
> > > The alternative is that RESOLVE_* flags are special to openat2(). But
> > > that seems strange, imho. The semantics openat2() has might be very
> > > useful for new system calls as well which might also want to support
> > > parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> > > flags mirroring the new RESOLVE_* flags or we end up adding new
> > > RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> > > possibility I vote for RESOLVE_* flags going forward. The have better
> > > naming too imho.
> > >
> > > An argument against this could be that we might end up causing more
> > > confusion for userspace due to yet another set of flags. But maybe this
> > > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > > When we introduce a new syscall userspace will have to add support for
> > > it anyway.
> > 
> > I missed the start of the dicussion and what this is about, sorry.
> > 
> > Regarding open flags, I think the key point for future APIs is to avoid
> > using the set of flags for both control of the operation itself
> > (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> > resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would

Yeah, we have touched on that already and we have other APIs having
related problems. A clean way to avoid this problem is to require new
syscalls to either have two flag arguments, or - if appropriate -
suggest they make use of struct open_how that was implemented for
openat2().

 * @flags: O_* flags.
 * @mode: O_CREAT/O_TMPFILE file mode.
 * @resolve: RESOLVE_* flags.
 */
struct open_how {
	__u64 flags;
	__u64 mode;
	__u64 resolve;
};

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Florian Weimer]

* Christian Brauner:

>> But that's inconsistent with the rest of the system.  And for example,
>> if you make /etc/resolv.conf a symbolic link, a program which uses a new
>> I/O library (with the new interfaces) will not be able to read it.
>
> Fair, but I expect that e.g. a C library would simply implement openat()
> on top of openat2() if the latter is available and thus could simply
> pass RESOLVE_SYMLINKS so any new I/O library not making use of the
> syscall directly would simply get the old behavior. For anyone using the
> syscall directly they need to know about its exact semantics anyway. But
> again, maybe just having it opt-in is fine.

I'm more worried about fancy new libraries which go directly to the new
system calls, but set the wrong defaults for a general-purpose open
operation.

Can we pass RESOLVE_SYMLINKS with O_NOFLLOW, so that we can easily
implement open/openat for architectures that provide only the openat2
system call?

>> AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
>> so it's relatively little protection.
>
> So this is partially why I think it's at least worth considerings: the
> new RESOLVE_NO_SYMLINKS flag does block all symlink resolution, not just
> for the last component in contrast to AT_SYMLINK_NOFOLLOW. This is
> 278121417a72d87fb29dd8c48801f80821e8f75a

RESOLVE_NO_SYMLINKS shouldn't be the default, though (whoever is
responsible for applying that default).  Otherwise system administrators
can no longer move around data between different file systems and set
symbolic links accordingly.

Thanks,
Florian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 01:09:06PM +0100, Florian Weimer wrote:
> * Christian Brauner:
> 
> >> But that's inconsistent with the rest of the system.  And for example,
> >> if you make /etc/resolv.conf a symbolic link, a program which uses a new
> >> I/O library (with the new interfaces) will not be able to read it.
> >
> > Fair, but I expect that e.g. a C library would simply implement openat()
> > on top of openat2() if the latter is available and thus could simply
> > pass RESOLVE_SYMLINKS so any new I/O library not making use of the
> > syscall directly would simply get the old behavior. For anyone using the
> > syscall directly they need to know about its exact semantics anyway. But
> > again, maybe just having it opt-in is fine.
> 
> I'm more worried about fancy new libraries which go directly to the new
> system calls, but set the wrong defaults for a general-purpose open
> operation.
> 
> Can we pass RESOLVE_SYMLINKS with O_NOFLLOW, so that we can easily
> implement open/openat for architectures that provide only the openat2
> system call?

You can currently do RESOLVE_NO_SYMLINKS | O_NOFOLLOW. So I'd expect
RESOLVE_SYMLINKS | O_NOFOLLOW would work as well. But from what it looks
like having no symlink resolution be opt-in seems more likely.

> 
> >> AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
> >> so it's relatively little protection.
> >
> > So this is partially why I think it's at least worth considerings: the
> > new RESOLVE_NO_SYMLINKS flag does block all symlink resolution, not just
> > for the last component in contrast to AT_SYMLINK_NOFOLLOW. This is
> > 278121417a72d87fb29dd8c48801f80821e8f75a
> 
> RESOLVE_NO_SYMLINKS shouldn't be the default, though (whoever is
> responsible for applying that default).  Otherwise system administrators
> can no longer move around data between different file systems and set
> symbolic links accordingly.

Ok, maybe then we'll just leave RESOLVE_NO_SYMLINKS as opt-in.

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 01:20:00PM +0100, Christian Brauner wrote:
> On Mon, Mar 02, 2020 at 01:09:06PM +0100, Florian Weimer wrote:
> > * Christian Brauner:
> > 
> > >> But that's inconsistent with the rest of the system.  And for example,
> > >> if you make /etc/resolv.conf a symbolic link, a program which uses a new
> > >> I/O library (with the new interfaces) will not be able to read it.
> > >
> > > Fair, but I expect that e.g. a C library would simply implement openat()
> > > on top of openat2() if the latter is available and thus could simply
> > > pass RESOLVE_SYMLINKS so any new I/O library not making use of the
> > > syscall directly would simply get the old behavior. For anyone using the
> > > syscall directly they need to know about its exact semantics anyway. But
> > > again, maybe just having it opt-in is fine.
> > 
> > I'm more worried about fancy new libraries which go directly to the new
> > system calls, but set the wrong defaults for a general-purpose open
> > operation.
> > 
> > Can we pass RESOLVE_SYMLINKS with O_NOFLLOW, so that we can easily
> > implement open/openat for architectures that provide only the openat2
> > system call?
> 
> You can currently do RESOLVE_NO_SYMLINKS | O_NOFOLLOW. So I'd expect
> RESOLVE_SYMLINKS | O_NOFOLLOW would work as well. But from what it looks
> like having no symlink resolution be opt-in seems more likely.

One difference to openat() is that openat2() doesn't silently ignore
unknown flags. But I'm not sure that would matter for iplementing
openat() via openat2() since there are no flags that openat() knows about
that openat2() doesn't know about afaict. So the only risks would be
programs that accidently have a bit set that isn't used yet. But that
seems unlikely. And I'm not aware of any flag that was deprecated that
some programs could still pass (a problem we had with CLONE_DETACHED for
example).

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Florian Weimer]

* Christian Brauner:

> One difference to openat() is that openat2() doesn't silently ignore
> unknown flags. But I'm not sure that would matter for iplementing
> openat() via openat2() since there are no flags that openat() knows about
> that openat2() doesn't know about afaict. So the only risks would be
> programs that accidently have a bit set that isn't used yet.

Will there be any new flags for openat in the future?  If not, we can
just use a constant mask in an openat2-based implementation of openat.

Thanks,
Florian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 01:42:50PM +0100, Florian Weimer wrote:
> * Christian Brauner:
> 
> > One difference to openat() is that openat2() doesn't silently ignore
> > unknown flags. But I'm not sure that would matter for iplementing
> > openat() via openat2() since there are no flags that openat() knows about
> > that openat2() doesn't know about afaict. So the only risks would be
> > programs that accidently have a bit set that isn't used yet.
> 
> Will there be any new flags for openat in the future?  If not, we can
> just use a constant mask in an openat2-based implementation of openat.

From past experiences with other syscalls I would expect that any new
features would only be available through openat2().
The way I see it in general is that a revised version of a syscall
basically deprecates the old syscall _wrt to new features_, i.e. new
features will only be available through the revised version unless there
are very strong reasons to also allow it in the old version (security
bug or whatever).
(But I don't want to be presumptuous here and pretend I can make any
definiteve statement. Ultimately it's up to the community, I guess. :))

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Christian Brauner <christian.brauner@ubuntu.com> wrote:

> > AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
> > so it's relatively little protection.
> 
> So this is partially why I think it's at least worth considerings: the
> new RESOLVE_NO_SYMLINKS flag does block all symlink resolution, not just
> for the last component in contrast to AT_SYMLINK_NOFOLLOW. This is
> 278121417a72d87fb29dd8c48801f80821e8f75a

That sounds like a potentially significant UAPI change.  What will that break?

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Florian Weimer <fweimer@redhat.com> wrote:

> Regarding open flags, I think the key point for future APIs is to avoid
> using the set of flags for both control of the operation itself
> (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> help code that has to re-create an equivalent descriptor.  The operation
> flags are largely irrelevant to that if you can get the descriptor by
> other means.

It would also be nice to sort out the problem with O_CLOEXEC.  That can have a
different value, depending on the arch - so it excludes at least three bits
from the O_* flag set.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 02:27:08PM +0000, David Howells wrote:
> Christian Brauner <christian.brauner@ubuntu.com> wrote:
> 
> > > AT_SYMLINK_NOFOLLOW only applies to the last pathname component anyway,
> > > so it's relatively little protection.
> > 
> > So this is partially why I think it's at least worth considerings: the
> > new RESOLVE_NO_SYMLINKS flag does block all symlink resolution, not just
> > for the last component in contrast to AT_SYMLINK_NOFOLLOW. This is
> > 278121417a72d87fb29dd8c48801f80821e8f75a
> 
> That sounds like a potentially significant UAPI change.  What will that break?

I think we settled this and can agree on RESOLVE_NO_SYMLINKS being the
right thing to do, i.e. not resolving symlinks will stay opt-in.
Or is your worry even with the current semantics of openat2()? I don't
see the issue since O_NOFOLLOW still works with openat2().

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Christian Brauner <christian.brauner@ubuntu.com> wrote:

> I think we settled this and can agree on RESOLVE_NO_SYMLINKS being the
> right thing to do, i.e. not resolving symlinks will stay opt-in.
> Or is your worry even with the current semantics of openat2()? I don't
> see the issue since O_NOFOLLOW still works with openat2().

Say, for example, my home dir is on a network volume somewhere and /home has a
symlink pointing to it.  RESOLVE_NO_SYMLINKS cannot be used to access a file
inside my homedir if the pathwalk would go through /home/dhowells - this would
affect fsinfo() - so RESOLVE_NO_SYMLINKS is not a substitute for
AT_SYMLINK_NOFOLLOW (O_NOFOLLOW would not come into it).

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On 2020-03-02, David Howells <dhowells@redhat.com> wrote:
> Florian Weimer <fweimer@redhat.com> wrote:
> 
> > Regarding open flags, I think the key point for future APIs is to avoid
> > using the set of flags for both control of the operation itself
> > (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> > resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> > help code that has to re-create an equivalent descriptor.  The operation
> > flags are largely irrelevant to that if you can get the descriptor by
> > other means.
> 
> It would also be nice to sort out the problem with O_CLOEXEC.  That can have a
> different value, depending on the arch - so it excludes at least three bits
> from the O_* flag set.

Not to mention there are (at least?) three or four different values for
_CLOEXEC for different syscalls...

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 02:50:03PM +0000, David Howells wrote:
> Christian Brauner <christian.brauner@ubuntu.com> wrote:
> 
> > I think we settled this and can agree on RESOLVE_NO_SYMLINKS being the
> > right thing to do, i.e. not resolving symlinks will stay opt-in.
> > Or is your worry even with the current semantics of openat2()? I don't
> > see the issue since O_NOFOLLOW still works with openat2().
> 
> Say, for example, my home dir is on a network volume somewhere and /home has a
> symlink pointing to it.  RESOLVE_NO_SYMLINKS cannot be used to access a file
> inside my homedir if the pathwalk would go through /home/dhowells - this would
> affect fsinfo() - so RESOLVE_NO_SYMLINKS is not a substitute for
> AT_SYMLINK_NOFOLLOW (O_NOFOLLOW would not come into it).

I think we didn't really have this issue/face that question because
openat() never supported AT_SYMLINK_{NO}FOLLOW. Whereas e.g. fsinfo()
does. So in such cases we are back to: either allow both AT_* and
RESOLVE_* flags (imho not the best option) or add (a) new RESOLVE_*
variant(s). It seems we leaned toward the latter so far...

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

On 2020-03-02, David Howells <dhowells@redhat.com> wrote:
> Christian Brauner <christian.brauner@ubuntu.com> wrote:
> 
> > I think we settled this and can agree on RESOLVE_NO_SYMLINKS being the
> > right thing to do, i.e. not resolving symlinks will stay opt-in.
> > Or is your worry even with the current semantics of openat2()? I don't
> > see the issue since O_NOFOLLOW still works with openat2().
> 
> Say, for example, my home dir is on a network volume somewhere and /home has a
> symlink pointing to it.  RESOLVE_NO_SYMLINKS cannot be used to access a file
> inside my homedir if the pathwalk would go through /home/dhowells - this would
> affect fsinfo()

Yes, though this only happens if you're opening "/home/dhowells/foobar".
If you are doing "./foobar" from within "/home/dhowells" it will work
(or if you open a dirfd to "/home/dhowells") -- because no symlink
resolution is done as part of that openat2() call.

> So RESOLVE_NO_SYMLINKS is not a substitute for AT_SYMLINK_NOFOLLOW
> (O_NOFOLLOW would not come into it).

This is what I was saying up-thread -- the semantics are not the same
*on purpose*. If you want "don't follow symlinks *only for the final
component*" then you need to have an AT_SYMLINK_NOFOLLOW equivalent.

My counter-argument is that most people actually want
RESOLVE_NO_SYMLINKS (as evidenced by the countless symlink-related
security bugs -- many of which used O_NOFOLLOW incorrectly), it just
wasn't available before Linux 5.6.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Mon, Mar 02, 2020 at 01:05:04PM +0100, Christian Brauner wrote:
> On Mon, Mar 02, 2020 at 12:52:39PM +0100, Christian Brauner wrote:
> > On Mon, Mar 02, 2020 at 12:30:47PM +0100, Florian Weimer wrote:
> > > * Christian Brauner:
> > > 
> > > > [Cc Florian since that ends up on libc's table sooner or later...]
> > > 
> > > I'm not sure what you are after here …
> > 
> > Exactly what you've commented below. Input on whether any of these
> > changes would be either problematic if you e.g. were to implement
> > openat() on top of openat2() in the future or if it would be problematic
> > if we e.g. were to really deprecate AT_* flags for new syscalls.
> > 
> > > 
> > > > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > > >> 	
> > > >> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > > >> should be used instead of the equivalent AT_* flags for new system calls.  Is
> > > >> this the case?
> > > >
> > > > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > > > and afair this was the original intention.
> > > > The alternative is that RESOLVE_* flags are special to openat2(). But
> > > > that seems strange, imho. The semantics openat2() has might be very
> > > > useful for new system calls as well which might also want to support
> > > > parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> > > > flags mirroring the new RESOLVE_* flags or we end up adding new
> > > > RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> > > > possibility I vote for RESOLVE_* flags going forward. The have better
> > > > naming too imho.
> > > >
> > > > An argument against this could be that we might end up causing more
> > > > confusion for userspace due to yet another set of flags. But maybe this
> > > > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > > > When we introduce a new syscall userspace will have to add support for
> > > > it anyway.
> > > 
> > > I missed the start of the dicussion and what this is about, sorry.
> > > 
> > > Regarding open flags, I think the key point for future APIs is to avoid
> > > using the set of flags for both control of the operation itself
> > > (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> > > resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> 
> Yeah, we have touched on that already and we have other APIs having
> related problems. A clean way to avoid this problem is to require new
> syscalls to either have two flag arguments, or - if appropriate -
> suggest they make use of struct open_how that was implemented for
> openat2().

By the way, if we really means business wrt to: separate resolution from
fd-property falgs then shouldn't we either require O_NOFOLLOW for
openat2() be specified in open_how->resolve or disallow O_NOFOLLOW for
openat2() and introduce a new RESOLVE_* variant?

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Aleksa Sarai <cyphar@cyphar.com> wrote:

> My counter-argument is that most people actually want
> RESOLVE_NO_SYMLINKS (as evidenced by the countless symlink-related
> security bugs -- many of which used O_NOFOLLOW incorrectly), it just
> wasn't available before Linux 5.6.

I would quibble as to whether they actually want this in all situations.
There are some in which the difference in behaviour will conceivably break
things - though that's more the case for things like stat(), statx(), fsinfo()
and getxattr() where you might want to be able to query a specific symlink
than for openat2() where you almost always want to follow it (save O_PATH |
O_NOFOLLOW).

However, if you're okay with me adding, say, RESOLVE_NO_TERMINAL_SYMLINK and
RESOLVE_NO_TERMINAL_AUTOMOUNT, I can use these flags.

I don't want to have to allow both RESOLVE_* and AT_*.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On 2020-03-02, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> On Mon, Mar 02, 2020 at 02:50:03PM +0000, David Howells wrote:
> > Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > 
> > > I think we settled this and can agree on RESOLVE_NO_SYMLINKS being the
> > > right thing to do, i.e. not resolving symlinks will stay opt-in.
> > > Or is your worry even with the current semantics of openat2()? I don't
> > > see the issue since O_NOFOLLOW still works with openat2().
> > 
> > Say, for example, my home dir is on a network volume somewhere and /home has a
> > symlink pointing to it.  RESOLVE_NO_SYMLINKS cannot be used to access a file
> > inside my homedir if the pathwalk would go through /home/dhowells - this would
> > affect fsinfo() - so RESOLVE_NO_SYMLINKS is not a substitute for
> > AT_SYMLINK_NOFOLLOW (O_NOFOLLOW would not come into it).
> 
> I think we didn't really have this issue/face that question because
> openat() never supported AT_SYMLINK_{NO}FOLLOW. Whereas e.g. fsinfo()
> does. So in such cases we are back to: either allow both AT_* and
> RESOLVE_* flags (imho not the best option) or add (a) new RESOLVE_*
> variant(s). It seems we leaned toward the latter so far...

So, RESOLVE_NO_TRAILING_SYMLINKS?

... *sigh*. Yeah, okay I'm fine (though not super happy) with that. We'd
also presumably need RESOLVE_NO_TRAILING_AUTOMOUNTS for David's
AT_NO_AUTOMOUNT usecases -- as well as RESOLVE_NO_AUTOMOUNTS eventually.

Now let's just hope no new syscalls need both AT_RECURSIVE and
RESOLVE_NO_SYMLINKS -- that will put us in a very interesting situation
where you have two ways of specifying "don't follow trailing
symlinks"...

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 3916 bytes --]

On 2020-03-02, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> On Mon, Mar 02, 2020 at 01:05:04PM +0100, Christian Brauner wrote:
> > On Mon, Mar 02, 2020 at 12:52:39PM +0100, Christian Brauner wrote:
> > > On Mon, Mar 02, 2020 at 12:30:47PM +0100, Florian Weimer wrote:
> > > > * Christian Brauner:
> > > > 
> > > > > [Cc Florian since that ends up on libc's table sooner or later...]
> > > > 
> > > > I'm not sure what you are after here …
> > > 
> > > Exactly what you've commented below. Input on whether any of these
> > > changes would be either problematic if you e.g. were to implement
> > > openat() on top of openat2() in the future or if it would be problematic
> > > if we e.g. were to really deprecate AT_* flags for new syscalls.
> > > 
> > > > 
> > > > > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > > > >> 	
> > > > >> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > > > >> should be used instead of the equivalent AT_* flags for new system calls.  Is
> > > > >> this the case?
> > > > >
> > > > > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > > > > and afair this was the original intention.
> > > > > The alternative is that RESOLVE_* flags are special to openat2(). But
> > > > > that seems strange, imho. The semantics openat2() has might be very
> > > > > useful for new system calls as well which might also want to support
> > > > > parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> > > > > flags mirroring the new RESOLVE_* flags or we end up adding new
> > > > > RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> > > > > possibility I vote for RESOLVE_* flags going forward. The have better
> > > > > naming too imho.
> > > > >
> > > > > An argument against this could be that we might end up causing more
> > > > > confusion for userspace due to yet another set of flags. But maybe this
> > > > > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > > > > When we introduce a new syscall userspace will have to add support for
> > > > > it anyway.
> > > > 
> > > > I missed the start of the dicussion and what this is about, sorry.
> > > > 
> > > > Regarding open flags, I think the key point for future APIs is to avoid
> > > > using the set of flags for both control of the operation itself
> > > > (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> > > > resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> > 
> > Yeah, we have touched on that already and we have other APIs having
> > related problems. A clean way to avoid this problem is to require new
> > syscalls to either have two flag arguments, or - if appropriate -
> > suggest they make use of struct open_how that was implemented for
> > openat2().
> 
> By the way, if we really means business wrt to: separate resolution from
> fd-property falgs then shouldn't we either require O_NOFOLLOW for
> openat2() be specified in open_how->resolve or disallow O_NOFOLLOW for
> openat2() and introduce a new RESOLVE_* variant?

I think we agreed a while ago we aren't touching O_ flags for openat2()
because it would hamper adoption (this is the same reason we aren't
fixing the whole O_ACCMODE mess, and O_LARGEFILE, and the arch-specific
O_ flags, and O_TMPFILE, and __O_SYNC, and FASYNC/O_ASYNC, and
__FMODE_EXEC and __FMODE_NONOTIFY, and ...).

To be fair, we did fix O_PATH|O_TMPFILE and invalid mode combinations
but that's only because those were fairly broken.

But as I mentioned in a sister mail, I do agree that allowing O_NOFOLLOW
and RESOLVE_NO_TRAILING_SYMLINKS makes me feel a little uneasy. But
maybe it's totally fine and I'm worrying for no reason.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Tue, Mar 03, 2020 at 02:36:57AM +1100, Aleksa Sarai wrote:
> On 2020-03-02, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > On Mon, Mar 02, 2020 at 01:05:04PM +0100, Christian Brauner wrote:
> > > On Mon, Mar 02, 2020 at 12:52:39PM +0100, Christian Brauner wrote:
> > > > On Mon, Mar 02, 2020 at 12:30:47PM +0100, Florian Weimer wrote:
> > > > > * Christian Brauner:
> > > > > 
> > > > > > [Cc Florian since that ends up on libc's table sooner or later...]
> > > > > 
> > > > > I'm not sure what you are after here …
> > > > 
> > > > Exactly what you've commented below. Input on whether any of these
> > > > changes would be either problematic if you e.g. were to implement
> > > > openat() on top of openat2() in the future or if it would be problematic
> > > > if we e.g. were to really deprecate AT_* flags for new syscalls.
> > > > 
> > > > > 
> > > > > > On Fri, Feb 28, 2020 at 02:53:32PM +0000, David Howells wrote:
> > > > > >> 	
> > > > > >> I've been told that RESOLVE_* flags, which can be found in linux/openat2.h,
> > > > > >> should be used instead of the equivalent AT_* flags for new system calls.  Is
> > > > > >> this the case?
> > > > > >
> > > > > > Imho, it would make sense to use RESOLVE_* flags for new system calls
> > > > > > and afair this was the original intention.
> > > > > > The alternative is that RESOLVE_* flags are special to openat2(). But
> > > > > > that seems strange, imho. The semantics openat2() has might be very
> > > > > > useful for new system calls as well which might also want to support
> > > > > > parts of AT_* flags (see fsinfo()). So we either end up adding new AT_*
> > > > > > flags mirroring the new RESOLVE_* flags or we end up adding new
> > > > > > RESOLVE_* flags mirroring parts of AT_* flags. And if that's a
> > > > > > possibility I vote for RESOLVE_* flags going forward. The have better
> > > > > > naming too imho.
> > > > > >
> > > > > > An argument against this could be that we might end up causing more
> > > > > > confusion for userspace due to yet another set of flags. But maybe this
> > > > > > isn't an issue as long as we restrict RESOLVE_* flags to new syscalls.
> > > > > > When we introduce a new syscall userspace will have to add support for
> > > > > > it anyway.
> > > > > 
> > > > > I missed the start of the dicussion and what this is about, sorry.
> > > > > 
> > > > > Regarding open flags, I think the key point for future APIs is to avoid
> > > > > using the set of flags for both control of the operation itself
> > > > > (O_NOFOLLOW/AT_SYMLINK_NOFOLLOW, O_NOCTTY) and properaties of the
> > > > > resulting descriptor (O_RDWR, O_SYNC).  I expect that doing that would
> > > 
> > > Yeah, we have touched on that already and we have other APIs having
> > > related problems. A clean way to avoid this problem is to require new
> > > syscalls to either have two flag arguments, or - if appropriate -
> > > suggest they make use of struct open_how that was implemented for
> > > openat2().
> > 
> > By the way, if we really means business wrt to: separate resolution from
> > fd-property falgs then shouldn't we either require O_NOFOLLOW for
> > openat2() be specified in open_how->resolve or disallow O_NOFOLLOW for
> > openat2() and introduce a new RESOLVE_* variant?
> 
> I think we agreed a while ago we aren't touching O_ flags for openat2()
> because it would hamper adoption (this is the same reason we aren't
> fixing the whole O_ACCMODE mess, and O_LARGEFILE, and the arch-specific
> O_ flags, and O_TMPFILE, and __O_SYNC, and FASYNC/O_ASYNC, and
> __FMODE_EXEC and __FMODE_NONOTIFY, and ...).
> 
> To be fair, we did fix O_PATH|O_TMPFILE and invalid mode combinations
> but that's only because those were fairly broken.

Right, O_NOFOLLOW would've been kinda neat too because afaict it's the
only flag left that is specifically related to path resolution in there
that would fit nicely into open_how->resolve. :)

> 
> But as I mentioned in a sister mail, I do agree that allowing O_NOFOLLOW
> and RESOLVE_NO_TRAILING_SYMLINKS makes me feel a little uneasy. But

No version of this will be completey satisfying I fear.

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Aleksa Sarai <cyphar@cyphar.com> wrote:

> Now let's just hope no new syscalls need both AT_RECURSIVE and
> RESOLVE_NO_SYMLINKS

Ummm...  AT_RECURSIVE is used by open_tree() to determine whether to copy just
the mount it's looking at or the entire subtree from that point.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Florian Weimer <fweimer@redhat.com> wrote:

> Will there be any new flags for openat in the future?  If not, we can
> just use a constant mask in an openat2-based implementation of openat.

One thing we might want to look at is implementing support for
lock-on-open/create and sharing modes in openat2().  Various network
filesystems support this.  Wine, CIFS and Samba particularly might be
interested in this.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Florian Weimer]

* David Howells:

> Florian Weimer <fweimer@redhat.com> wrote:
>
>> Will there be any new flags for openat in the future?  If not, we can
>> just use a constant mask in an openat2-based implementation of openat.
>
> One thing we might want to look at is implementing support for
> lock-on-open/create and sharing modes in openat2().  Various network
> filesystems support this.  Wine, CIFS and Samba particularly might be
> interested in this.

But will those be O_ flags that need to be passed to openat?

Ignoring locking requests on older kernels because of the openat flag
handling seems problematic.

Thanks,
Florian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Florian Weimer <fweimer@redhat.com> wrote:

> But will those be O_ flags that need to be passed to openat?

Ah, sorry, you were talking about openat().  I doubt there's sufficient O_*
space for that.

David

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[Christian Brauner]

On Fri, Mar 06, 2020 at 01:11:54AM +1100, Aleksa Sarai wrote:
> On 2020-03-02, Florian Weimer <fweimer@redhat.com> wrote:
> > * Christian Brauner:
> > > One difference to openat() is that openat2() doesn't silently ignore
> > > unknown flags. But I'm not sure that would matter for iplementing
> > > openat() via openat2() since there are no flags that openat() knows about
> > > that openat2() doesn't know about afaict. So the only risks would be
> > > programs that accidently have a bit set that isn't used yet.
> > 
> > Will there be any new flags for openat in the future?  If not, we can
> > just use a constant mask in an openat2-based implementation of openat.
> 
> There is one being proposed at the moment as part of the compressed
> read/write work[1].

That work predates openat2() having been merged so there's an argument
to be made that it should be on top of openat2() imho. But that assumes
people agree with
https://lore.kernel.org/linux-fsdevel/3607683.1583419401@warthog.procyon.org.uk/T/#m58c1b6c2697e72e7b42bdbea248178ed31b7d787
and I haven't heard anything in either direction...

Christian

Re: Have RESOLVE_* flags superseded AT_* flags for new syscalls?

[David Howells]

Aleksa Sarai <cyphar@cyphar.com> wrote:

> Right, but open_tree() doesn't need RESOLVE_ flags (nor can you add them
> without an open_tree2()). Instead you can pass an O_PATH file descriptor
> with AT_EMPTY_PATH which you could've safely resolved with openat2().

Note that openat2() is not a substitute for open_tree(). See the effect of
the OPEN_TREE_CLONE flag.

David