linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
       [not found]                 ` <5c9fa556-da00-4b76-8a70-8e2d1dddd92d@cs.ucla.edu>
@ 2025-08-27 22:48                   ` Aleksa Sarai
  2025-08-27 23:19                     ` Paul Eggert
  0 siblings, 1 reply; 12+ messages in thread
From: Aleksa Sarai @ 2025-08-27 22:48 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Adhemerval Zanella Netto, Arjun Shankar, libc-alpha, linux-api

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

On 2025-08-27, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 2025-08-26 22:58, Aleksa Sarai wrote:
> > Personally I think both approaches are less than ideal, and having rich
> > feature flags for the entire system would be better but I don't think
> > this is something that would be feasible to apply to everything in the
> > entire kernel.
> 
> Agreed. But I'm not seeing how a hypothetical "give me the supported flags"
> flag would be useful enough to justify the flag.
> 
> I'm looking at this from the user point of view, and it is not ringing a
> bell for me. Granted, the current "try the flag combination you want and see
> whether it works" is not ideal, but it's accurate (which is not always true
> for "give me the supported flags" flag) and you need to do it anyway
> (because the "give me the supported flags" flag is inherently inaccurate),
> so why bother with a "give me the supported flags" flag?
> 
> Here's an example. Suppose we want to extend openat2 so that it also does
> the equivalent of statx atomically with the open, to avoid some races with
> the current openat/fstat pair of system calls. Under the approach you're
> proposing, I suppose we could extend struct open_how so that it has a new
> struct statx member, add new flags to be put into struct open_how's flags
> member, and programs would be able to query the new flags via a "give me the
> supported flags" call.
> 
> But in this scenario, the "give me the supported flags" flag is useless. If
> I'm an old program I can't use the new flags even if I detect them because
> my struct open_how is too small. And if I'm a new program I can simply use
> the new flags - and even if I tested for the new flags (with the "give me
> the supported flags" flag) I'd have to test the result anyway because
> perhaps the new flags are not supported for this particular flag combination
> or file.
> 
> What specific scenario would make the "give me supported flags" flag worth
> the hassle of supporting and documenting and testing such a flag?

"Just try it" leads to programs that have to test dozens of flag
combinations for syscalls at startup, and for many syscalls you cannot
"just try" whether the new flag works (think of a new shutdown(2) flag,
or most clone3(2) flags). What you end up having to do is create an
elaborate setup where if the flag works you get an error (but not
-EINVAL!) so that you can be fairly confident that you didn't modify the
system when doing the check. As someone who has to write this
boilerplate whenever I need to use most system calls, this really
**really** sucks. In some cases you can just try it and then fallback
(caching whether it was supported), but in a lot of programs it is
preferable to know well in advance whether a feature is supported.

A simple example would be mounts -- if MOUNT_BENEATH is not supported
then you need to structure how you construct your mount tree differently
to try to emulate the same behaviour. This means that not knowing if
MOUNT_BENEATH is supported upfront causes you to redo a lot of work in
the fallback case. If changing id-mappings for mounts hadn't required
adding a new syscall, this would've also been an issue for programs that
needed to change the ID-mappings of mounts.

Some kind of "just tell me what flags are supported" mechanism avoids
this problem by telling you in one shot what features are supported (so
newer programs can take advantage of them). Most systems that expect to
be extended over time have something like this, but it's usually in the
form of string-based feature names (/sys/kernel/cgroup/features, for
instance). I wouldn't be against such an idea (if we could actually
guarantee that everyone actually used it), but something similar was
proposed back in 2020 and never happened -- CHECK_FIELDS is a very
simple solution to the problem that works for the most common case and
can be implemented per-syscall.

I've added linux-api to Cc, as I'm sure there are plenty of other ideas
on how to solve this.

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-08-27 22:48                   ` [PATCH v4] linux: Add openat2 (BZ 31664) Aleksa Sarai
@ 2025-08-27 23:19                     ` Paul Eggert
  2025-08-28  8:42                       ` Aleksa Sarai
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Eggert @ 2025-08-27 23:19 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: Adhemerval Zanella Netto, Arjun Shankar, libc-alpha, linux-api

On 2025-08-27 15:48, Aleksa Sarai wrote:
> On 2025-08-27, Paul Eggert <eggert@cs.ucla.edu> wrote:
>> What specific scenario would make the "give me supported flags" flag worth
>> the hassle of supporting and documenting and testing such a flag?
> 
> "Just try it" leads to programs that have to test dozens of flag
> combinations for syscalls at startup,

Although that sort of thing can indeed be a problem in general, I don't 
see how it's a problem for openat2 in particular.

The issue here is whether openat2's API should reflect current behavior 
(where the HOW argument is pointer-to-const) or a potential future 
behavior (where the kernel might modify the struct that HOW points to, 
if some hypothetical future flag is set in that struct). I am skeptical 
that this hypothetical situation is so plausible that it justifies the 
maintenance hassle of a glibc API that doesn't correspond to how openat2 
currently behaves.


> A simple example would be mounts -- if MOUNT_BENEATH is not supported

I don't understand this example. Are you talking about <linux/mount.h>'s 
MOVE_MOUNT_BENEATH? That's a move_mount flag, and I don't see what that 
has to do with openat2. Or are you saying that openat2 might not support 
<linux/openat2.h>'s RESOLVE_BENEATH flag? Under what conditions might 
that be, exactly? Can you give some plausible user code to illustrate 
the openat2 example you're thinking of?

I still fail to understand how a hypothetical "give me the supported 
flags" openat2 flag would be useful enough to justify complicating the 
openat2 API today.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-08-27 23:19                     ` Paul Eggert
@ 2025-08-28  8:42                       ` Aleksa Sarai
  2025-08-28 13:43                         ` Paul Eggert
  0 siblings, 1 reply; 12+ messages in thread
From: Aleksa Sarai @ 2025-08-28  8:42 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Adhemerval Zanella Netto, Arjun Shankar, libc-alpha, linux-api

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

On 2025-08-27, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 2025-08-27 15:48, Aleksa Sarai wrote:
> > On 2025-08-27, Paul Eggert <eggert@cs.ucla.edu> wrote:
> > > What specific scenario would make the "give me supported flags" flag worth
> > > the hassle of supporting and documenting and testing such a flag?
> > 
> > "Just try it" leads to programs that have to test dozens of flag
> > combinations for syscalls at startup,
> 
> Although that sort of thing can indeed be a problem in general, I don't see
> how it's a problem for openat2 in particular.

While O_* and RESOLVE_* flags are trivial to detect (since you can
always pass -EBADF to force a non-EINVAL error), my goal was to have a
unified interface for extensible-struct syscalls in this department.

> The issue here is whether openat2's API should reflect current behavior
> (where the HOW argument is pointer-to-const) or a potential future behavior
> (where the kernel might modify the struct that HOW points to, if some
> hypothetical future flag is set in that struct). I am skeptical that this
> hypothetical situation is so plausible that it justifies the maintenance
> hassle of a glibc API that doesn't correspond to how openat2 currently
> behaves.

I mean, the kernel definition doesn't mark the syscall argument as
"const" so making it const in glibc also means maintaining a divergence
from the kernel. Of course, glibc does this for plenty of other
syscalls so it's not my place to say which is better.

My intention was just to say that this *was* intentiona (which was how I
understood the initial question that I was Cc'd onl, and if you feel
that intention is misguided / doesn't mesh with what glibc wants then
that's your call.

> > A simple example would be mounts -- if MOUNT_BENEATH is not supported
>
> I don't understand this example. Are you talking about <linux/mount.h>'s
> MOVE_MOUNT_BENEATH? That's a move_mount flag, and I don't see what that has
> to do with openat2. Or are you saying that openat2 might not support
> <linux/openat2.h>'s RESOLVE_BENEATH flag? Under what conditions might that
> be, exactly? Can you give some plausible user code to illustrate the openat2
> example you're thinking of?

I was just giving it as an example where "just try it" is not really
ideal for userspace today. clone3(2) is an extensible-struct syscall
that needs this.

> I still fail to understand how a hypothetical "give me the supported flags"
> openat2 flag would be useful enough to justify complicating the openat2 API
> today.

My only concern is that it would break recompiles if/when we change it
back. If that is not a concern for glibc as a project then you are of
course free to do whatever makes sense for glibc.

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-08-28  8:42                       ` Aleksa Sarai
@ 2025-08-28 13:43                         ` Paul Eggert
  2025-08-28 17:06                           ` Adhemerval Zanella Netto
  2025-09-02  2:41                           ` Arjun Shankar
  0 siblings, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2025-08-28 13:43 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: Adhemerval Zanella Netto, Arjun Shankar, libc-alpha, linux-api

On 2025-08-28 01:42, Aleksa Sarai wrote:
>> I still fail to understand how a hypothetical "give me the supported flags"
>> openat2 flag would be useful enough to justify complicating the openat2 API
>> today.
> My only concern is that it would break recompiles if/when we change it
> back.

OK, but from what I can see there's no identified possibility that 
openat2 will modify the objects its arguments point to, just as there's 
no identified possibility that plain openat will do so (in a 
hypothetical extension to remove unnecessary slashes from its filename 
argument, say).

In that case it's pretty clear that glibc should mark the open_how 
argument as pointer-to-const, just as glibc already marks the filename 
argument.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-08-28 13:43                         ` Paul Eggert
@ 2025-08-28 17:06                           ` Adhemerval Zanella Netto
  2025-09-02  2:41                           ` Arjun Shankar
  1 sibling, 0 replies; 12+ messages in thread
From: Adhemerval Zanella Netto @ 2025-08-28 17:06 UTC (permalink / raw)
  To: Paul Eggert, Aleksa Sarai; +Cc: Arjun Shankar, libc-alpha, linux-api



On 28/08/25 10:43, Paul Eggert wrote:
> On 2025-08-28 01:42, Aleksa Sarai wrote:
>>> I still fail to understand how a hypothetical "give me the supported flags"
>>> openat2 flag would be useful enough to justify complicating the openat2 API
>>> today.
>> My only concern is that it would break recompiles if/when we change it
>> back.
> 
> OK, but from what I can see there's no identified possibility that openat2 will modify the objects its arguments point to, just as there's no identified possibility that plain openat will do so (in a hypothetical extension to remove unnecessary slashes from its filename argument, say).
> 
> In that case it's pretty clear that glibc should mark the open_how argument as pointer-to-const, just as glibc already marks the filename argument.

I am still not sure how a potentially CHECK_FIELDS feature would play with 
openat2 in the future, especially since glibc now prefers to first include 
the kernel headers before redefining a minimal API to the syscall usage 
(meaning that programs will have access to potentially new flags depending 
on the installed kernel header).

If the kernel intends to modify the open_how in the future, setting open_how 
as const will only add extra confusion. Users might be exposed to this feature 
without explicitly including the kernel headers.

Another option might to *not* include the kernel headers and keep syncing the 
kernel definitions on kernel releases (and maybe excluding flags that might 
modify the open_how). As Florian has said, this kind of mediation by glibc was 
historically time-consuming, complex, and subject to subtle bugs (and that's 
why we abandoned this over time).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-08-28 13:43                         ` Paul Eggert
  2025-08-28 17:06                           ` Adhemerval Zanella Netto
@ 2025-09-02  2:41                           ` Arjun Shankar
  2025-09-02 16:23                             ` enh
  2025-09-02 16:34                             ` Paul Eggert
  1 sibling, 2 replies; 12+ messages in thread
From: Arjun Shankar @ 2025-09-02  2:41 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Aleksa Sarai, Adhemerval Zanella Netto, libc-alpha, linux-api

Hi Paul,

> On 2025-08-28 01:42, Aleksa Sarai wrote:
> >> I still fail to understand how a hypothetical "give me the supported flags"
> >> openat2 flag would be useful enough to justify complicating the openat2 API
> >> today.
> > My only concern is that it would break recompiles if/when we change it
> > back.
>
> OK, but from what I can see there's no identified possibility that
> openat2 will modify the objects its arguments point to, just as there's
> no identified possibility that plain openat will do so (in a
> hypothetical extension to remove unnecessary slashes from its filename
> argument, say).

While it is true that openat cannot be extended in this way, for
openat2 (whether or not it eventually materializes in Linux) there
already is the RFC patch series proposing CHECK_FIELDS that Aleksa
referred to earlier. And it's not just that: it has been mentioned as
a potential future direction even when the openat2 syscall was
implemented [1]. I think we should interpret this to mean that there
is indeed a possibility for openat2.

> In that case it's pretty clear that glibc should mark the open_how
> argument as pointer-to-const, just as glibc already marks the filename
> argument.

Unless the kernel marks open_how as const, glibc marking it as const
can lead to additional maintenance complications down the line: in the
future if the kernel starts modifying open_how, glibc's openat2
wrapper will no longer align with the kernel's behavior. At that
point, glibc will either need to discard the const (which will cause
any existing users of the wrapper to fail to recompile), or glibc will
need to handle the kernel's new behavior in the wrapper (which will
lead to further divergence from the behavior of the syscall that we
would claim to wrap). Neither of these seems problem-free. On the
other hand, following the kernel's declaration will mean that should
the kernel choose to mark it const, we can easily follow suit in glibc
without breaking recompiles.

Earlier on in this thread, Aleksa mentioned sched_setattr as
establishing precedent for the kernel modifying non-const objects. It
looks like glibc actually does provide a sched_setattr wrapper since
2.41. The relevant argument hasn't been marked as const and the kernel
does modify the contents, and glibc's syscall wrapper simply passes it
through. So we already do this.

Based on all this, I feel that leaving open_how as-is is the easier
and more maintenance-friendly choice for the syscall wrapper.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fddb5d430ad9fa91b49b1d34d0202ffe2fa0e179

--
Arjun Shankar
he/him/his


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-02  2:41                           ` Arjun Shankar
@ 2025-09-02 16:23                             ` enh
  2025-09-03 12:09                               ` Arjun Shankar
  2025-09-02 16:34                             ` Paul Eggert
  1 sibling, 1 reply; 12+ messages in thread
From: enh @ 2025-09-02 16:23 UTC (permalink / raw)
  To: Arjun Shankar
  Cc: Paul Eggert, Aleksa Sarai, Adhemerval Zanella Netto, libc-alpha,
	linux-api

On Mon, Sep 1, 2025 at 10:42 PM Arjun Shankar <arjun@redhat.com> wrote:
>
> Hi Paul,
>
> > On 2025-08-28 01:42, Aleksa Sarai wrote:
> > >> I still fail to understand how a hypothetical "give me the supported flags"
> > >> openat2 flag would be useful enough to justify complicating the openat2 API
> > >> today.
> > > My only concern is that it would break recompiles if/when we change it
> > > back.
> >
> > OK, but from what I can see there's no identified possibility that
> > openat2 will modify the objects its arguments point to, just as there's
> > no identified possibility that plain openat will do so (in a
> > hypothetical extension to remove unnecessary slashes from its filename
> > argument, say).
>
> While it is true that openat cannot be extended in this way, for
> openat2 (whether or not it eventually materializes in Linux) there
> already is the RFC patch series proposing CHECK_FIELDS that Aleksa
> referred to earlier. And it's not just that: it has been mentioned as
> a potential future direction even when the openat2 syscall was
> implemented [1]. I think we should interpret this to mean that there
> is indeed a possibility for openat2.
>
> > In that case it's pretty clear that glibc should mark the open_how
> > argument as pointer-to-const, just as glibc already marks the filename
> > argument.
>
> Unless the kernel marks open_how as const, glibc marking it as const
> can lead to additional maintenance complications down the line: in the
> future if the kernel starts modifying open_how, glibc's openat2
> wrapper will no longer align with the kernel's behavior. At that
> point, glibc will either need to discard the const (which will cause
> any existing users of the wrapper to fail to recompile), or glibc will
> need to handle the kernel's new behavior in the wrapper (which will
> lead to further divergence from the behavior of the syscall that we
> would claim to wrap). Neither of these seems problem-free. On the
> other hand, following the kernel's declaration will mean that should
> the kernel choose to mark it const, we can easily follow suit in glibc
> without breaking recompiles.
>
> Earlier on in this thread, Aleksa mentioned sched_setattr as
> establishing precedent for the kernel modifying non-const objects. It
> looks like glibc actually does provide a sched_setattr wrapper since
> 2.41. The relevant argument hasn't been marked as const and the kernel
> does modify the contents, and glibc's syscall wrapper simply passes it
> through. So we already do this.

given that

SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,
                               unsigned int, flags)

calls sched_setattr(), which is defined thus:

int sched_setattr(struct task_struct *p, const struct sched_attr *attr)
{
        return __sched_setscheduler(p, attr, true, true);
}

i think that's just a copy & paste mistake in the kernel -- carefully
preserved in glibc and bionic -- no?

(i only see the kernel updating its own _copy_ of the passed-in struct.)

> Based on all this, I feel that leaving open_how as-is is the easier
> and more maintenance-friendly choice for the syscall wrapper.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fddb5d430ad9fa91b49b1d34d0202ffe2fa0e179
>
> --
> Arjun Shankar
> he/him/his
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-02  2:41                           ` Arjun Shankar
  2025-09-02 16:23                             ` enh
@ 2025-09-02 16:34                             ` Paul Eggert
  2025-09-02 17:11                               ` Adhemerval Zanella Netto
  1 sibling, 1 reply; 12+ messages in thread
From: Paul Eggert @ 2025-09-02 16:34 UTC (permalink / raw)
  To: Arjun Shankar
  Cc: Aleksa Sarai, Adhemerval Zanella Netto, libc-alpha, linux-api

On 2025-09-01 19:41, Arjun Shankar wrote:
> While it is true that openat cannot be extended in this way, for
> openat2 (whether or not it eventually materializes in Linux) there
> already is the RFC patch series proposing CHECK_FIELDS that Aleksa
> referred to earlier.

Is this the RFC Aleksa proposed last October 
<https://lkml.org/lkml/2024/10/10/25>? If so, I don't exactly see a 
rousing endorsement there.

If not, where is the later RFC? I'd like to send the critical comments 
I've already sent on this thread. These comments have not been responded 
to adequately.


> Unless the kernel marks open_how as const

? The kernel doesn't mark anything as const. It merely copies in or 
copies out. And for openat2, it copies only one way.


> future if the kernel starts modifying open_how, glibc's openat2
> wrapper will no longer align with the kernel's behavior. At that
> point, glibc will either need to discard the const (which will cause
> any existing users of the wrapper to fail to recompile),

There are multiple easy ways out there. For example, glibc could 
document the argument as being pointer-to-const now, but warn that this 
may change to unrestricted pointer later, if the misguided change is 
made to the kernel. This would be similar to the already-existing 
warning in the proposed glibc patch, which warns that you can't assume 
sizeof (struct open_how) is a constant and so you can't expose it in 
library APIs. Of course people can ignore the documentation warnings but 
that's on them.

Better, though, would be to keep the API pointer-to-const. That's much 
cleaner. We can extend it later for a "give me the supported flags" 
flag, wwithout changing it the API away from pointer-to-const.

> Earlier on in this thread, Aleksa mentioned sched_setattr as
> establishing precedent for the kernel modifying non-const objects. It
> looks like glibc actually does provide a sched_setattr wrapper since
> 2.41.

Although it may be too late to change that misfeature, it's not too late 
to change this one. And even if it was a good idea for sched_setattr, 
that doesn't mean it's a good a good idea for openat2.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-02 16:34                             ` Paul Eggert
@ 2025-09-02 17:11                               ` Adhemerval Zanella Netto
  2025-09-02 18:17                                 ` Paul Eggert
  0 siblings, 1 reply; 12+ messages in thread
From: Adhemerval Zanella Netto @ 2025-09-02 17:11 UTC (permalink / raw)
  To: Paul Eggert, Arjun Shankar; +Cc: Aleksa Sarai, libc-alpha, linux-api



On 02/09/25 13:34, Paul Eggert wrote:
> On 2025-09-01 19:41, Arjun Shankar wrote:
>> While it is true that openat cannot be extended in this way, for
>> openat2 (whether or not it eventually materializes in Linux) there
>> already is the RFC patch series proposing CHECK_FIELDS that Aleksa
>> referred to earlier.
> 
> Is this the RFC Aleksa proposed last October <https://lkml.org/lkml/2024/10/10/25>? If so, I don't exactly see a rousing endorsement there.
> 
> If not, where is the later RFC? I'd like to send the critical comments I've already sent on this thread. These comments have not been responded to adequately.
> 
> 
>> Unless the kernel marks open_how as const
> 
> ? The kernel doesn't mark anything as const. It merely copies in or copies out. And for openat2, it copies only one way.

I think in this case we can refer to the SYSCALL_DEFINE4 at the kernel
source kernel, since there is no explicit contract w.r.t to argument 
point-to-const in the kernel header. But I am not sure which is the 
Linux policy for changing the implementation, or if Linus or other
maintainer will chime in if someone tries to do it.


> 
> 
>> future if the kernel starts modifying open_how, glibc's openat2
>> wrapper will no longer align with the kernel's behavior. At that
>> point, glibc will either need to discard the const (which will cause
>> any existing users of the wrapper to fail to recompile),
> 
> There are multiple easy ways out there. For example, glibc could document the argument as being pointer-to-const now, but warn that this may change to unrestricted pointer later, if the misguided change is made to the kernel. This would be similar to the already-existing warning in the proposed glibc patch, which warns that you can't assume sizeof (struct open_how) is a constant and so you can't expose it in library APIs. Of course people can ignore the documentation warnings but that's on them.

Yes, we could proper document it but changing it later is always troublesome
and may force users to start to resort to hacks like bypassing the libc with
assemble hacks, and/or redefine the function prototype, or just using syscall()
instead (all far from ideal) to support multiple glibc version.

So I would *really* like to avoid going forward with this path.

> 
> Better, though, would be to keep the API pointer-to-const. That's much cleaner. We can extend it later for a "give me the supported flags" flag, wwithout changing it the API away from pointer-to-const.
> 
>> Earlier on in this thread, Aleksa mentioned sched_setattr as
>> establishing precedent for the kernel modifying non-const objects. It
>> looks like glibc actually does provide a sched_setattr wrapper since
>> 2.41.
> 
> Although it may be too late to change that misfeature, it's not too late to change this one. And even if it was a good idea for sched_setattr, that doesn't mean it's a good a good idea for openat2.
> 

The main problem I see is if we set for point-to-const, the kernel eventually
adds some API that changes the input, and the users will start to deploy the
aforementioned hacks to overcome possible issues using the glibc interface.

So I think if kernel developers are planing to make the argument in/out I think
glibc should just follow the kernel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-02 17:11                               ` Adhemerval Zanella Netto
@ 2025-09-02 18:17                                 ` Paul Eggert
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Eggert @ 2025-09-02 18:17 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, Arjun Shankar
  Cc: Aleksa Sarai, libc-alpha, linux-api

On 2025-09-02 10:11, Adhemerval Zanella Netto wrote:
> if kernel developers are planing to make the argument in/out

I'm not seeing much evidence for such a plan, and if I saw some I'd make 
my feelings known in the appropriate forum. There's no real need there, 
and the only stated proposal would not accomplish its goals.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-02 16:23                             ` enh
@ 2025-09-03 12:09                               ` Arjun Shankar
  2025-09-03 14:33                                 ` enh
  0 siblings, 1 reply; 12+ messages in thread
From: Arjun Shankar @ 2025-09-03 12:09 UTC (permalink / raw)
  To: enh
  Cc: Paul Eggert, Aleksa Sarai, Adhemerval Zanella Netto, libc-alpha,
	linux-api

Hello!

> > Earlier on in this thread, Aleksa mentioned sched_setattr as
> > establishing precedent for the kernel modifying non-const objects. It
> > looks like glibc actually does provide a sched_setattr wrapper since
> > 2.41. The relevant argument hasn't been marked as const and the kernel
> > does modify the contents, and glibc's syscall wrapper simply passes it
> > through. So we already do this.
>
> given that
>
> SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,
>                                unsigned int, flags)
>
> calls sched_setattr(), which is defined thus:
>
> int sched_setattr(struct task_struct *p, const struct sched_attr *attr)
> {
>         return __sched_setscheduler(p, attr, true, true);
> }
>
> i think that's just a copy & paste mistake in the kernel -- carefully
> preserved in glibc and bionic -- no?
>
> (i only see the kernel updating its own _copy_ of the passed-in struct.)

Based on my understanding, it all happens before the call to the const
marked sched_setattr. Starting from line 986 (as of today) on the same
syscalls.c file [1]:

        retval = sched_copy_attr(uattr, &attr);
        if (retval)
                return retval;

Which inside sched_copy_attr does:

        ret = copy_struct_from_user(attr, sizeof(*attr), uattr, size);
        if (ret) {
                if (ret == -E2BIG)
                        goto err_size;

And that leads to:

err_size:
        put_user(sizeof(*attr), &uattr->size);
        return -E2BIG;

Which writes to userspace.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/syscalls.c?id=e6b9dce0aeeb91dfc0974ab87f02454e24566182#n986

-- 
Arjun Shankar
he/him/his


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] linux: Add openat2 (BZ 31664)
  2025-09-03 12:09                               ` Arjun Shankar
@ 2025-09-03 14:33                                 ` enh
  0 siblings, 0 replies; 12+ messages in thread
From: enh @ 2025-09-03 14:33 UTC (permalink / raw)
  To: Arjun Shankar
  Cc: Paul Eggert, Aleksa Sarai, Adhemerval Zanella Netto, libc-alpha,
	linux-api

On Wed, Sep 3, 2025 at 8:10 AM Arjun Shankar <arjun@redhat.com> wrote:
>
> Hello!
>
> > > Earlier on in this thread, Aleksa mentioned sched_setattr as
> > > establishing precedent for the kernel modifying non-const objects. It
> > > looks like glibc actually does provide a sched_setattr wrapper since
> > > 2.41. The relevant argument hasn't been marked as const and the kernel
> > > does modify the contents, and glibc's syscall wrapper simply passes it
> > > through. So we already do this.
> >
> > given that
> >
> > SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,
> >                                unsigned int, flags)
> >
> > calls sched_setattr(), which is defined thus:
> >
> > int sched_setattr(struct task_struct *p, const struct sched_attr *attr)
> > {
> >         return __sched_setscheduler(p, attr, true, true);
> > }
> >
> > i think that's just a copy & paste mistake in the kernel -- carefully
> > preserved in glibc and bionic -- no?
> >
> > (i only see the kernel updating its own _copy_ of the passed-in struct.)
>
> Based on my understanding, it all happens before the call to the const
> marked sched_setattr. Starting from line 986 (as of today) on the same
> syscalls.c file [1]:
>
>         retval = sched_copy_attr(uattr, &attr);
>         if (retval)
>                 return retval;
>
> Which inside sched_copy_attr does:
>
>         ret = copy_struct_from_user(attr, sizeof(*attr), uattr, size);
>         if (ret) {
>                 if (ret == -E2BIG)
>                         goto err_size;
>
> And that leads to:
>
> err_size:
>         put_user(sizeof(*attr), &uattr->size);
>         return -E2BIG;

oh, wow. it didn't even occur to me to look inside a function called
sched_copy_attr(), though the fact that it wasn't a direct call to
copy_struct_from_user() should perhaps have been a clue :-(

> Which writes to userspace.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/syscalls.c?id=e6b9dce0aeeb91dfc0974ab87f02454e24566182#n986
>
> --
> Arjun Shankar
> he/him/his
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-03 14:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <5d4eaca9-930c-4fca-bdbd-5174733642ee@cs.ucla.edu>
     [not found] ` <b34176ff-883b-4a3d-b48a-8c6421f53ec7@cs.ucla.edu>
     [not found]   ` <2c5ae756-c624-4855-9afb-7b8e8ce91011@linaro.org>
     [not found]     ` <828f6dfb-7402-45e1-a9ed-9e17b6356c5c@linaro.org>
     [not found]       ` <2025-08-25.1756160579-pudgy-swank-chard-regalia-j3jdtD@cyphar.com>
     [not found]         ` <5c3b9baf-76b4-40d7-87fb-9b8dd5afd1ee@cs.ucla.edu>
     [not found]           ` <2025-08-26.1756212515-wealthy-molten-melody-nobody-a5HmWg@cyphar.com>
     [not found]             ` <6432a34d-fba9-414e-ad38-d3354fa0d775@cs.ucla.edu>
     [not found]               ` <2025-08-27.1756273344-decaf-ominous-thrift-twinge-h1gGBI@cyphar.com>
     [not found]                 ` <5c9fa556-da00-4b76-8a70-8e2d1dddd92d@cs.ucla.edu>
2025-08-27 22:48                   ` [PATCH v4] linux: Add openat2 (BZ 31664) Aleksa Sarai
2025-08-27 23:19                     ` Paul Eggert
2025-08-28  8:42                       ` Aleksa Sarai
2025-08-28 13:43                         ` Paul Eggert
2025-08-28 17:06                           ` Adhemerval Zanella Netto
2025-09-02  2:41                           ` Arjun Shankar
2025-09-02 16:23                             ` enh
2025-09-03 12:09                               ` Arjun Shankar
2025-09-03 14:33                                 ` enh
2025-09-02 16:34                             ` Paul Eggert
2025-09-02 17:11                               ` Adhemerval Zanella Netto
2025-09-02 18:17                                 ` Paul Eggert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).