* RE: [PATCH 1/2] uapi: split openat2(2) definitions from fcntl.h
From: David Laight @ 2019-12-19 14:05 UTC (permalink / raw)
To: 'Aleksa Sarai', Florian Weimer
Cc: Alexander Viro, Jeff Layton, J. Bruce Fields, Shuah Khan,
Christian Brauner, dev@opencontainers.org,
containers@lists.linux-foundation.org, libc-alpha@sourceware.org,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
In-Reply-To: <20191219134525.mgzmjbsp4wo5b2bw@yavin.dot.cyphar.com>
From: Aleksa Sarai
> Sent: 19 December 2019 13:45
> On 2019-12-19, Florian Weimer <fweimer@redhat.com> wrote:
> > * Aleksa Sarai:
> >
> > > diff --git a/include/uapi/linux/openat2.h b/include/uapi/linux/openat2.h
> > > new file mode 100644
> > > index 000000000000..19ef775e8e5e
> > > --- /dev/null
> > > +++ b/include/uapi/linux/openat2.h
> > > @@ -0,0 +1,41 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI_LINUX_OPENAT2_H
> > > +#define _UAPI_LINUX_OPENAT2_H
> >
> > I think you should include the relevant header for __align_u64
> > etc. here.
>
> Right -- no idea why I forgot to include them.
I'm guessing that is just 64bit aligned on 32bit archs like x86?
No need to enforce it provided the structure will have no padding
on archs where the 64bit fields are 64bit aligned.
A plain __u64 should be fine.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH v4 2/5] pid: Add PIDFD_IOCTL_GETFD to fetch file descriptors from processes
From: Sargun Dhillon @ 2019-12-19 16:15 UTC (permalink / raw)
To: Christian Brauner
Cc: Arnd Bergmann, Oleg Nesterov, Florian Weimer,
linux-kernel@vger.kernel.org, Linux Containers, Linux API,
Linux FS-devel Mailing List, Tycho Andersen, Jann Horn,
Aleksa Sarai, Andy Lutomirski, Al Viro, Gian-Carlo Pascutto,
Emilio Cobos Álvarez, Jed Davis
In-Reply-To: <20191219103525.yqb5f4pbd2dvztkb@wittgenstein>
On Thu, Dec 19, 2019 at 2:35 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
> I guess this is the remaining question we should settle, i.e. what do we
> prefer.
> I still think that adding a new syscall for this seems a bit rich. On
> the other hand it seems that a lot more people agree that using a
> dedicated syscall instead of an ioctl is the correct way; especially
> when it touches core kernel functionality. I mean that was one of the
> takeaways from the pidfd API ioctl-vs-syscall discussion.
>
> A syscall is nicer especially for core-kernel code like this.
> So I guess the only way to find out is to try the syscall approach and
> either get yelled and switch to an ioctl() or have it accepted.
>
> What does everyone else think? Arnd, still in favor of a syscall I take
> it. Oleg, you had suggested a syscall too, right? Florian, any
> thoughts/worries on/about this from the glibc side?
>
> Christian
My feelings towards this are that syscalls might pose a problem if we
ever want to extend this API. Of course we can have a reserved
"flags" field, and populate it later, but what if we turn out to need
a proper struct? I already know we're going to want to add one
around cgroup metadata (net_cls), and likely we'll want to add
a "steal" flag as well. As Arnd mentioned earlier, this is trivial to
fix in a traditional ioctl environment, as ioctls are "cheap". How
do we feel about potentially adding a pidfd_getfd2? Or are we
confident that reserved flags will save us?
^ permalink raw reply
* [PATCH 4.4 078/162] fs/proc: Stop reporting eip and esp in /proc/PID/stat
From: Greg Kroah-Hartman @ 2019-12-19 18:33 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Jann Horn, Andy Lutomirski,
Thomas Gleixner, Al Viro, Andrew Morton, Borislav Petkov,
Brian Gerst, Kees Cook, Linus Torvalds, Linux API, Peter Zijlstra,
Tetsuo Handa, Tycho Andersen, Ingo Molnar, zhangyi (F)
In-Reply-To: <20191219183150.477687052@linuxfoundation.org>
From: Andy Lutomirski <luto@kernel.org>
commit 0a1eb2d474edfe75466be6b4677ad84e5e8ca3f5 upstream.
Reporting these fields on a non-current task is dangerous. If the
task is in any state other than normal kernel code, they may contain
garbage or even kernel addresses on some architectures. (x86_64
used to do this. I bet lots of architectures still do.) With
CONFIG_THREAD_INFO_IN_TASK=y, it can OOPS, too.
As far as I know, there are no use programs that make any material
use of these fields, so just get rid of them.
Reported-by: Jann Horn <jann@thejh.net>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux API <linux-api@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Tycho Andersen <tycho.andersen@canonical.com>
Link: http://lkml.kernel.org/r/a5fed4c3f4e33ed25d4bb03567e329bc5a712bcc.1475257877.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/proc/array.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -425,10 +425,11 @@ static int do_task_stat(struct seq_file
mm = get_task_mm(task);
if (mm) {
vsize = task_vsize(mm);
- if (permitted) {
- eip = KSTK_EIP(task);
- esp = KSTK_ESP(task);
- }
+ /*
+ * esp and eip are intentionally zeroed out. There is no
+ * non-racy way to read them without freezing the task.
+ * Programs that need reliable values can use ptrace(2).
+ */
}
get_task_comm(tcomm, task);
^ permalink raw reply
* [PATCH 4.4 079/162] fs/proc: Report eip/esp in /prod/PID/stat for coredumping
From: Greg Kroah-Hartman @ 2019-12-19 18:33 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Marco Felsch, John Ogness,
Andy Lutomirski, Tycho Andersen, Kees Cook, Peter Zijlstra,
Brian Gerst, Tetsuo Handa, Borislav Petkov, Al Viro, Linux API,
Andrew Morton, Linus Torvalds, Thomas Gleixner, zhangyi (F)
In-Reply-To: <20191219183150.477687052@linuxfoundation.org>
From: John Ogness <john.ogness@linutronix.de>
commit fd7d56270b526ca3ed0c224362e3c64a0f86687a upstream.
Commit 0a1eb2d474ed ("fs/proc: Stop reporting eip and esp in
/proc/PID/stat") stopped reporting eip/esp because it is
racy and dangerous for executing tasks. The comment adds:
As far as I know, there are no use programs that make any
material use of these fields, so just get rid of them.
However, existing userspace core-dump-handler applications (for
example, minicoredumper) are using these fields since they
provide an excellent cross-platform interface to these valuable
pointers. So that commit introduced a user space visible
regression.
Partially revert the change and make the readout possible for
tasks with the proper permissions and only if the target task
has the PF_DUMPCORE flag set.
Fixes: 0a1eb2d474ed ("fs/proc: Stop reporting eip and esp in> /proc/PID/stat")
Reported-by: Marco Felsch <marco.felsch@preh.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Cc: Tycho Andersen <tycho.andersen@canonical.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: stable@vger.kernel.org
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Linux API <linux-api@vger.kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/87poatfwg6.fsf@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[ zhangyi: 68db0cf10678 does not merged, skip the task_stack.h for 4.4]
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/proc/array.c | 8 ++++++++
1 file changed, 8 insertions(+)
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -429,7 +429,15 @@ static int do_task_stat(struct seq_file
* esp and eip are intentionally zeroed out. There is no
* non-racy way to read them without freezing the task.
* Programs that need reliable values can use ptrace(2).
+ *
+ * The only exception is if the task is core dumping because
+ * a program is not able to use ptrace(2) in that case. It is
+ * safe because the task has stopped executing permanently.
*/
+ if (permitted && (task->flags & PF_DUMPCORE)) {
+ eip = KSTK_EIP(task);
+ esp = KSTK_ESP(task);
+ }
}
get_task_comm(tcomm, task);
^ permalink raw reply
* Re: [PATCH for 5.4 0/3] Restartable Sequences Fixes
From: Shuah Khan @ 2019-12-20 1:41 UTC (permalink / raw)
To: Mathieu Desnoyers, Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, paulmck, Boqun Feng, H. Peter Anvin,
Paul Turner, linux-api, stable, Shuah Khan
In-Reply-To: <b67930c1-c8e0-124f-9a88-6ecace27317c@linuxfoundation.org>
On 12/11/19 8:47 AM, Shuah Khan wrote:
> On 12/11/19 8:28 AM, Mathieu Desnoyers wrote:
>> Hi Thomas,
>>
>> I thought those rseq fixes posted in September were in the -tip tree,
>> but it
>> seems that they never made it to mainline.
>>
>> Now Shuah Khan noticed the issue with gettid() compatibility with glibc
>> 2.30+. This series contained that fix.
>>
>> Should I re-post it, or is this series on track to get into mainline
>> at some point ?
>>
>
> It will be great this can make it into 5.5-rc2 or so.
>
> thanks,
> -- Shuah
>
I am pulling this patch in for Linux 5.5-rc4.
Let me know if you have any objections.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v4 2/5] pid: Add PIDFD_IOCTL_GETFD to fetch file descriptors from processes
From: Andy Lutomirski @ 2019-12-20 1:43 UTC (permalink / raw)
To: Sargun Dhillon
Cc: LKML, Linux Containers, Linux API, Linux FS Devel, Tycho Andersen,
Jann Horn, Aleksa Sarai, Christian Brauner, Oleg Nesterov,
Al Viro, gpascutto, ealvarez, Florian Weimer, jld, Arnd Bergmann
In-Reply-To: <20191218235459.GA17271@ircssh-2.c.rugged-nimbus-611.internal>
On Wed, Dec 18, 2019 at 3:55 PM Sargun Dhillon <sargun@sargun.me> wrote:
>
> +
> + if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
> + file = ERR_PTR(-EPERM);
> + goto out;
> + }
I don't think this is MODE_READ. By copying an fd from the task, you
can easily change its state.
IMO it would be really nice if pidfd could act more like a capability
here and carry a ptrace mode, for example. But I guess it doesn't
right now.
--Andy
^ permalink raw reply
* Re: [PATCH for 5.5 1/1] rseq/selftests: Turn off timeout setting
From: Shuah Khan @ 2019-12-20 1:44 UTC (permalink / raw)
To: Mathieu Desnoyers, Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable, Dmitry Vyukov,
Shuah Khan
In-Reply-To: <20191211162857.11354-1-mathieu.desnoyers@efficios.com>
On 12/11/19 9:28 AM, Mathieu Desnoyers wrote:
> As the rseq selftests can run for a long period of time, disable the
> timeout that the general selftests have.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: "H . Peter Anvin" <hpa@zytor.com>
> Cc: Paul Turner <pjt@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> ---
> tools/testing/selftests/rseq/settings | 1 +
> 1 file changed, 1 insertion(+)
> create mode 100644 tools/testing/selftests/rseq/settings
>
> diff --git a/tools/testing/selftests/rseq/settings b/tools/testing/selftests/rseq/settings
> new file mode 100644
> index 000000000000..e7b9417537fb
> --- /dev/null
> +++ b/tools/testing/selftests/rseq/settings
> @@ -0,0 +1 @@
> +timeout=0
>
I am pulling this patch in for Linux 5.5-rc4.
Let me know if you have any objections.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH for 5.5 3/3] rseq/selftests: Fix: Namespace gettid() for compatibility with glibc 2.30
From: Shuah Khan @ 2019-12-20 1:44 UTC (permalink / raw)
To: Mathieu Desnoyers, Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable,
Tommi T . Rantala, Dmitry Vyukov, Shuah Khan
In-Reply-To: <20191211161713.4490-4-mathieu.desnoyers@efficios.com>
On 12/11/19 9:17 AM, Mathieu Desnoyers wrote:
> glibc 2.30 introduces gettid() in public headers, which clashes with
> the internal static definition within rseq selftests.
>
> Rename gettid() to rseq_gettid() to eliminate this symbol name clash.
>
> Reported-by: Tommi T. Rantala <tommi.t.rantala@nokia.com>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Tommi T. Rantala <tommi.t.rantala@nokia.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: "H . Peter Anvin" <hpa@zytor.com>
> Cc: Paul Turner <pjt@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: <stable@vger.kernel.org> # v4.18+
> ---
> tools/testing/selftests/rseq/param_test.c | 18 ++++++++++--------
I am pulling this patch in for Linux 5.5-rc4.
Let me know if you have any objections.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v4 2/5] pid: Add PIDFD_IOCTL_GETFD to fetch file descriptors from processes
From: Aleksa Sarai @ 2019-12-20 4:35 UTC (permalink / raw)
To: Sargun Dhillon
Cc: Christian Brauner, Arnd Bergmann, Oleg Nesterov, Florian Weimer,
linux-kernel@vger.kernel.org, Linux Containers, Linux API,
Linux FS-devel Mailing List, Tycho Andersen, Jann Horn,
Andy Lutomirski, Al Viro, Gian-Carlo Pascutto,
Emilio Cobos Álvarez, Jed Davis
In-Reply-To: <CAMp4zn_z-CCQYMpT=GjZeGVLobjHBCSbmfha1rtWdmptOQ8JtA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]
On 2019-12-19, Sargun Dhillon <sargun@sargun.me> wrote:
> On Thu, Dec 19, 2019 at 2:35 AM Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> > I guess this is the remaining question we should settle, i.e. what do we
> > prefer.
> > I still think that adding a new syscall for this seems a bit rich. On
> > the other hand it seems that a lot more people agree that using a
> > dedicated syscall instead of an ioctl is the correct way; especially
> > when it touches core kernel functionality. I mean that was one of the
> > takeaways from the pidfd API ioctl-vs-syscall discussion.
> >
> > A syscall is nicer especially for core-kernel code like this.
> > So I guess the only way to find out is to try the syscall approach and
> > either get yelled and switch to an ioctl() or have it accepted.
> >
> > What does everyone else think? Arnd, still in favor of a syscall I take
> > it. Oleg, you had suggested a syscall too, right? Florian, any
> > thoughts/worries on/about this from the glibc side?
> >
> > Christian
>
> My feelings towards this are that syscalls might pose a problem if we
> ever want to extend this API. Of course we can have a reserved
> "flags" field, and populate it later, but what if we turn out to need
> a proper struct? I already know we're going to want to add one
> around cgroup metadata (net_cls), and likely we'll want to add
> a "steal" flag as well. As Arnd mentioned earlier, this is trivial to
> fix in a traditional ioctl environment, as ioctls are "cheap". How
> do we feel about potentially adding a pidfd_getfd2? Or are we
> confident that reserved flags will save us?
If we end up making this a syscall, then we can re-use the
copy_struct_from_user() API to make it both extensible and compatible in
both directions. I wasn't aware that this was frowned upon for ioctls
(sorry for the extra work) but there are several syscalls which use this
model for extendability (clone3, openat2, sched_setattr,
perf_events_open) so there shouldn't be any such complaints for a
syscall which is extensible.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/5] pid: Add PIDFD_IOCTL_GETFD to fetch file descriptors from processes
From: Sargun Dhillon @ 2019-12-20 5:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: LKML, Linux Containers, Linux API, Linux FS Devel, Tycho Andersen,
Jann Horn, Aleksa Sarai, Christian Brauner, Oleg Nesterov,
Al Viro, Gian-Carlo Pascutto, Emilio Cobos Álvarez,
Florian Weimer, Jed Davis, Arnd Bergmann
In-Reply-To: <CALCETrUK-SHA=sOUrBscpf+Bpxxff2L3RpXEaAfRHNnHGxa-LQ@mail.gmail.com>
On Thu, Dec 19, 2019 at 5:43 PM Andy Lutomirski <luto@kernel.org> wrote:
>
>
> I don't think this is MODE_READ. By copying an fd from the task, you
> can easily change its state.
Would PTRACE_MODE_ATTACH_REALCREDS work? I'm curious what
kind of state change you can cause by borrowing an FD?
>
> IMO it would be really nice if pidfd could act more like a capability
> here and carry a ptrace mode, for example. But I guess it doesn't
> right now.
>
>
> --Andy
^ permalink raw reply
* Re: [PATCH v4 2/5] pid: Add PIDFD_IOCTL_GETFD to fetch file descriptors from processes
From: Christian Brauner @ 2019-12-20 9:20 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Sargun Dhillon, ealvarez, Arnd Bergmann, Jann Horn, gpascutto,
Linux API, Linux Containers, jld, LKML, Oleg Nesterov, Al Viro,
Linux FS Devel
In-Reply-To: <CALCETrUK-SHA=sOUrBscpf+Bpxxff2L3RpXEaAfRHNnHGxa-LQ@mail.gmail.com>
On Fri, Dec 20, 2019 at 2:43 AM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Wed, Dec 18, 2019 at 3:55 PM Sargun Dhillon <sargun@sargun.me> wrote:
> >
> > +
> > + if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
> > + file = ERR_PTR(-EPERM);
> > + goto out;
> > + }
>
> I don't think this is MODE_READ. By copying an fd from the task, you
> can easily change its state.
>
> IMO it would be really nice if pidfd could act more like a capability
That's ultimately what I would like to get to.
> here and carry a ptrace mode, for example. But I guess it doesn't
> right now.
It doesn't right now for mainly two reasons.
The way I think about it is that a pidfd gets a capability at process
creation time. Before v5.3 we couldn't have done that because legacy
clone() couldn't be extended anymore. Imho, this has changed with clone3().
The other reason was that the basic properties a process can be created
with right now do not lend itself to be turned into a capability. Even
if they did
suddenly treating them like such would prevent userspace from switching to
clone3() because it would regress usecases they had.
However, for new properties this is not a problem. I have some ideas around this
(e.g. spawning private processes only reapable through pidfds and auto-cleanup
if there's no pidfd anymore).
>From an implementation perspective clone3() could get a __aligned_u64 caps
(naming up for debate since we don't want people to think this is equivalent
to our current capabilities) field.
Where at process creation time you could e.g. specify PIDFD_CAP_GET_FD and
only then can you use that pidfd to get file descriptors from other processes.
You still need ptrace_access() to get the actual fd of course.
Christian
^ permalink raw reply
* Re: [PATCH 1/2] uapi: split openat2(2) definitions from fcntl.h
From: Aleksa Sarai @ 2019-12-20 9:31 UTC (permalink / raw)
To: David Laight
Cc: Florian Weimer, Alexander Viro, Jeff Layton, J. Bruce Fields,
Shuah Khan, Christian Brauner, dev@opencontainers.org,
containers@lists.linux-foundation.org, libc-alpha@sourceware.org,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
In-Reply-To: <845fc9e8b55e4868bb4d20655e674b50@AcuMS.aculab.com>
[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]
On 2019-12-19, David Laight <David.Laight@ACULAB.COM> wrote:
> From: Aleksa Sarai
> > Sent: 19 December 2019 13:45
> > On 2019-12-19, Florian Weimer <fweimer@redhat.com> wrote:
> > > * Aleksa Sarai:
> > >
> > > > diff --git a/include/uapi/linux/openat2.h b/include/uapi/linux/openat2.h
> > > > new file mode 100644
> > > > index 000000000000..19ef775e8e5e
> > > > --- /dev/null
> > > > +++ b/include/uapi/linux/openat2.h
> > > > @@ -0,0 +1,41 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI_LINUX_OPENAT2_H
> > > > +#define _UAPI_LINUX_OPENAT2_H
> > >
> > > I think you should include the relevant header for __align_u64
> > > etc. here.
> >
> > Right -- no idea why I forgot to include them.
>
> I'm guessing that is just 64bit aligned on 32bit archs like x86?
Yeah,
#define __aligned_u64 __u64 __attribute__((aligned(8)))
> No need to enforce it provided the structure will have no padding on
> archs where the 64bit fields are 64bit aligned. A plain __u64 should
> be fine.
Will this cause problems for x86-on-x86_64 emulation? Requiring an
8-byte alignment for 'struct open_how' really isn't that undue of a
burden IMHO. Then again, clone3 is a bit of an outlier since both
perf_event_open and sched_setattr just use __u64s.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* RE: [PATCH 1/2] uapi: split openat2(2) definitions from fcntl.h
From: David Laight @ 2019-12-20 10:18 UTC (permalink / raw)
To: 'Aleksa Sarai'
Cc: Florian Weimer, Alexander Viro, Jeff Layton, J. Bruce Fields,
Shuah Khan, Christian Brauner, dev@opencontainers.org,
containers@lists.linux-foundation.org, libc-alpha@sourceware.org,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
In-Reply-To: <20191220093153.v7jpzvch3lohabll@yavin.dot.cyphar.com>
From: Aleksa Sarai
> Sent: 20 December 2019 09:32
...
> > I'm guessing that is just 64bit aligned on 32bit archs like x86?
>
> Yeah,
>
> #define __aligned_u64 __u64 __attribute__((aligned(8)))
>
> > No need to enforce it provided the structure will have no padding on
> > archs where the 64bit fields are 64bit aligned. A plain __u64 should
> > be fine.
>
> Will this cause problems for x86-on-x86_64 emulation? Requiring an
> 8-byte alignment for 'struct open_how' really isn't that undue of a
> burden IMHO. Then again, clone3 is a bit of an outlier since both
> perf_event_open and sched_setattr just use __u64s.
Makes diddly-squit difference.
The 64bit kernel will 64bit align the structure.
The kernel must allow for the userspace structure having arbitrary alignment.
So there is no reason to (try to) align the user structure.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH v2 0/2] openat2: minor uapi cleanups
From: Aleksa Sarai @ 2019-12-20 14:03 UTC (permalink / raw)
To: Alexander Viro, Jeff Layton, J. Bruce Fields, Shuah Khan
Cc: Aleksa Sarai, Florian Weimer, David Laight, Christian Brauner,
dev, containers, libc-alpha, linux-api, linux-fsdevel,
linux-kernel, linux-kselftest
Patch changelog:
v2:
* Add include <linux/types.h> to openat2.h. [Florian Weimer]
* Move OPEN_HOW_SIZE_* constants out of UAPI. [Florian Weimer]
* Switch from __aligned_u64 to __u64 since it isn't necessary.
[David Laight]
v1: <https://lore.kernel.org/lkml/20191219105533.12508-1-cyphar@cyphar.com/>
While openat2(2) is still not yet in Linus's tree, we can take this
opportunity to iron out some small warts that weren't noticed earlier:
* A fix was suggested by Florian Weimer, to separate the openat2
definitions so glibc can use the header directly. I've put the
maintainership under VFS but let me know if you'd prefer it belong
ot the fcntl folks.
* Having heterogenous field sizes in an extensible struct results in
"padding hole" problems when adding new fields (in addition the
correct error to use for non-zero padding isn't entirely clear ).
The simplest solution is to just copy clone(3)'s model -- always use
u64s. It will waste a little more space in the struct, but it
removes a possible future headache.
Aleksa Sarai (2):
openat2: drop open_how->__padding field
uapi: split openat2(2) definitions from fcntl.h
MAINTAINERS | 1 +
fs/open.c | 2 -
include/linux/fcntl.h | 4 ++
include/uapi/linux/fcntl.h | 37 +-----------------
include/uapi/linux/openat2.h | 39 +++++++++++++++++++
tools/testing/selftests/openat2/helpers.h | 7 ++--
.../testing/selftests/openat2/openat2_test.c | 24 ++++--------
7 files changed, 56 insertions(+), 58 deletions(-)
create mode 100644 include/uapi/linux/openat2.h
base-commit: 912dfe068c43fa13c587b8d30e73d335c5ba7d44
--
2.24.0
^ permalink raw reply
* [PATCH v2 1/2] openat2: drop open_how->__padding field
From: Aleksa Sarai @ 2019-12-20 14:03 UTC (permalink / raw)
To: Alexander Viro, Jeff Layton, J. Bruce Fields, Shuah Khan
Cc: Aleksa Sarai, Christian Brauner, David Laight, Florian Weimer,
dev, containers, libc-alpha, linux-api, linux-fsdevel,
linux-kernel, linux-kselftest
In-Reply-To: <20191220140328.20907-1-cyphar@cyphar.com>
The purpose of explicit padding was to allow us to use the space in the
future (C provides no guarantee about the value of padding bytes and
thus userspace could've provided garbage).
However, the downside of explicit padding is that any extension we wish
to add should fit the space exactly (otherwise we may end up with a u16
which will never be used). In addition, the correct error to return for
non-zero padding is not clear (-EINVAL doesn't imply "you're using an
extension field unsupported by this kernel", but -E2BIG seems a bit odd
if the structure size isn't different).
The simplest solution is to just match the design of clone3(2) -- use
u64s for all fields. The extra few-bytes cost of extra fields is not
significant (it's unlikely configuration structs will ever be extremely
large) and it allows for more flag space if necessary. There is also no
need to align the u64s because we will not permit any padding in the
structure.
As openat2(2) is not yet in Linus's tree, we can iron out these minor
warts before we commit to this as a stable ABI.
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Suggested-by: David Laight <david.laight@aculab.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/open.c | 2 --
include/uapi/linux/fcntl.h | 17 +++++++------
tools/testing/selftests/openat2/helpers.h | 7 +++---
.../testing/selftests/openat2/openat2_test.c | 24 +++++++------------
4 files changed, 19 insertions(+), 31 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 50a46501bcc9..8cdb2b675867 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -993,8 +993,6 @@ static inline int build_open_flags(const struct open_how *how,
return -EINVAL;
if (how->resolve & ~VALID_RESOLVE_FLAGS)
return -EINVAL;
- if (memchr_inv(how->__padding, 0, sizeof(how->__padding)))
- return -EINVAL;
/* Deal with the mode. */
if (WILL_CREATE(flags)) {
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index d886bdb585e4..5aaadfd79dd5 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -101,22 +101,21 @@
#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
/*
- * Arguments for how openat2(2) should open the target path. If @resolve is
- * zero, then openat2(2) operates very similarly to openat(2).
+ * Arguments for how openat2(2) should open the target path. If only @flags and
+ * @mode are non-zero, then openat2(2) operates very similarly to openat(2).
*
- * However, unlike openat(2), unknown bits in @flags result in -EINVAL rather
- * than being silently ignored. @mode must be zero unless one of {O_CREAT,
- * O_TMPFILE} are set.
+ * However, unlike openat(2), unknown or invalid bits in @flags result in
+ * -EINVAL rather than being silently ignored. @mode must be zero unless one of
+ * {O_CREAT, O_TMPFILE} are set.
*
* @flags: O_* flags.
* @mode: O_CREAT/O_TMPFILE file mode.
* @resolve: RESOLVE_* flags.
*/
struct open_how {
- __aligned_u64 flags;
- __u16 mode;
- __u16 __padding[3]; /* must be zeroed */
- __aligned_u64 resolve;
+ __u64 flags;
+ __u64 mode;
+ __u64 resolve;
};
#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
diff --git a/tools/testing/selftests/openat2/helpers.h b/tools/testing/selftests/openat2/helpers.h
index 43ca5ceab6e3..a6ea27344db2 100644
--- a/tools/testing/selftests/openat2/helpers.h
+++ b/tools/testing/selftests/openat2/helpers.h
@@ -36,10 +36,9 @@
* @resolve: RESOLVE_* flags.
*/
struct open_how {
- __aligned_u64 flags;
- __u16 mode;
- __u16 __padding[3]; /* must be zeroed */
- __aligned_u64 resolve;
+ __u64 flags;
+ __u64 mode;
+ __u64 resolve;
};
#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c
index 0b64fedc008b..b386367c606b 100644
--- a/tools/testing/selftests/openat2/openat2_test.c
+++ b/tools/testing/selftests/openat2/openat2_test.c
@@ -40,7 +40,7 @@ struct struct_test {
int err;
};
-#define NUM_OPENAT2_STRUCT_TESTS 10
+#define NUM_OPENAT2_STRUCT_TESTS 7
#define NUM_OPENAT2_STRUCT_VARIATIONS 13
void test_openat2_struct(void)
@@ -57,20 +57,6 @@ void test_openat2_struct(void)
.arg.inner.flags = O_RDONLY,
.size = sizeof(struct open_how_ext) },
- /* Normal struct with broken padding. */
- { .name = "normal struct (non-zero padding[0])",
- .arg.inner.flags = O_RDONLY,
- .arg.inner.__padding = {0xa0, 0x00, 0x00},
- .size = sizeof(struct open_how_ext), .err = -EINVAL },
- { .name = "normal struct (non-zero padding[1])",
- .arg.inner.flags = O_RDONLY,
- .arg.inner.__padding = {0x00, 0x1a, 0x00},
- .size = sizeof(struct open_how_ext), .err = -EINVAL },
- { .name = "normal struct (non-zero padding[2])",
- .arg.inner.flags = O_RDONLY,
- .arg.inner.__padding = {0x00, 0x00, 0xef},
- .size = sizeof(struct open_how_ext), .err = -EINVAL },
-
/* TODO: Once expanded, check zero-padding. */
/* Smaller than version-0 struct. */
@@ -169,7 +155,7 @@ struct flag_test {
int err;
};
-#define NUM_OPENAT2_FLAG_TESTS 21
+#define NUM_OPENAT2_FLAG_TESTS 23
void test_openat2_flags(void)
{
@@ -214,9 +200,15 @@ void test_openat2_flags(void)
{ .name = "invalid how.mode and O_CREAT",
.how.flags = O_CREAT,
.how.mode = 0xFFFF, .err = -EINVAL },
+ { .name = "invalid (very large) how.mode and O_CREAT",
+ .how.flags = O_CREAT,
+ .how.mode = 0xC000000000000000ULL, .err = -EINVAL },
{ .name = "invalid how.mode and O_TMPFILE",
.how.flags = O_TMPFILE | O_RDWR,
.how.mode = 0x1337, .err = -EINVAL },
+ { .name = "invalid (very large) how.mode and O_TMPFILE",
+ .how.flags = O_TMPFILE | O_RDWR,
+ .how.mode = 0x0000A00000000000ULL, .err = -EINVAL },
/* ->resolve must only contain RESOLVE_* flags. */
{ .name = "invalid how.resolve and O_RDONLY",
--
2.24.0
^ permalink raw reply related
* [PATCH v2 2/2] uapi: split openat2(2) definitions from fcntl.h
From: Aleksa Sarai @ 2019-12-20 14:03 UTC (permalink / raw)
To: Alexander Viro, Jeff Layton, J. Bruce Fields, Shuah Khan
Cc: Aleksa Sarai, Florian Weimer, David Laight, Christian Brauner,
dev, containers, libc-alpha, linux-api, linux-fsdevel,
linux-kernel, linux-kselftest
In-Reply-To: <20191220140328.20907-1-cyphar@cyphar.com>
Florian mentioned that glibc doesn't use fcntl.h because it has some
issues with namespace cleanliness, and that we should have a separate
header for openat2(2) if possible. In addition, userspace has no real
use for the OPEN_HOW_SIZE_* constants so move them to the in-kernel
headers.
Suggested-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
MAINTAINERS | 1 +
include/linux/fcntl.h | 4 ++++
include/uapi/linux/fcntl.h | 36 +--------------------------------
include/uapi/linux/openat2.h | 39 ++++++++++++++++++++++++++++++++++++
4 files changed, 45 insertions(+), 35 deletions(-)
create mode 100644 include/uapi/linux/openat2.h
diff --git a/MAINTAINERS b/MAINTAINERS
index bd5847e802de..737ada377ac3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6397,6 +6397,7 @@ F: fs/*
F: include/linux/fs.h
F: include/linux/fs_types.h
F: include/uapi/linux/fs.h
+F: include/uapi/linux/openat2.h
FINTEK F75375S HARDWARE MONITOR AND FAN CONTROLLER DRIVER
M: Riku Voipio <riku.voipio@iki.fi>
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index f2eb05bd3af3..7bcdcf4f6ab2 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -21,6 +21,10 @@
(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
RESOLVE_BENEATH | RESOLVE_IN_ROOT)
+/* List of all open_how "versions". */
+#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
+#define OPEN_HOW_SIZE_LATEST OPEN_HOW_SIZE_VER0
+
#ifndef force_o_largefile
#define force_o_largefile() (!IS_ENABLED(CONFIG_ARCH_32BIT_OFF_T))
#endif
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 5aaadfd79dd5..ca88b7bce553 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -3,6 +3,7 @@
#define _UAPI_LINUX_FCNTL_H
#include <asm/fcntl.h>
+#include <linux/openat2.h>
#define F_SETLEASE (F_LINUX_SPECIFIC_BASE + 0)
#define F_GETLEASE (F_LINUX_SPECIFIC_BASE + 1)
@@ -100,39 +101,4 @@
#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
-/*
- * Arguments for how openat2(2) should open the target path. If only @flags and
- * @mode are non-zero, then openat2(2) operates very similarly to openat(2).
- *
- * However, unlike openat(2), unknown or invalid bits in @flags result in
- * -EINVAL rather than being silently ignored. @mode must be zero unless one of
- * {O_CREAT, O_TMPFILE} are set.
- *
- * @flags: O_* flags.
- * @mode: O_CREAT/O_TMPFILE file mode.
- * @resolve: RESOLVE_* flags.
- */
-struct open_how {
- __u64 flags;
- __u64 mode;
- __u64 resolve;
-};
-
-#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
-#define OPEN_HOW_SIZE_LATEST OPEN_HOW_SIZE_VER0
-
-/* how->resolve flags for openat2(2). */
-#define RESOLVE_NO_XDEV 0x01 /* Block mount-point crossings
- (includes bind-mounts). */
-#define RESOLVE_NO_MAGICLINKS 0x02 /* Block traversal through procfs-style
- "magic-links". */
-#define RESOLVE_NO_SYMLINKS 0x04 /* Block traversal through all symlinks
- (implies OEXT_NO_MAGICLINKS) */
-#define RESOLVE_BENEATH 0x08 /* Block "lexical" trickery like
- "..", symlinks, and absolute
- paths which escape the dirfd. */
-#define RESOLVE_IN_ROOT 0x10 /* Make all jumps to "/" and ".."
- be scoped inside the dirfd
- (similar to chroot(2)). */
-
#endif /* _UAPI_LINUX_FCNTL_H */
diff --git a/include/uapi/linux/openat2.h b/include/uapi/linux/openat2.h
new file mode 100644
index 000000000000..58b1eb711360
--- /dev/null
+++ b/include/uapi/linux/openat2.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_OPENAT2_H
+#define _UAPI_LINUX_OPENAT2_H
+
+#include <linux/types.h>
+
+/*
+ * Arguments for how openat2(2) should open the target path. If only @flags and
+ * @mode are non-zero, then openat2(2) operates very similarly to openat(2).
+ *
+ * However, unlike openat(2), unknown or invalid bits in @flags result in
+ * -EINVAL rather than being silently ignored. @mode must be zero unless one of
+ * {O_CREAT, O_TMPFILE} are set.
+ *
+ * @flags: O_* flags.
+ * @mode: O_CREAT/O_TMPFILE file mode.
+ * @resolve: RESOLVE_* flags.
+ */
+struct open_how {
+ __u64 flags;
+ __u64 mode;
+ __u64 resolve;
+};
+
+/* how->resolve flags for openat2(2). */
+#define RESOLVE_NO_XDEV 0x01 /* Block mount-point crossings
+ (includes bind-mounts). */
+#define RESOLVE_NO_MAGICLINKS 0x02 /* Block traversal through procfs-style
+ "magic-links". */
+#define RESOLVE_NO_SYMLINKS 0x04 /* Block traversal through all symlinks
+ (implies OEXT_NO_MAGICLINKS) */
+#define RESOLVE_BENEATH 0x08 /* Block "lexical" trickery like
+ "..", symlinks, and absolute
+ paths which escape the dirfd. */
+#define RESOLVE_IN_ROOT 0x10 /* Make all jumps to "/" and ".."
+ be scoped inside the dirfd
+ (similar to chroot(2)). */
+
+#endif /* _UAPI_LINUX_OPENAT2_H */
--
2.24.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] openat2: drop open_how->__padding field
From: Christian Brauner @ 2019-12-20 14:33 UTC (permalink / raw)
To: containers, Aleksa Sarai, Alexander Viro, Jeff Layton,
J. Bruce Fields, Shuah Khan, Arnd Bergmann
Cc: libc-alpha, linux-api, linux-kernel, dev, David Laight,
linux-kselftest, linux-fsdevel
In-Reply-To: <20191220140328.20907-2-cyphar@cyphar.com>
[Cc Arnd for struct layout sanity checking]
On December 20, 2019 3:03:27 PM GMT+01:00, Aleksa Sarai <cyphar@cyphar.com> wrote:
>The purpose of explicit padding was to allow us to use the space in the
>future (C provides no guarantee about the value of padding bytes and
>thus userspace could've provided garbage).
>
>However, the downside of explicit padding is that any extension we wish
>to add should fit the space exactly (otherwise we may end up with a u16
>which will never be used). In addition, the correct error to return for
>non-zero padding is not clear (-EINVAL doesn't imply "you're using an
>extension field unsupported by this kernel", but -E2BIG seems a bit odd
>if the structure size isn't different).
>
>The simplest solution is to just match the design of clone3(2) -- use
>u64s for all fields. The extra few-bytes cost of extra fields is not
>significant (it's unlikely configuration structs will ever be extremely
>large) and it allows for more flag space if necessary. There is also no
>need to align the u64s because we will not permit any padding in the
>structure.
>
>As openat2(2) is not yet in Linus's tree, we can iron out these minor
>warts before we commit to this as a stable ABI.
>
>Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
>Suggested-by: David Laight <david.laight@aculab.com>
>Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
>---
> fs/open.c | 2 --
> include/uapi/linux/fcntl.h | 17 +++++++------
> tools/testing/selftests/openat2/helpers.h | 7 +++---
> .../testing/selftests/openat2/openat2_test.c | 24 +++++++------------
> 4 files changed, 19 insertions(+), 31 deletions(-)
>
>diff --git a/fs/open.c b/fs/open.c
>index 50a46501bcc9..8cdb2b675867 100644
>--- a/fs/open.c
>+++ b/fs/open.c
>@@ -993,8 +993,6 @@ static inline int build_open_flags(const struct
>open_how *how,
> return -EINVAL;
> if (how->resolve & ~VALID_RESOLVE_FLAGS)
> return -EINVAL;
>- if (memchr_inv(how->__padding, 0, sizeof(how->__padding)))
>- return -EINVAL;
>
> /* Deal with the mode. */
> if (WILL_CREATE(flags)) {
>diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
>index d886bdb585e4..5aaadfd79dd5 100644
>--- a/include/uapi/linux/fcntl.h
>+++ b/include/uapi/linux/fcntl.h
>@@ -101,22 +101,21 @@
> #define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
>
> /*
>- * Arguments for how openat2(2) should open the target path. If
>@resolve is
>- * zero, then openat2(2) operates very similarly to openat(2).
>+ * Arguments for how openat2(2) should open the target path. If only
>@flags and
>+ * @mode are non-zero, then openat2(2) operates very similarly to
>openat(2).
> *
>- * However, unlike openat(2), unknown bits in @flags result in -EINVAL
>rather
>- * than being silently ignored. @mode must be zero unless one of
>{O_CREAT,
>- * O_TMPFILE} are set.
>+ * However, unlike openat(2), unknown or invalid bits in @flags result
>in
>+ * -EINVAL rather than being silently ignored. @mode must be zero
>unless one of
>+ * {O_CREAT, O_TMPFILE} are set.
> *
> * @flags: O_* flags.
> * @mode: O_CREAT/O_TMPFILE file mode.
> * @resolve: RESOLVE_* flags.
> */
> struct open_how {
>- __aligned_u64 flags;
>- __u16 mode;
>- __u16 __padding[3]; /* must be zeroed */
>- __aligned_u64 resolve;
>+ __u64 flags;
>+ __u64 mode;
>+ __u64 resolve;
> };
>
> #define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
>diff --git a/tools/testing/selftests/openat2/helpers.h
>b/tools/testing/selftests/openat2/helpers.h
>index 43ca5ceab6e3..a6ea27344db2 100644
>--- a/tools/testing/selftests/openat2/helpers.h
>+++ b/tools/testing/selftests/openat2/helpers.h
>@@ -36,10 +36,9 @@
> * @resolve: RESOLVE_* flags.
> */
> struct open_how {
>- __aligned_u64 flags;
>- __u16 mode;
>- __u16 __padding[3]; /* must be zeroed */
>- __aligned_u64 resolve;
>+ __u64 flags;
>+ __u64 mode;
>+ __u64 resolve;
> };
>
> #define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */
>diff --git a/tools/testing/selftests/openat2/openat2_test.c
>b/tools/testing/selftests/openat2/openat2_test.c
>index 0b64fedc008b..b386367c606b 100644
>--- a/tools/testing/selftests/openat2/openat2_test.c
>+++ b/tools/testing/selftests/openat2/openat2_test.c
>@@ -40,7 +40,7 @@ struct struct_test {
> int err;
> };
>
>-#define NUM_OPENAT2_STRUCT_TESTS 10
>+#define NUM_OPENAT2_STRUCT_TESTS 7
> #define NUM_OPENAT2_STRUCT_VARIATIONS 13
>
> void test_openat2_struct(void)
>@@ -57,20 +57,6 @@ void test_openat2_struct(void)
> .arg.inner.flags = O_RDONLY,
> .size = sizeof(struct open_how_ext) },
>
>- /* Normal struct with broken padding. */
>- { .name = "normal struct (non-zero padding[0])",
>- .arg.inner.flags = O_RDONLY,
>- .arg.inner.__padding = {0xa0, 0x00, 0x00},
>- .size = sizeof(struct open_how_ext), .err = -EINVAL },
>- { .name = "normal struct (non-zero padding[1])",
>- .arg.inner.flags = O_RDONLY,
>- .arg.inner.__padding = {0x00, 0x1a, 0x00},
>- .size = sizeof(struct open_how_ext), .err = -EINVAL },
>- { .name = "normal struct (non-zero padding[2])",
>- .arg.inner.flags = O_RDONLY,
>- .arg.inner.__padding = {0x00, 0x00, 0xef},
>- .size = sizeof(struct open_how_ext), .err = -EINVAL },
>-
> /* TODO: Once expanded, check zero-padding. */
>
> /* Smaller than version-0 struct. */
>@@ -169,7 +155,7 @@ struct flag_test {
> int err;
> };
>
>-#define NUM_OPENAT2_FLAG_TESTS 21
>+#define NUM_OPENAT2_FLAG_TESTS 23
>
> void test_openat2_flags(void)
> {
>@@ -214,9 +200,15 @@ void test_openat2_flags(void)
> { .name = "invalid how.mode and O_CREAT",
> .how.flags = O_CREAT,
> .how.mode = 0xFFFF, .err = -EINVAL },
>+ { .name = "invalid (very large) how.mode and O_CREAT",
>+ .how.flags = O_CREAT,
>+ .how.mode = 0xC000000000000000ULL, .err = -EINVAL },
> { .name = "invalid how.mode and O_TMPFILE",
> .how.flags = O_TMPFILE | O_RDWR,
> .how.mode = 0x1337, .err = -EINVAL },
>+ { .name = "invalid (very large) how.mode and O_TMPFILE",
>+ .how.flags = O_TMPFILE | O_RDWR,
>+ .how.mode = 0x0000A00000000000ULL, .err = -EINVAL },
>
> /* ->resolve must only contain RESOLVE_* flags. */
> { .name = "invalid how.resolve and O_RDONLY",
^ permalink raw reply
* Re: [PATCH for 5.5 0/3] Restartable Sequences Fixes
From: Peter Zijlstra @ 2019-12-20 14:48 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, linux-kernel, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable
In-Reply-To: <20191211161713.4490-1-mathieu.desnoyers@efficios.com>
On Wed, Dec 11, 2019 at 11:17:10AM -0500, Mathieu Desnoyers wrote:
> Hi,
>
> Here is a repost of a small set of rseq fixes which was initially posted
> in September 2019. It now targets kernel 5.5. Those should be backported
> to stable kernels >= 4.18.
>
> Thanks,
>
> Mathieu
>
> Mathieu Desnoyers (3):
> rseq: Fix: Reject unknown flags on rseq unregister
> rseq: Fix: Unregister rseq for clone CLONE_VM
> rseq/selftests: Fix: Namespace gettid() for compatibility with glibc
> 2.30
I've picked up the first two patches, thanks!
^ permalink raw reply
* [PATCH for 5.5 1/2] rseq: Fix: Clarify rseq.h UAPI rseq_cs memory reclaim requirements
From: Mathieu Desnoyers @ 2019-12-20 20:12 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable,
Mathieu Desnoyers, Florian Weimer, Dmitry Vyukov, Neel Natu
The rseq.h UAPI documents that the rseq_cs field must be cleared
before reclaiming memory that contains the targeted struct rseq_cs.
We should extend this comment to also dictate that the rseq_cs field
must be cleared before reclaiming memory of the code pointed to by
the rseq_cs start_ip and post_commit_offset fields.
While we can expect that use of dlclose(3) will typically unmap
both struct rseq_cs and its associated code at once, nothing would
theoretically prevent a JIT from reclaiming the code without
reclaiming the struct rseq_cs, which would erroneously allow the
kernel to consider new code which is not a rseq critical section
as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer <fw@deneb.enyo.de>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Florian Weimer <fw@deneb.enyo.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Paul Turner <pjt@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Neel Natu <neelnatu@google.com>
Cc: linux-api@vger.kernel.org
---
include/uapi/linux/rseq.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index 9a402fdb60e9..6f26b0b148a6 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -100,7 +100,9 @@ struct rseq {
* instruction sequence block, as well as when the kernel detects that
* it is preempting or delivering a signal outside of the range
* targeted by the rseq_cs. Also needs to be set to NULL by user-space
- * before reclaiming memory that contains the targeted struct rseq_cs.
+ * before reclaiming memory that contains the targeted struct rseq_cs
+ * or reclaiming memory that contains the code refered to by the
+ * start_ip and post_commit_offset fields of struct rseq_cs.
*
* Read and set by the kernel. Set by user-space with single-copy
* atomicity semantics. This field should only be updated by the
--
2.17.1
^ permalink raw reply related
* [PATCH for 5.5 2/2] rseq/selftests: Clarify rseq_prepare_unload() helper requirements
From: Mathieu Desnoyers @ 2019-12-20 20:12 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable,
Mathieu Desnoyers, Shuah Khan, Florian Weimer, Dmitry Vyukov
In-Reply-To: <20191220201207.17389-1-mathieu.desnoyers@efficios.com>
The rseq.h UAPI now documents that the rseq_cs field must be cleared
before reclaiming memory that contains the targeted struct rseq_cs, but
also that the rseq_cs field must be cleared before reclaiming memory of
the code pointed to by the rseq_cs start_ip and post_commit_offset
fields.
While we can expect that use of dlclose(3) will typically unmap
both struct rseq_cs and its associated code at once, nothing would
theoretically prevent a JIT from reclaiming the code without
reclaiming the struct rseq_cs, which would erroneously allow the
kernel to consider new code which is not a rseq critical section
as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer <fw@deneb.enyo.de>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: Florian Weimer <fw@deneb.enyo.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Paul Turner <pjt@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
---
tools/testing/selftests/rseq/rseq.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index d40d60e7499e..15cbd51d0818 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -149,11 +149,13 @@ static inline void rseq_clear_rseq_cs(void)
/*
* rseq_prepare_unload() should be invoked by each thread executing a rseq
* critical section at least once between their last critical section and
- * library unload of the library defining the rseq critical section
- * (struct rseq_cs). This also applies to use of rseq in code generated by
- * JIT: rseq_prepare_unload() should be invoked at least once by each
- * thread executing a rseq critical section before reclaim of the memory
- * holding the struct rseq_cs.
+ * library unload of the library defining the rseq critical section (struct
+ * rseq_cs) or the code refered to by the struct rseq_cs start_ip and
+ * post_commit_offset fields. This also applies to use of rseq in code
+ * generated by JIT: rseq_prepare_unload() should be invoked at least once by
+ * each thread executing a rseq critical section before reclaim of the memory
+ * holding the struct rseq_cs or reclaim of the code pointed to by struct
+ * rseq_cs start_ip and post_commit_offset fields.
*/
static inline void rseq_prepare_unload(void)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH for 5.5 2/2] rseq/selftests: Clarify rseq_prepare_unload() helper requirements
From: Shuah Khan @ 2019-12-20 20:27 UTC (permalink / raw)
To: Mathieu Desnoyers, Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable, Florian Weimer,
Dmitry Vyukov, skh >> Shuah Khan
In-Reply-To: <20191220201207.17389-2-mathieu.desnoyers@efficios.com>
Hi Mathieu,
On 12/20/19 1:12 PM, Mathieu Desnoyers wrote:
> The rseq.h UAPI now documents that the rseq_cs field must be cleared
> before reclaiming memory that contains the targeted struct rseq_cs, but
> also that the rseq_cs field must be cleared before reclaiming memory of
> the code pointed to by the rseq_cs start_ip and post_commit_offset
> fields.
>
> While we can expect that use of dlclose(3) will typically unmap
> both struct rseq_cs and its associated code at once, nothing would
> theoretically prevent a JIT from reclaiming the code without
> reclaiming the struct rseq_cs, which would erroneously allow the
> kernel to consider new code which is not a rseq critical section
> as a rseq critical section following a code reclaim.
>
> Suggested-by: Florian Weimer <fw@deneb.enyo.de>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Florian Weimer <fw@deneb.enyo.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: "H . Peter Anvin" <hpa@zytor.com>
> Cc: Paul Turner <pjt@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> ---
> tools/testing/selftests/rseq/rseq.h | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
> index d40d60e7499e..15cbd51d0818 100644
> --- a/tools/testing/selftests/rseq/rseq.h
> +++ b/tools/testing/selftests/rseq/rseq.h
> @@ -149,11 +149,13 @@ static inline void rseq_clear_rseq_cs(void)
> /*
> * rseq_prepare_unload() should be invoked by each thread executing a rseq
> * critical section at least once between their last critical section and
> - * library unload of the library defining the rseq critical section
> - * (struct rseq_cs). This also applies to use of rseq in code generated by
> - * JIT: rseq_prepare_unload() should be invoked at least once by each
> - * thread executing a rseq critical section before reclaim of the memory
> - * holding the struct rseq_cs.
> + * library unload of the library defining the rseq critical section (struct
> + * rseq_cs) or the code refered to by the struct rseq_cs start_ip and
Nit: referred instead of refered
> + * post_commit_offset fields. This also applies to use of rseq in code
> + * generated by JIT: rseq_prepare_unload() should be invoked at least once by
> + * each thread executing a rseq critical section before reclaim of the memory
> + * holding the struct rseq_cs or reclaim of the code pointed to by struct
> + * rseq_cs start_ip and post_commit_offset fields.
> */
> static inline void rseq_prepare_unload(void)
> {
>
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH for 5.5 2/2] rseq/selftests: Clarify rseq_prepare_unload() helper requirements
From: Mathieu Desnoyers @ 2019-12-20 20:32 UTC (permalink / raw)
To: Shuah Khan
Cc: Thomas Gleixner, linux-kernel, Peter Zijlstra, paulmck,
Boqun Feng, H. Peter Anvin, Paul Turner, linux-api, stable,
Florian Weimer, Dmitry Vyukov
In-Reply-To: <2ad7d561-2cbc-09c2-2806-97c3be3727e2@linuxfoundation.org>
----- On Dec 20, 2019, at 3:27 PM, Shuah Khan skhan@linuxfoundation.org wrote:
> Hi Mathieu,
>
> On 12/20/19 1:12 PM, Mathieu Desnoyers wrote:
>> The rseq.h UAPI now documents that the rseq_cs field must be cleared
>> before reclaiming memory that contains the targeted struct rseq_cs, but
>> also that the rseq_cs field must be cleared before reclaiming memory of
>> the code pointed to by the rseq_cs start_ip and post_commit_offset
>> fields.
>>
>> While we can expect that use of dlclose(3) will typically unmap
>> both struct rseq_cs and its associated code at once, nothing would
>> theoretically prevent a JIT from reclaiming the code without
>> reclaiming the struct rseq_cs, which would erroneously allow the
>> kernel to consider new code which is not a rseq critical section
>> as a rseq critical section following a code reclaim.
>>
>> Suggested-by: Florian Weimer <fw@deneb.enyo.de>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Shuah Khan <skhan@linuxfoundation.org>
>> Cc: Florian Weimer <fw@deneb.enyo.de>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
>> Cc: Boqun Feng <boqun.feng@gmail.com>
>> Cc: "H . Peter Anvin" <hpa@zytor.com>
>> Cc: Paul Turner <pjt@google.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> ---
>> tools/testing/selftests/rseq/rseq.h | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/testing/selftests/rseq/rseq.h
>> b/tools/testing/selftests/rseq/rseq.h
>> index d40d60e7499e..15cbd51d0818 100644
>> --- a/tools/testing/selftests/rseq/rseq.h
>> +++ b/tools/testing/selftests/rseq/rseq.h
>> @@ -149,11 +149,13 @@ static inline void rseq_clear_rseq_cs(void)
>> /*
>> * rseq_prepare_unload() should be invoked by each thread executing a rseq
>> * critical section at least once between their last critical section and
>> - * library unload of the library defining the rseq critical section
>> - * (struct rseq_cs). This also applies to use of rseq in code generated by
>> - * JIT: rseq_prepare_unload() should be invoked at least once by each
>> - * thread executing a rseq critical section before reclaim of the memory
>> - * holding the struct rseq_cs.
>> + * library unload of the library defining the rseq critical section (struct
>> + * rseq_cs) or the code refered to by the struct rseq_cs start_ip and
>
> Nit: referred instead of refered
Good catch. I've done the same error in patch 1/2. I'll update both and
resend.
Thanks!
Mathieu
>
>> + * post_commit_offset fields. This also applies to use of rseq in code
>> + * generated by JIT: rseq_prepare_unload() should be invoked at least once by
>> + * each thread executing a rseq critical section before reclaim of the memory
>> + * holding the struct rseq_cs or reclaim of the code pointed to by struct
>> + * rseq_cs start_ip and post_commit_offset fields.
>> */
>> static inline void rseq_prepare_unload(void)
>> {
>>
>
> thanks,
> -- Shuah
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH for 5.5 1/2 v2] rseq: Fix: Clarify rseq.h UAPI rseq_cs memory reclaim requirements
From: Mathieu Desnoyers @ 2019-12-20 20:33 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable,
Mathieu Desnoyers, Florian Weimer, Dmitry Vyukov, Neel Natu
The rseq.h UAPI documents that the rseq_cs field must be cleared
before reclaiming memory that contains the targeted struct rseq_cs.
We should extend this comment to also dictate that the rseq_cs field
must be cleared before reclaiming memory of the code pointed to by
the rseq_cs start_ip and post_commit_offset fields.
While we can expect that use of dlclose(3) will typically unmap
both struct rseq_cs and its associated code at once, nothing would
theoretically prevent a JIT from reclaiming the code without
reclaiming the struct rseq_cs, which would erroneously allow the
kernel to consider new code which is not a rseq critical section
as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer <fw@deneb.enyo.de>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Florian Weimer <fw@deneb.enyo.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Paul Turner <pjt@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Neel Natu <neelnatu@google.com>
Cc: linux-api@vger.kernel.org
---
include/uapi/linux/rseq.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index 9a402fdb60e9..d94afdfc4b7c 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -100,7 +100,9 @@ struct rseq {
* instruction sequence block, as well as when the kernel detects that
* it is preempting or delivering a signal outside of the range
* targeted by the rseq_cs. Also needs to be set to NULL by user-space
- * before reclaiming memory that contains the targeted struct rseq_cs.
+ * before reclaiming memory that contains the targeted struct rseq_cs
+ * or reclaiming memory that contains the code referred to by the
+ * start_ip and post_commit_offset fields of struct rseq_cs.
*
* Read and set by the kernel. Set by user-space with single-copy
* atomicity semantics. This field should only be updated by the
--
2.17.1
^ permalink raw reply related
* [PATCH for 5.5 2/2 v2] rseq/selftests: Clarify rseq_prepare_unload() helper requirements
From: Mathieu Desnoyers @ 2019-12-20 20:33 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, stable,
Mathieu Desnoyers, Shuah Khan, Florian Weimer, Dmitry Vyukov
In-Reply-To: <20191220203318.18739-1-mathieu.desnoyers@efficios.com>
The rseq.h UAPI now documents that the rseq_cs field must be cleared
before reclaiming memory that contains the targeted struct rseq_cs, but
also that the rseq_cs field must be cleared before reclaiming memory of
the code pointed to by the rseq_cs start_ip and post_commit_offset
fields.
While we can expect that use of dlclose(3) will typically unmap
both struct rseq_cs and its associated code at once, nothing would
theoretically prevent a JIT from reclaiming the code without
reclaiming the struct rseq_cs, which would erroneously allow the
kernel to consider new code which is not a rseq critical section
as a rseq critical section following a code reclaim.
Suggested-by: Florian Weimer <fw@deneb.enyo.de>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: Florian Weimer <fw@deneb.enyo.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Paul Turner <pjt@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
---
tools/testing/selftests/rseq/rseq.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index d40d60e7499e..3f63eb362b92 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -149,11 +149,13 @@ static inline void rseq_clear_rseq_cs(void)
/*
* rseq_prepare_unload() should be invoked by each thread executing a rseq
* critical section at least once between their last critical section and
- * library unload of the library defining the rseq critical section
- * (struct rseq_cs). This also applies to use of rseq in code generated by
- * JIT: rseq_prepare_unload() should be invoked at least once by each
- * thread executing a rseq critical section before reclaim of the memory
- * holding the struct rseq_cs.
+ * library unload of the library defining the rseq critical section (struct
+ * rseq_cs) or the code referred to by the struct rseq_cs start_ip and
+ * post_commit_offset fields. This also applies to use of rseq in code
+ * generated by JIT: rseq_prepare_unload() should be invoked at least once by
+ * each thread executing a rseq critical section before reclaim of the memory
+ * holding the struct rseq_cs or reclaim of the code pointed to by struct
+ * rseq_cs start_ip and post_commit_offset fields.
*/
static inline void rseq_prepare_unload(void)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 2/3] clone3: allow spawning processes into cgroups
From: Oleg Nesterov @ 2019-12-20 20:36 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-api, linux-kernel, Tejun Heo, Ingo Molnar, Johannes Weiner,
Li Zefan, Peter Zijlstra, cgroups
In-Reply-To: <20191218173516.7875-3-christian.brauner@ubuntu.com>
On 12/18, Christian Brauner wrote:
>
> This adds support for creating a process in a different cgroup than its
> parent.
Cough... I will not comment the intent ;) I can't review the cgroup patches
anyway.
However,
> +int cgroup_lock_fork(struct kernel_clone_args *kargs)
> + __acquires(&cgroup_mutex)
> +{
> + struct cgroup *cgrp;
> +
> + if (!(kargs->flags & CLONE_INTO_CGROUP))
> + return 0;
> +
> + cgrp = kargs->cgrp;
> + if (!cgrp)
> + return 0;
> +
> + mutex_lock(&cgroup_mutex);
> +
> + if (!cgroup_is_dead(cgrp))
> + return 0;
> +
> + mutex_unlock(&cgroup_mutex);
> + return -ENODEV;
...
> @@ -2172,7 +2172,7 @@ static __latent_entropy struct task_struct *copy_process(
> * between here and cgroup_post_fork() if an organisation operation is in
> * progress.
> */
> - retval = cgroup_can_fork(p);
> + retval = cgroup_can_fork(current, p, args);
> if (retval)
> goto bad_fork_cgroup_threadgroup_change_end;
>
> @@ -2226,6 +2226,10 @@ static __latent_entropy struct task_struct *copy_process(
> goto bad_fork_cancel_cgroup;
> }
>
> + retval = cgroup_lock_fork(args);
mutex_lock() under spin_lock() ??
just in case, note that mutex_lock(&cgroup_mutex) is not safe even under
cgroup_threadgroup_change_begin(), this can deadlock.
Oleg.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox