* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Peter Zijlstra @ 2024-04-26 10:14 UTC (permalink / raw)
To: Florian Weimer
Cc: André Almeida, Mathieu Desnoyers, Thomas Gleixner,
linux-kernel, Paul E . McKenney, Boqun Feng, H . Peter Anvin,
Paul Turner, linux-api, Christian Brauner, David.Laight, carlos,
Peter Oskolkov, Alexander Mikhalitsyn, Chris Kennelly,
Ingo Molnar, Darren Hart, Davidlohr Bueso, libc-alpha,
Steven Rostedt, Jonathan Corbet, Noah Goldstein,
Daniel Colascione, longman, kernel-dev
In-Reply-To: <875xw44fxk.fsf@oldenburg.str.redhat.com>
On Fri, Apr 26, 2024 at 11:43:51AM +0200, Florian Weimer wrote:
> * André Almeida:
>
> > With FUTEX2_SPIN flag set during a futex_wait(), the futex value is
> > expected to be the PID of the lock owner. Then, the kernel gets the
> > task_struct of the corresponding PID, and checks if it's running. It
> > spins until the futex is awaken, the task is scheduled out or if a
> > timeout happens. If the lock owner is scheduled out at any time, then
> > the syscall follows the normal path of sleeping as usual.
>
> PID or TID?
TID, just like PI_LOCK I would presume.
> I think we'd like to have at least one, possibly more, bits for free
> use, so the kernel ID comparison should at least mask off the MSB,
> possibly more.
Yeah, it should be using FUTEX_TID_MASK -- just like PI_LOCK :-)
I suppose the question is if this thing should then also imply
FUTEX_WAITERS or not.
^ permalink raw reply
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Christian Brauner @ 2024-04-26 10:26 UTC (permalink / raw)
To: André Almeida
Cc: Mathieu Desnoyers, Peter Zijlstra, Thomas Gleixner, linux-kernel,
Paul E . McKenney, Boqun Feng, H . Peter Anvin, Paul Turner,
linux-api, Florian Weimer, David.Laight, carlos, Peter Oskolkov,
Alexander Mikhalitsyn, Chris Kennelly, Ingo Molnar, Darren Hart,
Davidlohr Bueso, libc-alpha, Steven Rostedt, Jonathan Corbet,
Noah Goldstein, Daniel Colascione, longman, kernel-dev
In-Reply-To: <20240425204332.221162-1-andrealmeid@igalia.com>
On Thu, Apr 25, 2024 at 05:43:31PM -0300, André Almeida wrote:
> Hi,
>
> In the last LPC, Mathieu Desnoyers and I presented[0] a proposal to extend the
> rseq interface to be able to implement spin locks in userspace correctly. Thomas
> Gleixner agreed that this is something that Linux could improve, but asked for
> an alternative proposal first: a futex operation that allows to spin a user
> lock inside the kernel. This patchset implements a prototype of this idea for
> further discussion.
>
> With FUTEX2_SPIN flag set during a futex_wait(), the futex value is expected to
> be the PID of the lock owner. Then, the kernel gets the task_struct of the
> corresponding PID, and checks if it's running. It spins until the futex
> is awaken, the task is scheduled out or if a timeout happens. If the lock owner
> is scheduled out at any time, then the syscall follows the normal path of
> sleeping as usual.
>
> If the futex is awaken and we are spinning, we can return to userspace quickly,
> avoid the scheduling out and in again to wake from a futex_wait(), thus
> speeding up the wait operation.
>
> I didn't manage to find a good mechanism to prevent race conditions between
> setting *futex = PID in userspace and doing find_get_task_by_vpid(PID) in kernel
> space, giving that there's enough room for the original PID owner exit and such
> PID to be relocated to another unrelated task in the system. I didn't performed
One option would be to also allow pidfds. Starting with v6.9 they can be
used to reference individual threads.
So for the really fast case where you have multiple threads and you
somehow may really do care about the impact of the atomic_long_inc() on
pidfd_file->f_count during fdget() (for the single-threaded case the
increment is elided), callers can pass the TID. But in cases where the
inc and put aren't a performance sensitive, you can use pidfds.
So something like the _completely untested_ below:
diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
index 94feac92cf4f..b842680aa7e0 100644
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -4,6 +4,9 @@
#include <linux/sched/task.h>
#include <linux/sched/signal.h>
#include <linux/freezer.h>
+#include <linux/cleanup.h>
+#include <linux/file.h>
+#include <uapi/linux/pidfd.h>
#include "futex.h"
@@ -385,19 +388,29 @@ static int futex_spin(struct futex_hash_bucket *hb, struct futex_q *q,
struct hrtimer_sleeper *timeout, void __user *uaddr, u32 val)
{
struct task_struct *p;
- u32 pid, uval;
+ struct pid *pid;
+ u32 pidfd, uval;
unsigned int i = 0;
if (futex_get_value_locked(&uval, uaddr))
return -EFAULT;
- pid = uval;
+ pidfd = uval;
+ CLASS(fd, f)(pidfd);
- p = find_get_task_by_vpid(pid);
- if (!p) {
- printk("%s: no task found with PID %d\n", __func__, pid);
- return -EAGAIN;
- }
+ if (!f.file)
+ return -EBADF;
+
+ pid = pidfd_pid(f.file);
+ if (IS_ERR(pid))
+ return PTR_ERR(pid);
+
+ if (f.file->f_flags & PIDFD_THREAD)
+ p = get_pid_task(pid, PIDTYPE_PID); /* individual thread */
+ else
+ p = get_pid_task(pid, PIDTYPE_TGID); /* thread-group leader */
+ if (!p)
+ return -ESRCH;
if (unlikely(p->flags & PF_KTHREAD)) {
put_task_struct(p);
> benchmarks so far, as I hope to clarify if this interface makes sense prior to
> doing measurements on it.
>
> This implementation has some debug prints to make it easy to inspect what the
> kernel is doing, so you can check if the futex woke during spinning or if
> just slept as the normal path:
>
> [ 6331] futex_spin: spinned 64738 times, sleeping
> [ 6331] futex_spin: woke after 1864606 spins
> [ 6332] futex_spin: woke after 1820906 spins
> [ 6351] futex_spin: spinned 1603293 times, sleeping
> [ 6352] futex_spin: woke after 1848199 spins
>
> [0] https://lpc.events/event/17/contributions/1481/
>
> You can find a small snippet to play with this interface here:
>
> ---
>
> /*
> * futex2_spin example, by André Almeida <andrealmeid@igalia.com>
> *
> * gcc spin.c -o spin
> */
>
> #define _GNU_SOURCE
> #include <err.h>
> #include <errno.h>
> #include <linux/futex.h>
> #include <linux/sched.h>
> #include <pthread.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/mman.h>
> #include <unistd.h>
>
> #define __NR_futex_wake 454
> #define __NR_futex_wait 455
>
> #define WAKE_WAIT_US 10000
> #define FUTEX2_SPIN 0x08
> #define STACK_SIZE (1024 * 1024)
>
> #define FUTEX2_SIZE_U32 0x02
> #define FUTEX2_PRIVATE FUTEX_PRIVATE_FLAG
>
> #define timeout_ns 30000000
>
> void *futex;
>
> static inline int futex2_wake(volatile void *uaddr, unsigned long mask, int nr, unsigned int flags)
> {
> return syscall(__NR_futex_wake, uaddr, mask, nr, flags);
> }
>
> static inline int futex2_wait(volatile void *uaddr, unsigned long val, unsigned long mask,
> unsigned int flags, struct timespec *timo, clockid_t clockid)
> {
> return syscall(__NR_futex_wait, uaddr, val, mask, flags, timo, clockid);
> }
>
> void waiter_fn()
> {
> struct timespec to;
> unsigned int flags = FUTEX2_PRIVATE | FUTEX2_SIZE_U32 | FUTEX2_SPIN;
>
> uint32_t child_pid = *(uint32_t *) futex;
>
> clock_gettime(CLOCK_MONOTONIC, &to);
> to.tv_nsec += timeout_ns;
> if (to.tv_nsec >= 1000000000) {
> to.tv_sec++;
> to.tv_nsec -= 1000000000;
> }
>
> printf("waiting on PID %d...\n", child_pid);
> if (futex2_wait(futex, child_pid, ~0U, flags, &to, CLOCK_MONOTONIC))
> printf("waiter failed errno %d\n", errno);
>
> puts("waiting done");
> }
>
> int function(int n)
> {
> return n + n;
> }
>
> #define CHILD_LOOPS 500000
>
> static int child_fn(void *arg)
> {
> int i, n = 2;
>
> for (i = 0; i < CHILD_LOOPS; i++)
> n = function(n);
>
> futex2_wake(futex, ~0U, 1, FUTEX2_SIZE_U32 | FUTEX_PRIVATE_FLAG);
>
> puts("child thread is done");
>
> return 0;
> }
>
> int main() {
> uint32_t child_pid = 0;
> char *stack;
>
> futex = &child_pid;
>
> stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
>
> if (stack == MAP_FAILED)
> err(EXIT_FAILURE, "mmap");
>
> child_pid = clone(child_fn, stack + STACK_SIZE, CLONE_VM, NULL);
>
> waiter_fn();
>
> usleep(WAKE_WAIT_US * 10);
>
> return 0;
> }
>
> ---
>
> André Almeida (1):
> futex: Add FUTEX_SPIN operation
>
> include/uapi/linux/futex.h | 2 +-
> kernel/futex/futex.h | 6 ++-
> kernel/futex/waitwake.c | 79 +++++++++++++++++++++++++++++++++++++-
> 3 files changed, 83 insertions(+), 4 deletions(-)
>
> --
> 2.44.0
>
^ permalink raw reply related
* [PATCH v5 1/3] fs: reorganize path_openat()
From: Stas Sergeev @ 2024-04-26 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240426133310.1159976-1-stsp2@yandex.ru>
This patch moves the call to alloc_empty_file() down the if branches.
That changes is needed for the next patch, which adds a cred override
for alloc_empty_file(). The cred override is only needed in one branch,
i.e. it is not needed for O_PATH and O_TMPFILE..
No functional changes are intended by that patch.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Andy Lutomirski <luto@kernel.org>
CC: David Laight <David.Laight@ACULAB.COM>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
fs/namei.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index c5b2a25be7d0..dd50345f7260 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3781,17 +3781,24 @@ static struct file *path_openat(struct nameidata *nd,
{
struct file *file;
int error;
+ int open_flags = op->open_flag;
- file = alloc_empty_file(op->open_flag, current_cred());
- if (IS_ERR(file))
- return file;
-
- if (unlikely(file->f_flags & __O_TMPFILE)) {
- error = do_tmpfile(nd, flags, op, file);
- } else if (unlikely(file->f_flags & O_PATH)) {
- error = do_o_path(nd, flags, file);
+ if (unlikely(open_flags & (__O_TMPFILE | O_PATH))) {
+ file = alloc_empty_file(open_flags, current_cred());
+ if (IS_ERR(file))
+ return file;
+ if (open_flags & __O_TMPFILE)
+ error = do_tmpfile(nd, flags, op, file);
+ else
+ error = do_o_path(nd, flags, file);
} else {
- const char *s = path_init(nd, flags);
+ const char *s;
+
+ file = alloc_empty_file(open_flags, current_cred());
+ if (IS_ERR(file))
+ return file;
+
+ s = path_init(nd, flags);
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
;
--
2.44.0
^ permalink raw reply related
* [PATCH v5 3/3] openat2: add OA2_CRED_INHERIT flag
From: Stas Sergeev @ 2024-04-26 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240426133310.1159976-1-stsp2@yandex.ru>
This flag performs the open operation with the fs credentials
(fsuid, fsgid, group_info) that were in effect when dir_fd was opened.
dir_fd must be opened with O_CRED_ALLOW flag for this to work.
This allows the process to pre-open some directories and then
change eUID (and all other UIDs/GIDs) to a less-privileged user,
retaining the ability to open/create files within these directories.
Design goal:
The idea is to provide a very light-weight sandboxing, where the
process, without the use of any heavy-weight techniques like chroot
within namespaces, can restrict the access to the set of pre-opened
directories.
This patch is just a first step to such sandboxing. If things go
well, in the future the same extension can be added to more syscalls.
These should include at least unlinkat(), renameat2() and the
not-yet-upstreamed setxattrat().
Security considerations:
- Only the bare minimal set of credentials is overridden:
fsuid, fsgid and group_info. The rest, for example capabilities,
are not overridden to avoid unneeded security risks.
- To avoid sandboxing escape, this patch makes sure the restricted
lookup modes are used. Namely, RESOLVE_BENEATH or RESOLVE_IN_ROOT.
- Magic /proc symlinks are discarded, as suggested by
Andy Lutomirski <luto@kernel.org>
- O_CRED_ALLOW fds cannot be passed via unix socket and are always
closed on exec() to prevent "unsuspecting userspace" from not being
able to fully drop privs.
Use cases:
Virtual machines that deal with untrusted code, can use that
instead of a more heavy-weighted approaches.
Currently the approach is being tested on a dosemu2 VM.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Stefan Metzmacher <metze@samba.org>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Andy Lutomirski <luto@kernel.org>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Aring <alex.aring@gmail.com>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Christian Göttsche <cgzones@googlemail.com>
---
fs/fcntl.c | 2 ++
fs/namei.c | 56 ++++++++++++++++++++++++++++++++++--
fs/open.c | 10 ++++++-
include/linux/fcntl.h | 2 ++
include/uapi/linux/openat2.h | 2 ++
5 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 78c96b1293c2..283c2e65fc2c 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1043,6 +1043,8 @@ static int __init fcntl_init(void)
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
+ BUILD_BUG_ON(HWEIGHT32(VALID_OPENAT2_FLAGS) !=
+ HWEIGHT32(VALID_OPEN_FLAGS) + 1);
fasync_cache = kmem_cache_create("fasync_cache",
sizeof(struct fasync_struct), 0,
diff --git a/fs/namei.c b/fs/namei.c
index dd50345f7260..aa5dcf57851b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3776,6 +3776,43 @@ static int do_o_path(struct nameidata *nd, unsigned flags, struct file *file)
return error;
}
+static const struct cred *openat2_init_creds(int dfd)
+{
+ struct cred *cred;
+ struct fd f;
+
+ if (dfd == AT_FDCWD)
+ return ERR_PTR(-EINVAL);
+
+ f = fdget_raw(dfd);
+ if (!f.file)
+ return ERR_PTR(-EBADF);
+
+ cred = ERR_PTR(-EPERM);
+ if (!(f.file->f_flags & O_CRED_ALLOW))
+ goto done;
+
+ cred = prepare_creds();
+ if (!cred) {
+ cred = ERR_PTR(-ENOMEM);
+ goto done;
+ }
+
+ cred->fsuid = f.file->f_cred->fsuid;
+ cred->fsgid = f.file->f_cred->fsgid;
+ cred->group_info = get_group_info(f.file->f_cred->group_info);
+
+done:
+ fdput(f);
+ return cred;
+}
+
+static void openat2_done_creds(const struct cred *cred)
+{
+ put_group_info(cred->group_info);
+ put_cred(cred);
+}
+
static struct file *path_openat(struct nameidata *nd,
const struct open_flags *op, unsigned flags)
{
@@ -3793,18 +3830,33 @@ static struct file *path_openat(struct nameidata *nd,
error = do_o_path(nd, flags, file);
} else {
const char *s;
+ const struct cred *old_cred = NULL, *cred = NULL;
- file = alloc_empty_file(open_flags, current_cred());
- if (IS_ERR(file))
+ if (open_flags & OA2_CRED_INHERIT) {
+ cred = openat2_init_creds(nd->dfd);
+ if (IS_ERR(cred))
+ return ERR_CAST(cred);
+ }
+ file = alloc_empty_file(open_flags, cred ?: current_cred());
+ if (IS_ERR(file)) {
+ if (cred)
+ openat2_done_creds(cred);
return file;
+ }
s = path_init(nd, flags);
+ if (cred)
+ old_cred = override_creds(cred);
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
;
if (!error)
error = do_open(nd, file, op);
+ if (old_cred)
+ revert_creds(old_cred);
terminate_walk(nd);
+ if (cred)
+ openat2_done_creds(cred);
}
if (likely(!error)) {
if (likely(file->f_mode & FMODE_OPENED))
diff --git a/fs/open.c b/fs/open.c
index ee8460c83c77..dd4fab536135 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1225,7 +1225,7 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
* values before calling build_open_flags(), but openat2(2) checks all
* of its arguments.
*/
- if (flags & ~VALID_OPEN_FLAGS)
+ if (flags & ~VALID_OPENAT2_FLAGS)
return -EINVAL;
if (how->resolve & ~VALID_RESOLVE_FLAGS)
return -EINVAL;
@@ -1326,6 +1326,14 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
lookup_flags |= LOOKUP_CACHED;
}
+ if (flags & OA2_CRED_INHERIT) {
+ /* Inherit creds only with scoped look-up modes. */
+ if (!(lookup_flags & LOOKUP_IS_SCOPED))
+ return -EPERM;
+ /* Reject /proc "magic" links if inheriting creds. */
+ lookup_flags |= LOOKUP_NO_MAGICLINKS;
+ }
+
op->lookup_flags = lookup_flags;
return 0;
}
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index e074ee9c1e36..33b9c7ad056b 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -12,6 +12,8 @@
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_CRED_ALLOW)
+#define VALID_OPENAT2_FLAGS (VALID_OPEN_FLAGS | OA2_CRED_INHERIT)
+
/* List of all valid flags for the how->resolve argument: */
#define VALID_RESOLVE_FLAGS \
(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
diff --git a/include/uapi/linux/openat2.h b/include/uapi/linux/openat2.h
index a5feb7604948..f803558ad62f 100644
--- a/include/uapi/linux/openat2.h
+++ b/include/uapi/linux/openat2.h
@@ -40,4 +40,6 @@ struct open_how {
return -EAGAIN if that's not
possible. */
+#define OA2_CRED_INHERIT (1UL << 28)
+
#endif /* _UAPI_LINUX_OPENAT2_H */
--
2.44.0
^ permalink raw reply related
* [PATCH v5 2/3] open: add O_CRED_ALLOW flag
From: Stas Sergeev @ 2024-04-26 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche, Arnd Bergmann,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jens Axboe, Kuniyuki Iwashima, Pavel Begunkov, linux-arch, netdev
In-Reply-To: <20240426133310.1159976-1-stsp2@yandex.ru>
This flag prevents an fd from being passed via unix socket, and
makes it to be always closed on exec().
It is needed for the subsequent OA2_CRED_INHERIT addition, to work
as an "opt-in" for the new cred-inherit functionality. Without using
O_CRED_ALLOW when opening dir fd, it won't be possible to use
OA2_CRED_INHERIT on that dir fd.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Andy Lutomirski <luto@kernel.org>
CC: David Laight <David.Laight@ACULAB.COM>
CC: Arnd Bergmann <arnd@arndb.de>
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: Kuniyuki Iwashima <kuniyu@amazon.com>
CC: Pavel Begunkov <asml.silence@gmail.com>
CC: linux-arch@vger.kernel.org
CC: netdev@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
fs/fcntl.c | 2 +-
fs/file.c | 15 ++++++++-------
include/linux/fcntl.h | 2 +-
include/uapi/asm-generic/fcntl.h | 4 ++++
net/core/scm.c | 5 +++++
5 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 54cc85d3338e..78c96b1293c2 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1039,7 +1039,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+ BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/file.c b/fs/file.c
index 3b683b9101d8..2a09d5276676 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -827,22 +827,23 @@ void do_close_on_exec(struct files_struct *files)
/* exec unshares first */
spin_lock(&files->file_lock);
for (i = 0; ; i++) {
+ int j;
unsigned long set;
unsigned fd = i * BITS_PER_LONG;
fdt = files_fdtable(files);
if (fd >= fdt->max_fds)
break;
set = fdt->close_on_exec[i];
- if (!set)
- continue;
fdt->close_on_exec[i] = 0;
- for ( ; set ; fd++, set >>= 1) {
- struct file *file;
- if (!(set & 1))
- continue;
- file = fdt->fd[fd];
+ for (j = 0; j < BITS_PER_LONG; j++, fd++, set >>= 1) {
+ struct file *file = fdt->fd[fd];
if (!file)
continue;
+ /* Close all cred-allow files. */
+ if (file->f_flags & O_CRED_ALLOW)
+ set |= 1;
+ if (!(set & 1))
+ continue;
rcu_assign_pointer(fdt->fd[fd], NULL);
__put_unused_fd(files, fd);
spin_unlock(&files->file_lock);
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..e074ee9c1e36 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -10,7 +10,7 @@
(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
O_APPEND | O_NDELAY | O_NONBLOCK | __O_SYNC | O_DSYNC | \
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
- O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+ O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_CRED_ALLOW)
/* List of all valid flags for the how->resolve argument: */
#define VALID_RESOLVE_FLAGS \
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 80f37a0d40d7..ee8c2267c516 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -89,6 +89,10 @@
#define __O_TMPFILE 020000000
#endif
+#ifndef O_CRED_ALLOW
+#define O_CRED_ALLOW 040000000
+#endif
+
/* a horrid kludge trying to make sure that this will fail on old kernels */
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
diff --git a/net/core/scm.c b/net/core/scm.c
index 9cd4b0a01cd6..f54fb0ee9727 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -111,6 +111,11 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct scm_fp_list **fplp)
fput(file);
return -EINVAL;
}
+ /* don't allow files with creds */
+ if (file->f_flags & O_CRED_ALLOW) {
+ fput(file);
+ return -EPERM;
+ }
if (unix_get_socket(file))
fpl->count_unix++;
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag
From: stsp @ 2024-04-26 13:36 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Jan Kara, Jeff Layton, Chuck Lever,
Alexander Aring, David Laight, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240425-akrobatisch-palmen-4c1e7a0f69d2@brauner>
25.04.2024 17:02, Christian Brauner пишет:
>> struct open_flags {
>> - int open_flag;
>> + u64 open_flag;
> Btw, this change taken together with
All fixed in v5.
I dropped u64 use.
Other comments are addressed as well.
Please let me know if I missed some.
Thank you.
^ permalink raw reply
* [PATCH v5 0/3] implement OA2_CRED_INHERIT flag for openat2()
From: Stas Sergeev @ 2024-04-26 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
This patch-set implements the OA2_CRED_INHERIT flag for openat2() syscall.
It is needed to perform an open operation with the creds that were in
effect when the dir_fd was opened, if the dir was opened with O_CRED_ALLOW
flag. This allows the process to pre-open some dirs and switch eUID
(and other UIDs/GIDs) to the less-privileged user, while still retaining
the possibility to open/create files within the pre-opened directory set.
The sand-boxing is security-oriented: symlinks leading outside of a
sand-box are rejected. /proc magic links are rejected. fds opened with
O_CRED_ALLOW are always closed on exec() and cannot be passed via unix
socket.
The more detailed description (including security considerations)
is available in the log messages of individual patches.
Changes in v5:
- rename OA2_INHERIT_CRED to OA2_CRED_INHERIT
- add an "opt-in" flag O_CRED_ALLOW as was suggested by many reviewers
- stop using 64bit types, as suggested by
Christian Brauner <brauner@kernel.org>
- add BUILD_BUG_ON() for VALID_OPENAT2_FLAGS, based on Christian Brauner's
comments
- fixed problems reported by patch-testing bot
- made O_CRED_ALLOW fds not passable via unix sockets and exec(),
based on Christian Brauner's comments
Changes in v4:
- add optimizations suggested by David Laight <David.Laight@ACULAB.COM>
- move security checks to build_open_flags()
- force RESOLVE_NO_MAGICLINKS as suggested by Andy Lutomirski <luto@kernel.org>
Changes in v3:
- partially revert v2 changes to avoid overriding capabilities.
Only the bare minimum is overridden: fsuid, fsgid and group_info.
Document the fact the full cred override is unwanted, as it may
represent an unneeded security risk.
Changes in v2:
- capture full struct cred instead of just fsuid/fsgid.
Suggested by Stefan Metzmacher <metze@samba.org>
CC: Stefan Metzmacher <metze@samba.org>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Andy Lutomirski <luto@kernel.org>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Aring <alex.aring@gmail.com>
CC: David Laight <David.Laight@ACULAB.COM>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Christian Göttsche <cgzones@googlemail.com>
--
2.44.0
^ permalink raw reply
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Dan Williams @ 2024-04-26 16:16 UTC (permalink / raw)
To: Jonathan Cameron, Dan Williams
Cc: linux-cxl, Sreenivas Bagalkote, Brett Henning, Harold Johnson,
Sumanesh Samanta, linux-kernel, Davidlohr Bueso, Dave Jiang,
Alison Schofield, Vishal Verma, Ira Weiny, linuxarm, linux-api,
Lorenzo Pieralisi, Natu, Mahesh, gregkh
In-Reply-To: <20240426094550.00002e37@Huawei.com>
Jonathan Cameron wrote:
[..]
> To give people an incentive to play the standards game we have to
> provide an alternative. Userspace libraries will provide some incentive
> to standardize if we have enough vendors (we don't today - so they will
> do their own libraries), but it is a lot easier to encourage if we
> exercise control over the interface.
Yes, and I expect you and I are not far off on what can be done
here.
However, lets cut to a sentiment hanging over this discussion. Referring
to vendor specific commands:
"CXL spec has them for a reason and they need to be supported."
...that is an aggressive "vendor specific first" sentiment that
generates an aggressive "userspace drivers" reaction, because the best
way to get around community discussions about what ABI makes sense is
userspace drivers.
Now, if we can step back to where this discussion started, where typical
Linux collaboration shines, and where I think you and I are more aligned
than this thread would indicate, is "vendor specific last". Lets
carefully consider the vendor specific commands that are candidates to
be de facto cross vendor semantics if not de jure standards.
^ permalink raw reply
* [PATCH v3 2/2] fs/xattr: add *at family syscalls
From: Christian Göttsche @ 2024-04-26 16:20 UTC (permalink / raw)
To: cgzones
Cc: x86, linux-alpha, linux-kernel, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, audit, linux-arch, linux-api,
linux-security-module, selinux, Richard Henderson,
Ivan Kokshaysky, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, Eric Paris,
Arnd Bergmann, Jens Axboe, Pavel Begunkov, Peter Zijlstra,
Sohil Mehta, Palmer Dabbelt, Miklos Szeredi, Nhat Pham,
Casey Schaufler, Florian Fainelli, Kees Cook, Rick Edgecombe,
Mark Rutland, io-uring
From: Christian Göttsche <cgzones@googlemail.com>
Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
removexattrat(). Those can be used to operate on extended attributes,
especially security related ones, either relative to a pinned directory
or on a file descriptor without read access, avoiding a
/proc/<pid>/fd/<fd> detour, requiring a mounted procfs.
One use case will be setfiles(8) setting SELinux file contexts
("security.selinux") without race conditions and without a file
descriptor opened with read access requiring SELinux read permission.
Use the do_{name}at() pattern from fs/open.c.
Pass the value of the extended attribute, its length, and for
setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
struct xattr_args to not exceed six syscall arguments and not
merging the AT_* and XATTR_* flags.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
CC: x86@kernel.org
CC: linux-alpha@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-ia64@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: linux-mips@vger.kernel.org
CC: linux-parisc@vger.kernel.org
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-s390@vger.kernel.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: audit@vger.kernel.org
CC: linux-arch@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
---
v3:
- pass value, size and xattr_flags via new struct xattr_args to
split AT_* and XATTR_* flags
v2: https://lore.kernel.org/lkml/20230511150802.737477-1-cgzones@googlemail.com/
- squash syscall introduction and wire up commits
- add AT_XATTR_CREATE and AT_XATTR_REPLACE constants
v1 discussion: https://lore.kernel.org/all/20220830152858.14866-2-cgzones@googlemail.com/
Previous approach ("f*xattr: allow O_PATH descriptors"): https://lore.kernel.org/all/20220607153139.35588-1-cgzones@googlemail.com/
---
arch/alpha/kernel/syscalls/syscall.tbl | 4 +
arch/arm/tools/syscall.tbl | 4 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 8 ++
arch/m68k/kernel/syscalls/syscall.tbl | 4 +
arch/microblaze/kernel/syscalls/syscall.tbl | 4 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 4 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 4 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 4 +
arch/parisc/kernel/syscalls/syscall.tbl | 4 +
arch/powerpc/kernel/syscalls/syscall.tbl | 4 +
arch/s390/kernel/syscalls/syscall.tbl | 4 +
arch/sh/kernel/syscalls/syscall.tbl | 4 +
arch/sparc/kernel/syscalls/syscall.tbl | 4 +
arch/x86/entry/syscalls/syscall_32.tbl | 4 +
arch/x86/entry/syscalls/syscall_64.tbl | 4 +
arch/xtensa/kernel/syscalls/syscall.tbl | 4 +
fs/xattr.c | 128 ++++++++++++++++----
include/asm-generic/audit_change_attr.h | 6 +
include/linux/syscalls.h | 10 ++
include/uapi/asm-generic/unistd.h | 12 +-
include/uapi/linux/xattr.h | 6 +
22 files changed, 208 insertions(+), 24 deletions(-)
diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 8ff110826ce2..fdc11249f1b8 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -501,3 +501,7 @@
569 common lsm_get_self_attr sys_lsm_get_self_attr
570 common lsm_set_self_attr sys_lsm_set_self_attr
571 common lsm_list_modules sys_lsm_list_modules
+572 common setxattrat sys_setxattrat
+573 common getxattrat sys_getxattrat
+574 common listxattrat sys_listxattrat
+575 common removexattrat sys_removexattrat
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index b6c9e01e14f5..22fbbcd8e2b5 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -475,3 +475,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 491b2b9bd553..f3a77719eb05 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -39,7 +39,7 @@
#define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5)
#define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800)
-#define __NR_compat_syscalls 462
+#define __NR_compat_syscalls 466
#endif
#define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 7118282d1c79..963c999b8d2e 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -929,6 +929,14 @@ __SYSCALL(__NR_lsm_get_self_attr, sys_lsm_get_self_attr)
__SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+#define __NR_setxattrat 462
+__SYSCALL(__NR_setxattrat, sys_setxattrat)
+#define __NR_getxattrat 463
+__SYSCALL(__NR_getxattrat, sys_getxattrat)
+#define __NR_listxattrat 464
+__SYSCALL(__NR_listxattrat, sys_listxattrat)
+#define __NR_removexattrat 465
+__SYSCALL(__NR_removexattrat, sys_removexattrat)
/*
* Please add new compat syscalls above this comment and update
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 7fd43fd4c9f2..7e8e2d9c3b81 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -461,3 +461,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index b00ab2cabab9..7df7fc7c0528 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -467,3 +467,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 83cfc9eb6b88..07141274f9ff 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -400,3 +400,7 @@
459 n32 lsm_get_self_attr sys_lsm_get_self_attr
460 n32 lsm_set_self_attr sys_lsm_set_self_attr
461 n32 lsm_list_modules sys_lsm_list_modules
+462 n32 setxattrat sys_setxattrat
+463 n32 getxattrat sys_getxattrat
+464 n32 listxattrat sys_listxattrat
+465 n32 removexattrat sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 532b855df589..9773412f2812 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -376,3 +376,7 @@
459 n64 lsm_get_self_attr sys_lsm_get_self_attr
460 n64 lsm_set_self_attr sys_lsm_set_self_attr
461 n64 lsm_list_modules sys_lsm_list_modules
+462 n64 setxattrat sys_setxattrat
+463 n64 getxattrat sys_getxattrat
+464 n64 listxattrat sys_listxattrat
+465 n64 removexattrat sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index f45c9530ea93..8b5fec66ec18 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -449,3 +449,7 @@
459 o32 lsm_get_self_attr sys_lsm_get_self_attr
460 o32 lsm_set_self_attr sys_lsm_set_self_attr
461 o32 lsm_list_modules sys_lsm_list_modules
+462 o32 setxattrat sys_setxattrat
+463 o32 getxattrat sys_getxattrat
+464 o32 listxattrat sys_listxattrat
+465 o32 removexattrat sys_removexattrat
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index b236a84c4e12..b6ebcaadd460 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -460,3 +460,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 17173b82ca21..7e522a720e1a 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -548,3 +548,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 095bb86339a7..71afa1eb35fb 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -464,3 +464,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat sys_removexattrat
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index 86fe269f0220..1fb0ac9f6c58 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -464,3 +464,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index b23d59313589..bdd90010c8d1 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -507,3 +507,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 5f8591ce7f25..779dd1a9835d 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -466,3 +466,7 @@
459 i386 lsm_get_self_attr sys_lsm_get_self_attr
460 i386 lsm_set_self_attr sys_lsm_set_self_attr
461 i386 lsm_list_modules sys_lsm_list_modules
+462 i386 setxattrat sys_setxattrat
+463 i386 getxattrat sys_getxattrat
+464 i386 listxattrat sys_listxattrat
+465 i386 removexattrat sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f4147f..819c90564f82 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -383,6 +383,10 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index dd116598fb25..36bdfe759878 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -432,3 +432,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common setxattrat sys_setxattrat
+463 common getxattrat sys_getxattrat
+464 common listxattrat sys_listxattrat
+465 common removexattrat sys_removexattrat
diff --git a/fs/xattr.c b/fs/xattr.c
index 941aab719da0..d45e83224a7c 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -655,21 +655,28 @@ setxattr(struct mnt_idmap *idmap, struct dentry *d,
return error;
}
-static int path_setxattr(const char __user *pathname,
+static int do_setxattrat(int dfd, const char __user *pathname, unsigned int at_flags,
const char __user *name, const void __user *value,
- size_t size, int flags, unsigned int lookup_flags)
+ size_t size, int xattr_flags)
{
struct path path;
int error;
+ int lookup_flags;
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+ if (at_flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
retry:
- error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+ error = user_path_at(dfd, pathname, lookup_flags, &path);
if (error)
return error;
error = mnt_want_write(path.mnt);
if (!error) {
error = setxattr(mnt_idmap(path.mnt), path.dentry, name,
- value, size, flags);
+ value, size, xattr_flags);
mnt_drop_write(path.mnt);
}
path_put(&path);
@@ -680,18 +687,38 @@ static int path_setxattr(const char __user *pathname,
return error;
}
+SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+ const char __user *, name, const struct xattr_args __user *, uargs,
+ size_t, usize)
+{
+ struct xattr_args args = {};
+ int error;
+
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+ if (usize < sizeof(args))
+ return -EINVAL;
+
+ error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+ if (error)
+ return error;
+
+ return do_setxattrat(dfd, pathname, at_flags, name, (const void __user *)args.value,
+ args.size, args.flags);
+}
+
SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
const char __user *, name, const void __user *, value,
size_t, size, int, flags)
{
- return path_setxattr(pathname, name, value, size, flags, LOOKUP_FOLLOW);
+ return do_setxattrat(AT_FDCWD, pathname, 0, name, value, size, flags);
}
SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
const char __user *, name, const void __user *, value,
size_t, size, int, flags)
{
- return path_setxattr(pathname, name, value, size, flags, 0);
+ return do_setxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name, value, size, flags);
}
SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
@@ -774,14 +801,22 @@ getxattr(struct mnt_idmap *idmap, struct dentry *d,
return error;
}
-static ssize_t path_getxattr(const char __user *pathname,
+static ssize_t do_getxattrat(int dfd, const char __user *pathname, unsigned int at_flags,
const char __user *name, void __user *value,
- size_t size, unsigned int lookup_flags)
+ size_t size)
{
struct path path;
ssize_t error;
+ int lookup_flags;
+
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+ if (at_flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
retry:
- error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+ error = user_path_at(dfd, pathname, lookup_flags, &path);
if (error)
return error;
error = getxattr(mnt_idmap(path.mnt), path.dentry, name, value, size);
@@ -793,16 +828,37 @@ static ssize_t path_getxattr(const char __user *pathname,
return error;
}
+SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+ const char __user *, name, struct xattr_args __user *, uargs, size_t, usize)
+{
+ struct xattr_args args = {};
+ int error;
+
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+ if (usize < sizeof(args))
+ return -EINVAL;
+
+ error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+ if (error)
+ return error;
+
+ if (args.flags != 0)
+ return -EINVAL;
+
+ return do_getxattrat(dfd, pathname, at_flags, name, (void __user *)args.value, args.size);
+}
+
SYSCALL_DEFINE4(getxattr, const char __user *, pathname,
const char __user *, name, void __user *, value, size_t, size)
{
- return path_getxattr(pathname, name, value, size, LOOKUP_FOLLOW);
+ return do_getxattrat(AT_FDCWD, pathname, 0, name, value, size);
}
SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
const char __user *, name, void __user *, value, size_t, size)
{
- return path_getxattr(pathname, name, value, size, 0);
+ return do_getxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name, value, size);
}
SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
@@ -852,13 +908,21 @@ listxattr(struct dentry *d, char __user *list, size_t size)
return error;
}
-static ssize_t path_listxattr(const char __user *pathname, char __user *list,
- size_t size, unsigned int lookup_flags)
+static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user *list,
+ size_t size, int flags)
{
struct path path;
ssize_t error;
+ int lookup_flags;
+
+ if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+ if (flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
retry:
- error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+ error = user_path_at(dfd, pathname, lookup_flags, &path);
if (error)
return error;
error = listxattr(path.dentry, list, size);
@@ -870,16 +934,22 @@ static ssize_t path_listxattr(const char __user *pathname, char __user *list,
return error;
}
+SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname, char __user *, list,
+ size_t, size, int, flags)
+{
+ return do_listxattrat(dfd, pathname, list, size, flags);
+}
+
SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
size_t, size)
{
- return path_listxattr(pathname, list, size, LOOKUP_FOLLOW);
+ return do_listxattrat(AT_FDCWD, pathname, list, size, 0);
}
SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
size_t, size)
{
- return path_listxattr(pathname, list, size, 0);
+ return do_listxattrat(AT_FDCWD, pathname, list, size, AT_SYMLINK_NOFOLLOW);
}
SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
@@ -898,7 +968,7 @@ SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
/*
* Extended attribute REMOVE operations
*/
-static long
+static int
removexattr(struct mnt_idmap *idmap, struct dentry *d,
const char __user *name)
{
@@ -917,13 +987,21 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d,
return vfs_removexattr(idmap, d, kname);
}
-static int path_removexattr(const char __user *pathname,
- const char __user *name, unsigned int lookup_flags)
+static int do_removexattrat(int dfd, const char __user *pathname,
+ const char __user *name, int flags)
{
struct path path;
int error;
+ int lookup_flags;
+
+ if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+ if (flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
retry:
- error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+ error = user_path_at(dfd, pathname, lookup_flags, &path);
if (error)
return error;
error = mnt_want_write(path.mnt);
@@ -939,16 +1017,22 @@ static int path_removexattr(const char __user *pathname,
return error;
}
+SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname,
+ const char __user *, name, int, flags)
+{
+ return do_removexattrat(dfd, pathname, name, flags);
+}
+
SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
const char __user *, name)
{
- return path_removexattr(pathname, name, LOOKUP_FOLLOW);
+ return do_removexattrat(AT_FDCWD, pathname, name, 0);
}
SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
const char __user *, name)
{
- return path_removexattr(pathname, name, 0);
+ return do_removexattrat(AT_FDCWD, pathname, name, AT_SYMLINK_NOFOLLOW);
}
SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
diff --git a/include/asm-generic/audit_change_attr.h b/include/asm-generic/audit_change_attr.h
index 331670807cf0..cc840537885f 100644
--- a/include/asm-generic/audit_change_attr.h
+++ b/include/asm-generic/audit_change_attr.h
@@ -11,9 +11,15 @@ __NR_lchown,
__NR_fchown,
#endif
__NR_setxattr,
+#ifdef __NR_setxattrat
+__NR_setxattrat,
+#endif
__NR_lsetxattr,
__NR_fsetxattr,
__NR_removexattr,
+#ifdef __NR_removexattrat
+__NR_removexattrat,
+#endif
__NR_lremovexattr,
__NR_fremovexattr,
#ifdef __NR_fchownat
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e619ac10cd23..e06fffc48535 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -338,23 +338,33 @@ asmlinkage long sys_io_uring_register(unsigned int fd, unsigned int op,
void __user *arg, unsigned int nr_args);
asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
const void __user *value, size_t size, int flags);
+asmlinkage long sys_setxattrat(int dfd, const char __user *path, unsigned int at_flags,
+ const char __user *name,
+ const struct xattr_args __user *args, size_t size);
asmlinkage long sys_lsetxattr(const char __user *path, const char __user *name,
const void __user *value, size_t size, int flags);
asmlinkage long sys_fsetxattr(int fd, const char __user *name,
const void __user *value, size_t size, int flags);
asmlinkage long sys_getxattr(const char __user *path, const char __user *name,
void __user *value, size_t size);
+asmlinkage long sys_getxattrat(int dfd, const char __user *path, unsigned int at_flags,
+ const char __user *name,
+ struct xattr_args __user *args, size_t size);
asmlinkage long sys_lgetxattr(const char __user *path, const char __user *name,
void __user *value, size_t size);
asmlinkage long sys_fgetxattr(int fd, const char __user *name,
void __user *value, size_t size);
asmlinkage long sys_listxattr(const char __user *path, char __user *list,
size_t size);
+asmlinkage long sys_listxattrat(int dfd, const char __user *path, char __user *list,
+ size_t size, int flags);
asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
size_t size);
asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
asmlinkage long sys_removexattr(const char __user *path,
const char __user *name);
+asmlinkage long sys_removexattrat(int dfd, const char __user *path,
+ const char __user *name, int flags);
asmlinkage long sys_lremovexattr(const char __user *path,
const char __user *name);
asmlinkage long sys_fremovexattr(int fd, const char __user *name);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 75f00965ab15..21b275a8dcd6 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -842,8 +842,18 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+/* fs/xattr.c */
+#define __NR_setxattrat 462
+__SYSCALL(__NR_setxattrat, sys_setxattrat)
+#define __NR_getxattrat 463
+__SYSCALL(__NR_getxattrat, sys_getxattrat)
+#define __NR_listxattrat 464
+__SYSCALL(__NR_listxattrat, sys_listxattrat)
+#define __NR_removexattrat 465
+__SYSCALL(__NR_removexattrat, sys_removexattrat)
+
#undef __NR_syscalls
-#define __NR_syscalls 462
+#define __NR_syscalls 466
/*
* 32 bit systems traditionally used different
diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 9463db2dfa9d..e9ac2acc40c3 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -20,6 +20,12 @@
#define XATTR_CREATE 0x1 /* set value, fail if attr already exists */
#define XATTR_REPLACE 0x2 /* set value, fail if attr does not exist */
+
+struct xattr_args {
+ __aligned_u64 __user value;
+ __u32 size;
+ __u32 flags;
+};
#endif
/* Namespaces */
--
2.43.0
^ permalink raw reply related
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Jonathan Cameron @ 2024-04-26 16:53 UTC (permalink / raw)
To: Dan Williams
Cc: linux-cxl, Sreenivas Bagalkote, Brett Henning, Harold Johnson,
Sumanesh Samanta, linux-kernel, Davidlohr Bueso, Dave Jiang,
Alison Schofield, Vishal Verma, Ira Weiny, linuxarm, linux-api,
Lorenzo Pieralisi, Natu, Mahesh, gregkh
In-Reply-To: <662bd36caae55_a96f2943f@dwillia2-mobl3.amr.corp.intel.com.notmuch>
On Fri, 26 Apr 2024 09:16:44 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> Jonathan Cameron wrote:
> [..]
> > To give people an incentive to play the standards game we have to
> > provide an alternative. Userspace libraries will provide some incentive
> > to standardize if we have enough vendors (we don't today - so they will
> > do their own libraries), but it is a lot easier to encourage if we
> > exercise control over the interface.
>
> Yes, and I expect you and I are not far off on what can be done
> here.
>
> However, lets cut to a sentiment hanging over this discussion. Referring
> to vendor specific commands:
>
> "CXL spec has them for a reason and they need to be supported."
>
> ...that is an aggressive "vendor specific first" sentiment that
> generates an aggressive "userspace drivers" reaction, because the best
> way to get around community discussions about what ABI makes sense is
> userspace drivers.
>
> Now, if we can step back to where this discussion started, where typical
> Linux collaboration shines, and where I think you and I are more aligned
> than this thread would indicate, is "vendor specific last". Lets
> carefully consider the vendor specific commands that are candidates to
> be de facto cross vendor semantics if not de jure standards.
>
Agreed. I'd go a little further and say I generally have much more warm and
fuzzy feelings when what is a vendor defined command (today) maps to more
or less the same bit of code for a proposed standards ECN.
IP rules prevent us commenting on specific proposals, but there will be
things we review quicker and with a lighter touch vs others where we
ask lots of annoying questions about generality of the feature etc.
Given the effort we are putting in on the kernel side we all want CXL
to succeed and will do our best to encourage activities that make that
more likely. There are other standards bodies available... which may
make more sense for some features.
Command interfaces are not a good place to compete and maintain secrecy.
If vendors want to do that, then they don't get the pony of upstream
support. They get to convince distros to do a custom kernel build for them:
Good luck with that, some of those folk are 'blunt' in their responses to
such requests.
My proposal is we go forward with a bunch of the CXL spec defined commands
to show the 'how' and consider specific proposals for upstream support
of vendor defined commands on a case by case basis (so pretty much
what you say above). Maybe after a few are done we can formalize some
rules of thumb help vendors makes such proposals, though maybe some
will figure out it is a better and longer term solution to do 'standards
first development'.
I think we do need to look at the safety filtering of tunneled
commands but don't see that as a particularly tricky addition -
for the simple non destructive commands at least.
Jonathan
^ permalink raw reply
* Re: [PATCH v3 2/2] fs/xattr: add *at family syscalls
From: Arnd Bergmann @ 2024-04-26 17:38 UTC (permalink / raw)
To: cgzones
Cc: x86, linux-alpha, linux-kernel, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, audit, Linux-Arch, linux-api,
linux-security-module, selinux, Richard Henderson,
Ivan Kokshaysky, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E . J . Bottomley, Helge Deller,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S . Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, Eric Paris, Jens Axboe,
Pavel Begunkov, Peter Zijlstra, Sohil Mehta, Palmer Dabbelt,
Miklos Szeredi, Nhat Pham, Casey Schaufler, Florian Fainelli,
Kees Cook, Rick Edgecombe, Mark Rutland, io-uring
In-Reply-To: <20240426162042.191916-1-cgoettsche@seltendoof.de>
On Fri, Apr 26, 2024, at 18:20, Christian Göttsche wrote:
> From: Christian Göttsche <cgzones@googlemail.com>
>
> Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
> removexattrat(). Those can be used to operate on extended attributes,
> especially security related ones, either relative to a pinned directory
> or on a file descriptor without read access, avoiding a
> /proc/<pid>/fd/<fd> detour, requiring a mounted procfs.
>
> One use case will be setfiles(8) setting SELinux file contexts
> ("security.selinux") without race conditions and without a file
> descriptor opened with read access requiring SELinux read permission.
>
> Use the do_{name}at() pattern from fs/open.c.
>
> Pass the value of the extended attribute, its length, and for
> setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
> struct xattr_args to not exceed six syscall arguments and not
> merging the AT_* and XATTR_* flags.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> CC: x86@kernel.org
> CC: linux-alpha@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-ia64@vger.kernel.org
> CC: linux-m68k@lists.linux-m68k.org
> CC: linux-mips@vger.kernel.org
> CC: linux-parisc@vger.kernel.org
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-s390@vger.kernel.org
> CC: linux-sh@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> CC: linux-fsdevel@vger.kernel.org
> CC: audit@vger.kernel.org
> CC: linux-arch@vger.kernel.org
> CC: linux-api@vger.kernel.org
> CC: linux-security-module@vger.kernel.org
> CC: selinux@vger.kernel.org
I checked that the syscalls are all well-formed regarding
argument types, number of arguments and (absence of)
compat handling, and that they are wired up correctly
across architectures
I did not look at the actual implementation in detail.
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 1/7] uprobe: Wire up uretprobe system call
From: Andrii Nakryiko @ 2024-04-26 17:56 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <20240421194206.1010934-2-jolsa@kernel.org>
On Sun, Apr 21, 2024 at 12:42 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Wiring up uretprobe system call, which comes in following changes.
> We need to do the wiring before, because the uretprobe implementation
> needs the syscall number.
>
> Note at the moment uretprobe syscall is supported only for native
> 64-bit process.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> include/linux/syscalls.h | 2 ++
> include/uapi/asm-generic/unistd.h | 5 ++++-
> kernel/sys_ni.c | 2 ++
> 4 files changed, 9 insertions(+), 1 deletion(-)
>
LGTM
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 7e8d46f4147f..af0a33ab06ee 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -383,6 +383,7 @@
> 459 common lsm_get_self_attr sys_lsm_get_self_attr
> 460 common lsm_set_self_attr sys_lsm_set_self_attr
> 461 common lsm_list_modules sys_lsm_list_modules
> +462 64 uretprobe sys_uretprobe
>
> #
> # Due to a historical design error, certain syscalls are numbered differently
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e619ac10cd23..5318e0e76799 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -972,6 +972,8 @@ asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
> /* x86 */
> asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int on);
>
> +asmlinkage long sys_uretprobe(void);
> +
> /* pciconfig: alpha, arm, arm64, ia64, sparc */
> asmlinkage long sys_pciconfig_read(unsigned long bus, unsigned long dfn,
> unsigned long off, unsigned long len,
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 75f00965ab15..8a747cd1d735 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -842,8 +842,11 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
> #define __NR_lsm_list_modules 461
> __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
>
> +#define __NR_uretprobe 462
> +__SYSCALL(__NR_uretprobe, sys_uretprobe)
> +
> #undef __NR_syscalls
> -#define __NR_syscalls 462
> +#define __NR_syscalls 463
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index faad00cce269..be6195e0d078 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -391,3 +391,5 @@ COND_SYSCALL(setuid16);
>
> /* restartable sequence */
> COND_SYSCALL(rseq);
> +
> +COND_SYSCALL(uretprobe);
> --
> 2.44.0
>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe
From: Andrii Nakryiko @ 2024-04-26 17:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <20240421194206.1010934-3-jolsa@kernel.org>
On Sun, Apr 21, 2024 at 12:42 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding uretprobe syscall instead of trap to speed up return probe.
>
> At the moment the uretprobe setup/path is:
>
> - install entry uprobe
>
> - when the uprobe is hit, it overwrites probed function's return address
> on stack with address of the trampoline that contains breakpoint
> instruction
>
> - the breakpoint trap code handles the uretprobe consumers execution and
> jumps back to original return address
>
> This patch replaces the above trampoline's breakpoint instruction with new
> ureprobe syscall call. This syscall does exactly the same job as the trap
> with some more extra work:
>
> - syscall trampoline must save original value for rax/r11/rcx registers
> on stack - rax is set to syscall number and r11/rcx are changed and
> used by syscall instruction
>
> - the syscall code reads the original values of those registers and
> restore those values in task's pt_regs area
>
> - only caller from trampoline exposed in '[uprobes]' is allowed,
> the process will receive SIGILL signal otherwise
>
> Even with some extra work, using the uretprobes syscall shows speed
> improvement (compared to using standard breakpoint):
>
> On Intel (11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz)
>
> current:
> uretprobe-nop : 1.498 ± 0.000M/s
> uretprobe-push : 1.448 ± 0.001M/s
> uretprobe-ret : 0.816 ± 0.001M/s
>
> with the fix:
> uretprobe-nop : 1.969 ± 0.002M/s < 31% speed up
> uretprobe-push : 1.910 ± 0.000M/s < 31% speed up
> uretprobe-ret : 0.934 ± 0.000M/s < 14% speed up
>
> On Amd (AMD Ryzen 7 5700U)
>
> current:
> uretprobe-nop : 0.778 ± 0.001M/s
> uretprobe-push : 0.744 ± 0.001M/s
> uretprobe-ret : 0.540 ± 0.001M/s
>
> with the fix:
> uretprobe-nop : 0.860 ± 0.001M/s < 10% speed up
> uretprobe-push : 0.818 ± 0.001M/s < 10% speed up
> uretprobe-ret : 0.578 ± 0.000M/s < 7% speed up
>
> The performance test spawns a thread that runs loop which triggers
> uprobe with attached bpf program that increments the counter that
> gets printed in results above.
>
> The uprobe (and uretprobe) kind is determined by which instruction
> is being patched with breakpoint instruction. That's also important
> for uretprobes, because uprobe is installed for each uretprobe.
>
> The performance test is part of bpf selftests:
> tools/testing/selftests/bpf/run_bench_uprobes.sh
>
> Note at the moment uretprobe syscall is supported only for native
> 64-bit process, compat process still uses standard breakpoint.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> arch/x86/kernel/uprobes.c | 115 ++++++++++++++++++++++++++++++++++++++
> include/linux/uprobes.h | 3 +
> kernel/events/uprobes.c | 24 +++++---
> 3 files changed, 135 insertions(+), 7 deletions(-)
>
LGTM as far as I can follow the code
Acked-by: Andrii Nakryiko <andrii@kernel.org>
[...]
^ permalink raw reply
* Re: [PATCHv3 bpf-next 5/7] selftests/bpf: Add uretprobe syscall call from user space test
From: Andrii Nakryiko @ 2024-04-26 18:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <20240421194206.1010934-6-jolsa@kernel.org>
On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test to verify that when called from outside of the
> trampoline provided by kernel, the uretprobe syscall will cause
> calling process to receive SIGILL signal and the attached bpf
> program is no executed.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../selftests/bpf/prog_tests/uprobe_syscall.c | 92 +++++++++++++++++++
> .../selftests/bpf/progs/uprobe_syscall_call.c | 15 +++
> 2 files changed, 107 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
>
See nits below, but overall LGTM
Acked-by: Andrii Nakryiko <andrii@kernel.org>
[...]
> @@ -219,6 +301,11 @@ static void test_uretprobe_regs_change(void)
> {
> test__skip();
> }
> +
> +static void test_uretprobe_syscall_call(void)
> +{
> + test__skip();
> +}
> #endif
>
> void test_uprobe_syscall(void)
> @@ -228,3 +315,8 @@ void test_uprobe_syscall(void)
> if (test__start_subtest("uretprobe_regs_change"))
> test_uretprobe_regs_change();
> }
> +
> +void serial_test_uprobe_syscall_call(void)
does it need to be serial? non-serial are still run sequentially
within a process (there is no multi-threading), it's more about some
global effects on system.
> +{
> + test_uretprobe_syscall_call();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> new file mode 100644
> index 000000000000..5ea03bb47198
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <string.h>
> +
> +struct pt_regs regs;
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("uretprobe//proc/self/exe:uretprobe_syscall_call")
> +int uretprobe(struct pt_regs *regs)
> +{
> + bpf_printk("uretprobe called");
debugging leftover? we probably don't want to pollute trace_pipe from test
> + return 0;
> +}
> --
> 2.44.0
>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
From: Andrii Nakryiko @ 2024-04-26 18:06 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <20240421194206.1010934-7-jolsa@kernel.org>
On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test that adds return uprobe inside 32 bit task
> and verify the return uprobe and attached bpf programs
> get properly executed.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/.gitignore | 1 +
> tools/testing/selftests/bpf/Makefile | 6 ++-
> .../selftests/bpf/prog_tests/uprobe_syscall.c | 40 +++++++++++++++++++
> .../bpf/progs/uprobe_syscall_compat.c | 13 ++++++
> 4 files changed, 59 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index f1aebabfb017..69d71223c0dd 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -45,6 +45,7 @@ test_cpp
> /veristat
> /sign-file
> /uprobe_multi
> +/uprobe_compat
> *.ko
> *.tmp
> xskxceiver
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index edc73f8f5aef..d170b63eca62 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -134,7 +134,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> xdp_features bpf_test_no_cfi.ko
>
> -TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
> +TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi uprobe_compat
you need to add uprobe_compat to TRUNNER_EXTRA_FILES as well, no?
>
> # Emit succinct information message describing current building step
> # $1 - generic step name (e.g., CC, LINK, etc);
> @@ -761,6 +761,10 @@ $(OUTPUT)/uprobe_multi: uprobe_multi.c
> $(call msg,BINARY,,$@)
> $(Q)$(CC) $(CFLAGS) -O0 $(LDFLAGS) $^ $(LDLIBS) -o $@
>
> +$(OUTPUT)/uprobe_compat:
> + $(call msg,BINARY,,$@)
> + $(Q)echo "int main() { return 0; }" | $(CC) $(CFLAGS) -xc -m32 -O0 - -o $@
> +
> EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
> prog_tests/tests.h map_tests/tests.h verifier/tests.h \
> feature bpftool \
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index 9233210a4c33..3770254d893b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> @@ -11,6 +11,7 @@
> #include <sys/wait.h>
> #include "uprobe_syscall.skel.h"
> #include "uprobe_syscall_call.skel.h"
> +#include "uprobe_syscall_compat.skel.h"
>
> __naked unsigned long uretprobe_regs_trigger(void)
> {
> @@ -291,6 +292,35 @@ static void test_uretprobe_syscall_call(void)
> "read_trace_pipe_iter");
> ASSERT_EQ(found, 0, "found");
> }
> +
> +static void trace_pipe_compat_cb(const char *str, void *data)
> +{
> + if (strstr(str, "uretprobe compat") != NULL)
> + (*(int *)data)++;
> +}
> +
> +static void test_uretprobe_compat(void)
> +{
> + struct uprobe_syscall_compat *skel = NULL;
> + int err, found = 0;
> +
> + skel = uprobe_syscall_compat__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "uprobe_syscall_compat__open_and_load"))
> + goto cleanup;
> +
> + err = uprobe_syscall_compat__attach(skel);
> + if (!ASSERT_OK(err, "uprobe_syscall_compat__attach"))
> + goto cleanup;
> +
> + system("./uprobe_compat");
> +
> + ASSERT_OK(read_trace_pipe_iter(trace_pipe_compat_cb, &found, 1000),
> + "read_trace_pipe_iter");
why so complicated? can't you just set global variable that it was called
> + ASSERT_EQ(found, 1, "found");
> +
> +cleanup:
> + uprobe_syscall_compat__destroy(skel);
> +}
> #else
> static void test_uretprobe_regs_equal(void)
> {
> @@ -306,6 +336,11 @@ static void test_uretprobe_syscall_call(void)
> {
> test__skip();
> }
> +
> +static void test_uretprobe_compat(void)
> +{
> + test__skip();
> +}
> #endif
>
> void test_uprobe_syscall(void)
> @@ -320,3 +355,8 @@ void serial_test_uprobe_syscall_call(void)
> {
> test_uretprobe_syscall_call();
> }
> +
> +void serial_test_uprobe_syscall_compat(void)
and then no need for serial_test?
> +{
> + test_uretprobe_compat();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> new file mode 100644
> index 000000000000..f8adde7f08e2
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("uretprobe.multi/./uprobe_compat:main")
> +int uretprobe_compat(struct pt_regs *ctx)
> +{
> + bpf_printk("uretprobe compat\n");
> + return 0;
> +}
> --
> 2.44.0
>
^ permalink raw reply
* RE: RFC: Restricting userspace interfaces for CXL fabric management
From: Harold Johnson @ 2024-04-26 19:25 UTC (permalink / raw)
To: Jonathan Cameron, Dan Williams
Cc: linux-cxl, Sreenivas Bagalkote, Brett Henning, Sumanesh Samanta,
linux-kernel, Davidlohr Bueso, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, linuxarm, linux-api, Lorenzo Pieralisi,
Natu, Mahesh, gregkh
In-Reply-To: <20240426175341.00002e67@Huawei.com>
[-- Attachment #1: Type: text/plain, Size: 7740 bytes --]
Perhaps a bit more color on a few specifics might be helpful.
I think that there will always be a class of vendor specific APIs/Opcodes
that are related to an implementation of a standard instead of the
standard itself. I've been party to discussion on not creating CXL
defined API/Opcodes that get into the realm of specifying an
implementation. There are also a class of data that can be collected from
a specific implementation that is helpful for debug, for health
monitoring, and perhaps performance monitoring where the implementation
matters and therefore are not easily abstracted to a standard.
A few examples:
a) Temperature monitoring of a component or internal chip die
temperatures. Could CXL define a standard OpCode to gather temperatures,
yes it could; but is this really part of CXL? Then how many temperature
elements and what does each element mean? This enters into the
implementation and therefore is vendor specific. Unless the CXL spec
starts to define the implementation, something along the lines of "thou
shall have an average die temperature, rather than specific temperatures
across a die", etc.
b) Error counters, metrics, internal counters, etc. Could CXL define a
set of common error counters, absolutely. PCIe has done some of this.
However, a specific implementation may have counters and error reporting
that are meaningful only to a specific design and a specific
implementation rather than a "least common denominator" approach of a
standard body.
c) Performance counters, metric, indicators, etc. Performance can be very
implementation specific and tweaking performance is likely to be
implementation specific. Yes, generic and a least common denominator
elements could be created, but are likely to limiting in realizing the
maximum performance of an implementation.
d) Logs, errors and debug information. In addition to spec defined
logging of CXL topology errors, specific designs will have logs, crash
dumps, debug data that is very specific to a implementation. There are
likely to be cases where a product that conforms to a specification like
CXL, may have features that don't directly have anything to do with CXL,
but where a standards based management interface can be used to configure,
manage, and collect data for a non-CXL feature.
e) Innovation. I believe that innovation should be encouraged. There may
be designs that support CXL, but that also incorporate unique and
innovative features or functions that might service a niche market. The
AI space is ripe for innovation and perhaps specialized features that may
not make sense for the overall CXL specification.
I think that in most cases Vendor specific opcodes are not used to
circumvent the standards, but are used when the standards group has no
interested in driving into the standard certain features that are clearly
either implementation specific or are vendor specific additions that have
a specific appeal to a select class of customer, but yet are not relevant
to a specific standard.
At the end of the day, customer want products that solve a specific
problem. Sometimes vendor can address market segments or niches that a
standard group has no interest in supporting. It can also take months,
and in some cases years to reach an agreement on what standardized feature
should look like. I also believe that there can be competitive reasons
why there might be a group that wants to slow down a vendor's
implementation for fear of losing market share.
Thanks
Harold Johnson
-----Original Message-----
From: Jonathan Cameron [mailto:Jonathan.Cameron@Huawei.com]
Sent: Friday, April 26, 2024 11:54 AM
To: Dan Williams
Cc: linux-cxl@vger.kernel.org; Sreenivas Bagalkote; Brett Henning; Harold
Johnson; Sumanesh Samanta; linux-kernel@vger.kernel.org; Davidlohr Bueso;
Dave Jiang; Alison Schofield; Vishal Verma; Ira Weiny;
linuxarm@huawei.com; linux-api@vger.kernel.org; Lorenzo Pieralisi; Natu,
Mahesh; gregkh@linuxfoundation.org
Subject: Re: RFC: Restricting userspace interfaces for CXL fabric
management
On Fri, 26 Apr 2024 09:16:44 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> Jonathan Cameron wrote:
> [..]
> > To give people an incentive to play the standards game we have to
> > provide an alternative. Userspace libraries will provide some
incentive
> > to standardize if we have enough vendors (we don't today - so they
will
> > do their own libraries), but it is a lot easier to encourage if we
> > exercise control over the interface.
>
> Yes, and I expect you and I are not far off on what can be done
> here.
>
> However, lets cut to a sentiment hanging over this discussion. Referring
> to vendor specific commands:
>
> "CXL spec has them for a reason and they need to be supported."
>
> ...that is an aggressive "vendor specific first" sentiment that
> generates an aggressive "userspace drivers" reaction, because the best
> way to get around community discussions about what ABI makes sense is
> userspace drivers.
>
> Now, if we can step back to where this discussion started, where typical
> Linux collaboration shines, and where I think you and I are more aligned
> than this thread would indicate, is "vendor specific last". Lets
> carefully consider the vendor specific commands that are candidates to
> be de facto cross vendor semantics if not de jure standards.
>
Agreed. I'd go a little further and say I generally have much more warm
and
fuzzy feelings when what is a vendor defined command (today) maps to more
or less the same bit of code for a proposed standards ECN.
IP rules prevent us commenting on specific proposals, but there will be
things we review quicker and with a lighter touch vs others where we
ask lots of annoying questions about generality of the feature etc.
Given the effort we are putting in on the kernel side we all want CXL
to succeed and will do our best to encourage activities that make that
more likely. There are other standards bodies available... which may
make more sense for some features.
Command interfaces are not a good place to compete and maintain secrecy.
If vendors want to do that, then they don't get the pony of upstream
support. They get to convince distros to do a custom kernel build for
them:
Good luck with that, some of those folk are 'blunt' in their responses to
such requests.
My proposal is we go forward with a bunch of the CXL spec defined commands
to show the 'how' and consider specific proposals for upstream support
of vendor defined commands on a case by case basis (so pretty much
what you say above). Maybe after a few are done we can formalize some
rules of thumb help vendors makes such proposals, though maybe some
will figure out it is a better and longer term solution to do 'standards
first development'.
I think we do need to look at the safety filtering of tunneled
commands but don't see that as a particularly tricky addition -
for the simple non destructive commands at least.
Jonathan
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4215 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/3] open: add O_CRED_ALLOW flag
From: kernel test robot @ 2024-04-27 2:12 UTC (permalink / raw)
To: Stas Sergeev, linux-kernel
Cc: oe-kbuild-all, Stas Sergeev, Stefan Metzmacher, Eric Biederman,
Alexander Viro, Andy Lutomirski, Christian Brauner, Jan Kara,
Jeff Layton, Chuck Lever, Alexander Aring, David Laight,
linux-fsdevel, linux-api, Paolo Bonzini, Christian Göttsche,
Arnd Bergmann, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jens Axboe, Kuniyuki Iwashima, Pavel Begunkov, linux-arch, netdev
In-Reply-To: <20240426133310.1159976-3-stsp2@yandex.ru>
Hi Stas,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.9-rc5 next-20240426]
[cannot apply to arnd-asm-generic/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Stas-Sergeev/fs-reorganize-path_openat/20240426-214030
base: linus/master
patch link: https://lore.kernel.org/r/20240426133310.1159976-3-stsp2%40yandex.ru
patch subject: [PATCH v5 2/3] open: add O_CRED_ALLOW flag
config: parisc-allnoconfig (https://download.01.org/0day-ci/archive/20240427/202404270923.bAeBIJt1-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404270923.bAeBIJt1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404270923.bAeBIJt1-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
fs/fcntl.c: In function 'fcntl_init':
>> include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_297' declared with attribute error: BUILD_BUG_ON failed: 22 - 1 != HWEIGHT32( (VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) | __FMODE_EXEC | __FMODE_NONOTIFY)
449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
430 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
fs/fcntl.c:1042:9: note: in expansion of macro 'BUILD_BUG_ON'
1042 | BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
| ^~~~~~~~~~~~
vim +/__compiletime_assert_297 +449 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 435
eb5c2d4b45e3d2 Will Deacon 2020-07-21 436 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 437 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 438
eb5c2d4b45e3d2 Will Deacon 2020-07-21 439 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 440 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 441 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 442 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 443 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 444 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 445 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 446 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 447 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 448 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @449 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 450
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Greg KH @ 2024-04-27 11:12 UTC (permalink / raw)
To: Harold Johnson
Cc: Jonathan Cameron, Dan Williams, linux-cxl, Sreenivas Bagalkote,
Brett Henning, Sumanesh Samanta, linux-kernel, Davidlohr Bueso,
Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm,
linux-api, Lorenzo Pieralisi, Natu, Mahesh
In-Reply-To: <6351024b5d6206c4e9a8cd98d1a09d43@mail.gmail.com>
On Fri, Apr 26, 2024 at 02:25:29PM -0500, Harold Johnson wrote:
> A few examples:
> a) Temperature monitoring of a component or internal chip die
> temperatures. Could CXL define a standard OpCode to gather temperatures,
> yes it could; but is this really part of CXL? Then how many temperature
> elements and what does each element mean? This enters into the
> implementation and therefore is vendor specific. Unless the CXL spec
> starts to define the implementation, something along the lines of "thou
> shall have an average die temperature, rather than specific temperatures
> across a die", etc.
>
> b) Error counters, metrics, internal counters, etc. Could CXL define a
> set of common error counters, absolutely. PCIe has done some of this.
> However, a specific implementation may have counters and error reporting
> that are meaningful only to a specific design and a specific
> implementation rather than a "least common denominator" approach of a
> standard body.
>
> c) Performance counters, metric, indicators, etc. Performance can be very
> implementation specific and tweaking performance is likely to be
> implementation specific. Yes, generic and a least common denominator
> elements could be created, but are likely to limiting in realizing the
> maximum performance of an implementation.
>
> d) Logs, errors and debug information. In addition to spec defined
> logging of CXL topology errors, specific designs will have logs, crash
> dumps, debug data that is very specific to a implementation. There are
> likely to be cases where a product that conforms to a specification like
> CXL, may have features that don't directly have anything to do with CXL,
> but where a standards based management interface can be used to configure,
> manage, and collect data for a non-CXL feature.
All of the above should be able to be handled by vendor-specific KERNEL
drivers that feed the needed information to the proper user/kernel apis
that the kernel already provides.
So while innovating at the hardware level is fine, follow the ways that
everyone has done this for other specification types (USB, PCI, etc.)
and just allow vendor drivers to provide the information. Don't do this
in crazy userspace drivers which will circumvent the whole reason we
have standard kernel/user apis in the first place for these types of
things.
> e) Innovation. I believe that innovation should be encouraged. There may
> be designs that support CXL, but that also incorporate unique and
> innovative features or functions that might service a niche market. The
> AI space is ripe for innovation and perhaps specialized features that may
> not make sense for the overall CXL specification.
>
> I think that in most cases Vendor specific opcodes are not used to
> circumvent the standards, but are used when the standards group has no
> interested in driving into the standard certain features that are clearly
> either implementation specific or are vendor specific additions that have
> a specific appeal to a select class of customer, but yet are not relevant
> to a specific standard.
Then fight this out in the specification groups, which are highly
political, and do not push that into the kernel space please. Again,
this is nothing new, we have all done this for specs for decades now,
allow vendor additions to the spec and handle that in the kernel and all
should be ok, right?
Or am I missing something obvious here where we would NOT want to do
what all other specs have done?
thanks,
greg k-h
^ permalink raw reply
* [PATCH v6 1/3] fs: reorganize path_openat()
From: Stas Sergeev @ 2024-04-27 11:24 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240427112451.1609471-1-stsp2@yandex.ru>
This patch moves the call to alloc_empty_file() down the if branches.
That changes is needed for the next patch, which adds a cred override
for alloc_empty_file(). The cred override is only needed in one branch,
i.e. it is not needed for O_PATH and O_TMPFILE..
No functional changes are intended by that patch.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Andy Lutomirski <luto@kernel.org>
CC: David Laight <David.Laight@ACULAB.COM>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
fs/namei.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index c5b2a25be7d0..dd50345f7260 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3781,17 +3781,24 @@ static struct file *path_openat(struct nameidata *nd,
{
struct file *file;
int error;
+ int open_flags = op->open_flag;
- file = alloc_empty_file(op->open_flag, current_cred());
- if (IS_ERR(file))
- return file;
-
- if (unlikely(file->f_flags & __O_TMPFILE)) {
- error = do_tmpfile(nd, flags, op, file);
- } else if (unlikely(file->f_flags & O_PATH)) {
- error = do_o_path(nd, flags, file);
+ if (unlikely(open_flags & (__O_TMPFILE | O_PATH))) {
+ file = alloc_empty_file(open_flags, current_cred());
+ if (IS_ERR(file))
+ return file;
+ if (open_flags & __O_TMPFILE)
+ error = do_tmpfile(nd, flags, op, file);
+ else
+ error = do_o_path(nd, flags, file);
} else {
- const char *s = path_init(nd, flags);
+ const char *s;
+
+ file = alloc_empty_file(open_flags, current_cred());
+ if (IS_ERR(file))
+ return file;
+
+ s = path_init(nd, flags);
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
;
--
2.44.0
^ permalink raw reply related
* [PATCH v6 3/3] openat2: add OA2_CRED_INHERIT flag
From: Stas Sergeev @ 2024-04-27 11:24 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240427112451.1609471-1-stsp2@yandex.ru>
This flag performs the open operation with the fs credentials
(fsuid, fsgid, group_info) that were in effect when dir_fd was opened.
dir_fd must be opened with O_CRED_ALLOW, or EPERM is returned.
Selftests are added to check for these properties as well as for
the invalid flag combinations.
This allows the process to pre-open some directories and then
change eUID (and all other UIDs/GIDs) to a less-privileged user,
retaining the ability to open/create files within these directories.
Design goal:
The idea is to provide a very light-weight sandboxing, where the
process, without the use of any heavy-weight techniques like chroot
within namespaces, can restrict the access to the set of pre-opened
directories.
This patch is just a first step to such sandboxing. If things go
well, in the future the same extension can be added to more syscalls.
These should include at least unlinkat(), renameat2() and the
not-yet-upstreamed setxattrat().
Security considerations:
- Only the bare minimal set of credentials is overridden:
fsuid, fsgid and group_info. The rest, for example capabilities,
are not overridden to avoid unneeded security risks.
- To avoid sandboxing escape, this patch makes sure the restricted
lookup modes are used. Namely, RESOLVE_BENEATH or RESOLVE_IN_ROOT.
- Magic /proc symlinks are discarded, as suggested by
Andy Lutomirski <luto@kernel.org>
- O_CRED_ALLOW fds cannot be passed via unix socket and are always
closed on exec() to prevent "unsuspecting userspace" from not being
able to fully drop privs.
Use cases:
Virtual machines that deal with untrusted code, can use that
instead of a more heavy-weighted approaches.
Currently the approach is being tested on a dosemu2 VM.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Stefan Metzmacher <metze@samba.org>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Andy Lutomirski <luto@kernel.org>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Aring <alex.aring@gmail.com>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Christian Göttsche <cgzones@googlemail.com>
---
fs/fcntl.c | 2 +
fs/namei.c | 56 +++++++++-
fs/open.c | 10 +-
include/linux/fcntl.h | 2 +
include/uapi/linux/openat2.h | 2 +
tools/testing/selftests/openat2/Makefile | 2 +-
.../testing/selftests/openat2/cred_inherit.c | 105 ++++++++++++++++++
.../testing/selftests/openat2/openat2_test.c | 12 +-
8 files changed, 186 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/selftests/openat2/cred_inherit.c
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 78c96b1293c2..283c2e65fc2c 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1043,6 +1043,8 @@ static int __init fcntl_init(void)
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
+ BUILD_BUG_ON(HWEIGHT32(VALID_OPENAT2_FLAGS) !=
+ HWEIGHT32(VALID_OPEN_FLAGS) + 1);
fasync_cache = kmem_cache_create("fasync_cache",
sizeof(struct fasync_struct), 0,
diff --git a/fs/namei.c b/fs/namei.c
index dd50345f7260..aa5dcf57851b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3776,6 +3776,43 @@ static int do_o_path(struct nameidata *nd, unsigned flags, struct file *file)
return error;
}
+static const struct cred *openat2_init_creds(int dfd)
+{
+ struct cred *cred;
+ struct fd f;
+
+ if (dfd == AT_FDCWD)
+ return ERR_PTR(-EINVAL);
+
+ f = fdget_raw(dfd);
+ if (!f.file)
+ return ERR_PTR(-EBADF);
+
+ cred = ERR_PTR(-EPERM);
+ if (!(f.file->f_flags & O_CRED_ALLOW))
+ goto done;
+
+ cred = prepare_creds();
+ if (!cred) {
+ cred = ERR_PTR(-ENOMEM);
+ goto done;
+ }
+
+ cred->fsuid = f.file->f_cred->fsuid;
+ cred->fsgid = f.file->f_cred->fsgid;
+ cred->group_info = get_group_info(f.file->f_cred->group_info);
+
+done:
+ fdput(f);
+ return cred;
+}
+
+static void openat2_done_creds(const struct cred *cred)
+{
+ put_group_info(cred->group_info);
+ put_cred(cred);
+}
+
static struct file *path_openat(struct nameidata *nd,
const struct open_flags *op, unsigned flags)
{
@@ -3793,18 +3830,33 @@ static struct file *path_openat(struct nameidata *nd,
error = do_o_path(nd, flags, file);
} else {
const char *s;
+ const struct cred *old_cred = NULL, *cred = NULL;
- file = alloc_empty_file(open_flags, current_cred());
- if (IS_ERR(file))
+ if (open_flags & OA2_CRED_INHERIT) {
+ cred = openat2_init_creds(nd->dfd);
+ if (IS_ERR(cred))
+ return ERR_CAST(cred);
+ }
+ file = alloc_empty_file(open_flags, cred ?: current_cred());
+ if (IS_ERR(file)) {
+ if (cred)
+ openat2_done_creds(cred);
return file;
+ }
s = path_init(nd, flags);
+ if (cred)
+ old_cred = override_creds(cred);
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
;
if (!error)
error = do_open(nd, file, op);
+ if (old_cred)
+ revert_creds(old_cred);
terminate_walk(nd);
+ if (cred)
+ openat2_done_creds(cred);
}
if (likely(!error)) {
if (likely(file->f_mode & FMODE_OPENED))
diff --git a/fs/open.c b/fs/open.c
index ee8460c83c77..dd4fab536135 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1225,7 +1225,7 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
* values before calling build_open_flags(), but openat2(2) checks all
* of its arguments.
*/
- if (flags & ~VALID_OPEN_FLAGS)
+ if (flags & ~VALID_OPENAT2_FLAGS)
return -EINVAL;
if (how->resolve & ~VALID_RESOLVE_FLAGS)
return -EINVAL;
@@ -1326,6 +1326,14 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
lookup_flags |= LOOKUP_CACHED;
}
+ if (flags & OA2_CRED_INHERIT) {
+ /* Inherit creds only with scoped look-up modes. */
+ if (!(lookup_flags & LOOKUP_IS_SCOPED))
+ return -EPERM;
+ /* Reject /proc "magic" links if inheriting creds. */
+ lookup_flags |= LOOKUP_NO_MAGICLINKS;
+ }
+
op->lookup_flags = lookup_flags;
return 0;
}
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index e074ee9c1e36..33b9c7ad056b 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -12,6 +12,8 @@
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_CRED_ALLOW)
+#define VALID_OPENAT2_FLAGS (VALID_OPEN_FLAGS | OA2_CRED_INHERIT)
+
/* List of all valid flags for the how->resolve argument: */
#define VALID_RESOLVE_FLAGS \
(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
diff --git a/include/uapi/linux/openat2.h b/include/uapi/linux/openat2.h
index a5feb7604948..f803558ad62f 100644
--- a/include/uapi/linux/openat2.h
+++ b/include/uapi/linux/openat2.h
@@ -40,4 +40,6 @@ struct open_how {
return -EAGAIN if that's not
possible. */
+#define OA2_CRED_INHERIT (1UL << 28)
+
#endif /* _UAPI_LINUX_OPENAT2_H */
diff --git a/tools/testing/selftests/openat2/Makefile b/tools/testing/selftests/openat2/Makefile
index 254d676a2689..a1f4b5395f82 100644
--- a/tools/testing/selftests/openat2/Makefile
+++ b/tools/testing/selftests/openat2/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-or-later
CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined -static-libasan
-TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test
+TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test cred_inherit
include ../lib.mk
diff --git a/tools/testing/selftests/openat2/cred_inherit.c b/tools/testing/selftests/openat2/cred_inherit.c
new file mode 100644
index 000000000000..550a06763ac7
--- /dev/null
+++ b/tools/testing/selftests/openat2/cred_inherit.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/socket.h>
+#include <time.h>
+#include <unistd.h>
+#include <string.h>
+
+#include "../kselftest.h"
+#include "helpers.h"
+
+#ifndef O_CRED_ALLOW
+#define O_CRED_ALLOW 0x2000000
+#endif
+
+#ifndef OA2_CRED_INHERIT
+#define OA2_CRED_INHERIT (1UL << 28)
+#endif
+
+enum { FD_NORM, FD_NCA, FD_DIR, FD_DCA, FD_MAX };
+
+int main(int argc, char *argv[], char *env[])
+{
+ struct open_how how1 = {
+ .flags = O_RDONLY,
+ .resolve = RESOLVE_BENEATH,
+ };
+ struct open_how how2 = {
+ .flags = O_RDONLY | OA2_CRED_INHERIT,
+ .resolve = RESOLVE_BENEATH,
+ };
+ int size = sizeof(struct open_how);
+ int i;
+ int fd;
+ int err;
+ int fds[FD_MAX];
+#define NFD(n) ((n) + 3)
+#define FD_OK(n) (NFD(n) == fds[n])
+
+ if (!openat2_supported) {
+ ksft_print_msg("openat2(2) unsupported\n");
+ return 0;
+ }
+
+ ksft_set_plan(14);
+
+ fds[FD_NORM] = open("/proc/self/maps", O_RDONLY);
+ fds[FD_NCA] = open("/proc/self/maps", O_RDONLY | O_CRED_ALLOW);
+ fds[FD_DIR] = open("/proc/self", O_RDONLY | O_DIRECTORY);
+ fds[FD_DCA] = open("/proc/self", O_RDONLY | O_DIRECTORY | O_CRED_ALLOW);
+ ksft_test_result(FD_OK(FD_NORM), "file open\n");
+ ksft_test_result(FD_OK(FD_NCA), "file open with O_CRED_ALLOW\n");
+ ksft_test_result(FD_OK(FD_DIR), "directory open\n");
+ ksft_test_result(FD_OK(FD_DCA), "directory open with O_CRED_ALLOW\n");
+
+ err = fchdir(fds[FD_DIR]);
+ if (err) {
+ ksft_perror("fchdir() failed");
+ ksft_exit_fail_msg("fchdir\n");
+ return 1;
+ }
+ fd = syscall(SYS_openat2, AT_FDCWD, "maps", &how1, size);
+ ksft_test_result(fd != -1, "AT_FDCWD success\n");
+ close(fd);
+ /* OA2_CRED_INHERIT fails with AT_FDCWD */
+ fd = syscall(SYS_openat2, AT_FDCWD, "maps", &how2, size);
+ ksft_test_result(fd == -1 && errno == EINVAL, "AT_FDCWD EINVAL\n");
+
+ fd = syscall(SYS_openat2, fds[FD_NORM], "maps", &how1, size);
+ ksft_test_result(fd == -1 && errno == ENOTDIR, "regilar file ENOTDIR\n");
+ /* No O_CRED_ALLOW -> EPERM */
+ fd = syscall(SYS_openat2, fds[FD_NORM], "maps", &how2, size);
+ ksft_test_result(fd == -1 && errno == EPERM, "regilar file EPERM\n");
+
+ fd = syscall(SYS_openat2, fds[FD_NCA], "maps", &how1, size);
+ ksft_test_result(fd == -1 && errno == ENOTDIR, "regilar file ENOTDIR\n");
+ fd = syscall(SYS_openat2, fds[FD_NCA], "maps", &how2, size);
+ ksft_test_result(fd == -1 && errno == ENOTDIR, "regilar file ENOTDIR\n");
+
+ fd = syscall(SYS_openat2, fds[FD_DIR], "maps", &how1, size);
+ ksft_test_result(fd != -1, "dir fd success\n");
+ close(fd);
+ /* No O_CRED_ALLOW -> EPERM */
+ fd = syscall(SYS_openat2, fds[FD_DIR], "maps", &how2, size);
+ ksft_test_result(fd == -1 && errno == EPERM, "dir fd EPERM\n");
+
+ fd = syscall(SYS_openat2, fds[FD_DCA], "maps", &how1, size);
+ ksft_test_result(fd != -1, "dir O_CRED_ALLOW fd success\n");
+ close(fd);
+ fd = syscall(SYS_openat2, fds[FD_DCA], "maps", &how2, size);
+ ksft_test_result(fd != -1, "dir O_CRED_ALLOW fd O_CRED_INHERIT success\n");
+ close(fd);
+
+ for (i = 0; i < FD_MAX; i++)
+ close(fds[i]);
+ ksft_finished();
+ return 0;
+}
diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c
index 9024754530b2..5095288fe1ac 100644
--- a/tools/testing/selftests/openat2/openat2_test.c
+++ b/tools/testing/selftests/openat2/openat2_test.c
@@ -28,6 +28,10 @@
#define O_LARGEFILE 0x8000
#endif
+#ifndef OA2_CRED_INHERIT
+#define OA2_CRED_INHERIT (1UL << 28)
+#endif
+
struct open_how_ext {
struct open_how inner;
uint32_t extra1;
@@ -159,7 +163,7 @@ struct flag_test {
int err;
};
-#define NUM_OPENAT2_FLAG_TESTS 25
+#define NUM_OPENAT2_FLAG_TESTS 27
void test_openat2_flags(void)
{
@@ -233,6 +237,12 @@ void test_openat2_flags(void)
{ .name = "invalid how.resolve and O_PATH",
.how.flags = O_PATH,
.how.resolve = 0x1337, .err = -EINVAL },
+ { .name = "invalid how.resolve and OA2_CRED_INHERIT",
+ .how.flags = OA2_CRED_INHERIT,
+ .how.resolve = 0, .err = -EPERM },
+ { .name = "invalid AT_FDCWD and OA2_CRED_INHERIT",
+ .how.flags = OA2_CRED_INHERIT,
+ .how.resolve = 0x08, .err = -EINVAL },
/* currently unknown upper 32 bit rejected. */
{ .name = "currently unknown bit (1 << 63)",
--
2.44.0
^ permalink raw reply related
* [PATCH v6 2/3] open: add O_CRED_ALLOW flag
From: Stas Sergeev @ 2024-04-27 11:24 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche, Arnd Bergmann,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jens Axboe, Kuniyuki Iwashima, Pavel Begunkov, linux-arch, netdev
In-Reply-To: <20240427112451.1609471-1-stsp2@yandex.ru>
This flag prevents an fd from being passed via unix socket, and
makes it to be always closed on exec().
Selftest is added to check for both properties.
It is needed for the subsequent OA2_CRED_INHERIT addition, to work
as an "opt-in" for the new cred-inherit functionality. Without using
O_CRED_ALLOW when opening dir fd, OA2_CRED_INHERIT is going to return
EPERM.
Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Andy Lutomirski <luto@kernel.org>
CC: David Laight <David.Laight@ACULAB.COM>
CC: Arnd Bergmann <arnd@arndb.de>
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: Kuniyuki Iwashima <kuniyu@amazon.com>
CC: Pavel Begunkov <asml.silence@gmail.com>
CC: linux-arch@vger.kernel.org
CC: netdev@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
fs/fcntl.c | 2 +-
fs/file.c | 15 +--
include/linux/fcntl.h | 2 +-
include/uapi/asm-generic/fcntl.h | 5 +
net/core/scm.c | 5 +
tools/testing/selftests/core/Makefile | 2 +-
tools/testing/selftests/core/cred_allow.c | 139 ++++++++++++++++++++++
7 files changed, 160 insertions(+), 10 deletions(-)
create mode 100644 tools/testing/selftests/core/cred_allow.c
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 54cc85d3338e..78c96b1293c2 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1039,7 +1039,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+ BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/file.c b/fs/file.c
index 3b683b9101d8..2a09d5276676 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -827,22 +827,23 @@ void do_close_on_exec(struct files_struct *files)
/* exec unshares first */
spin_lock(&files->file_lock);
for (i = 0; ; i++) {
+ int j;
unsigned long set;
unsigned fd = i * BITS_PER_LONG;
fdt = files_fdtable(files);
if (fd >= fdt->max_fds)
break;
set = fdt->close_on_exec[i];
- if (!set)
- continue;
fdt->close_on_exec[i] = 0;
- for ( ; set ; fd++, set >>= 1) {
- struct file *file;
- if (!(set & 1))
- continue;
- file = fdt->fd[fd];
+ for (j = 0; j < BITS_PER_LONG; j++, fd++, set >>= 1) {
+ struct file *file = fdt->fd[fd];
if (!file)
continue;
+ /* Close all cred-allow files. */
+ if (file->f_flags & O_CRED_ALLOW)
+ set |= 1;
+ if (!(set & 1))
+ continue;
rcu_assign_pointer(fdt->fd[fd], NULL);
__put_unused_fd(files, fd);
spin_unlock(&files->file_lock);
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..e074ee9c1e36 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -10,7 +10,7 @@
(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
O_APPEND | O_NDELAY | O_NONBLOCK | __O_SYNC | O_DSYNC | \
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
- O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+ O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_CRED_ALLOW)
/* List of all valid flags for the how->resolve argument: */
#define VALID_RESOLVE_FLAGS \
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 80f37a0d40d7..9244c54bb933 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -89,6 +89,11 @@
#define __O_TMPFILE 020000000
#endif
+#ifndef O_CRED_ALLOW
+/* On parisc bit 23 is taken. On alpha bit 24 is also taken. Try bit 25. */
+#define O_CRED_ALLOW 0200000000
+#endif
+
/* a horrid kludge trying to make sure that this will fail on old kernels */
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
diff --git a/net/core/scm.c b/net/core/scm.c
index 9cd4b0a01cd6..f54fb0ee9727 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -111,6 +111,11 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct scm_fp_list **fplp)
fput(file);
return -EINVAL;
}
+ /* don't allow files with creds */
+ if (file->f_flags & O_CRED_ALLOW) {
+ fput(file);
+ return -EPERM;
+ }
if (unix_get_socket(file))
fpl->count_unix++;
diff --git a/tools/testing/selftests/core/Makefile b/tools/testing/selftests/core/Makefile
index ce262d097269..347a5a9d3f29 100644
--- a/tools/testing/selftests/core/Makefile
+++ b/tools/testing/selftests/core/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
CFLAGS += -g $(KHDR_INCLUDES)
-TEST_GEN_PROGS := close_range_test
+TEST_GEN_PROGS := close_range_test cred_allow
include ../lib.mk
diff --git a/tools/testing/selftests/core/cred_allow.c b/tools/testing/selftests/core/cred_allow.c
new file mode 100644
index 000000000000..07d533207a2c
--- /dev/null
+++ b/tools/testing/selftests/core/cred_allow.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/socket.h>
+#include <time.h>
+#include <unistd.h>
+#include <string.h>
+
+#include "../kselftest.h"
+
+#ifndef O_CRED_ALLOW
+#define O_CRED_ALLOW 0x2000000
+#endif
+
+enum { FD_NORM, FD_CE, FD_CA, FD_MAX };
+
+static int is_opened(int n)
+{
+ char buf[256];
+
+ snprintf(buf, sizeof(buf), "/proc/self/fd/%d", n);
+ return (access(buf, F_OK) == 0);
+}
+
+/* Sends an FD on a UNIX socket. Returns 0 on success or -errno. */
+static int send_fd(int usock, int fd_tx)
+{
+ union {
+ /* Aligned ancillary data buffer. */
+ char buf[CMSG_SPACE(sizeof(fd_tx))];
+ struct cmsghdr _align;
+ } cmsg_tx = {};
+ char data_tx = '.';
+ struct iovec io = {
+ .iov_base = &data_tx,
+ .iov_len = sizeof(data_tx),
+ };
+ struct msghdr msg = {
+ .msg_iov = &io,
+ .msg_iovlen = 1,
+ .msg_control = &cmsg_tx.buf,
+ .msg_controllen = sizeof(cmsg_tx.buf),
+ };
+ struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
+
+ cmsg->cmsg_len = CMSG_LEN(sizeof(fd_tx));
+ cmsg->cmsg_level = SOL_SOCKET;
+ cmsg->cmsg_type = SCM_RIGHTS;
+ memcpy(CMSG_DATA(cmsg), &fd_tx, sizeof(fd_tx));
+
+ if (sendmsg(usock, &msg, 0) < 0)
+ return -errno;
+ return 0;
+}
+
+int main(int argc, char *argv[], char *env[])
+{
+ int status;
+ int err;
+ pid_t pid;
+ int socket_fds[2];
+ int fds[FD_MAX];
+#define NFD(n) ((n) + 3)
+#define FD_OK(n) (NFD(n) == fds[n] && is_opened(fds[n]))
+
+ if (argc > 1 && strcmp(argv[1], "--child") == 0) {
+ int nfd = 0;
+ ksft_print_msg("we are child\n");
+ ksft_set_plan(3);
+ nfd += is_opened(NFD(FD_NORM));
+ ksft_test_result(nfd == 1, "normal fd opened\n");
+ nfd += is_opened(NFD(FD_CE));
+ ksft_test_result(nfd == 1, "O_CLOEXEC fd closed\n");
+ nfd += is_opened(NFD(FD_CA));
+ ksft_test_result(nfd == 1, "O_CRED_ALLOW fd closed\n");
+ /* exit with non-zero status propagates to parent's failure */
+ ksft_finished();
+ return 0;
+ }
+
+ ksft_set_plan(7);
+
+ fds[FD_NORM] = open("/proc/self/exe", O_RDONLY);
+ fds[FD_CE] = open("/proc/self/exe", O_RDONLY | O_CLOEXEC);
+ fds[FD_CA] = open("/proc/self/exe", O_RDONLY | O_CRED_ALLOW);
+ ksft_test_result(FD_OK(FD_NORM), "regular open\n");
+ ksft_test_result(FD_OK(FD_CE), "O_CLOEXEC open\n");
+ ksft_test_result(FD_OK(FD_CA), "O_CRED_ALLOW open\n");
+
+ err = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, socket_fds);
+ if (err) {
+ ksft_perror("socketpair() failed");
+ ksft_exit_fail_msg("socketpair\n");
+ return 1;
+ }
+ err = send_fd(socket_fds[0], fds[FD_NORM]);
+ ksft_test_result(err == 0, "normal fd sent\n");
+ err = send_fd(socket_fds[0], fds[FD_CE]);
+ ksft_test_result(err == 0, "O_CLOEXEC fd sent\n");
+ err = send_fd(socket_fds[0], fds[FD_CA]);
+ ksft_test_result(err == -EPERM, "O_CRED_ALLOW fd not sent, EPERM\n");
+ close(socket_fds[0]);
+ close(socket_fds[1]);
+
+ pid = fork();
+ if (pid < 0) {
+ ksft_perror("fork() failed");
+ ksft_exit_fail_msg("fork\n");
+ return 1;
+ }
+
+ if (pid == 0) {
+ char *cargv[] = {"cred_allow", "--child", NULL};
+
+ execve("/proc/self/exe", cargv, env);
+ ksft_perror("execve() failed");
+ ksft_exit_fail_msg("execve\n");
+ return 1;
+ }
+
+ if (waitpid(pid, &status, 0) != pid) {
+ ksft_perror("waitpid() failed");
+ ksft_exit_fail_msg("waitpid\n");
+ return 1;
+ }
+ ksft_print_msg("back to parent\n");
+
+ ksft_test_result(status == 0, "child success\n");
+
+ ksft_finished();
+ return 0;
+}
--
2.44.0
^ permalink raw reply related
* [PATCH v6 0/3] implement OA2_CRED_INHERIT flag for openat2()
From: Stas Sergeev @ 2024-04-27 11:24 UTC (permalink / raw)
To: linux-kernel
Cc: Stas Sergeev, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, David Laight, linux-fsdevel,
linux-api, Paolo Bonzini, Christian Göttsche
This patch-set implements the OA2_CRED_INHERIT flag for openat2() syscall.
It is needed to perform an open operation with the creds that were in
effect when the dir_fd was opened, if the dir was opened with O_CRED_ALLOW
flag. This allows the process to pre-open some dirs and switch eUID
(and other UIDs/GIDs) to the less-privileged user, while still retaining
the possibility to open/create files within the pre-opened directory set.
The sand-boxing is security-oriented: symlinks leading outside of a
sand-box are rejected. /proc magic links are rejected. fds opened with
O_CRED_ALLOW are always closed on exec() and cannot be passed via unix
socket.
The more detailed description (including security considerations)
is available in the log messages of individual patches.
Changes in v6:
- it appears open flags bit 23 is already taken on parisc, and bit 24
is taken on alpha. Move O_CRED_ALLOW to bit 25.
- added selftests for both O_CRED_ALLOW and O_CRED_INHERIT additions
Changes in v5:
- rename OA2_INHERIT_CRED to OA2_CRED_INHERIT
- add an "opt-in" flag O_CRED_ALLOW as was suggested by many reviewers
- stop using 64bit types, as suggested by
Christian Brauner <brauner@kernel.org>
- add BUILD_BUG_ON() for VALID_OPENAT2_FLAGS, based on Christian Brauner's
comments
- fixed problems reported by patch-testing bot
- made O_CRED_ALLOW fds not passable via unix sockets and exec(),
based on Christian Brauner's comments
Changes in v4:
- add optimizations suggested by David Laight <David.Laight@ACULAB.COM>
- move security checks to build_open_flags()
- force RESOLVE_NO_MAGICLINKS as suggested by Andy Lutomirski <luto@kernel.org>
Changes in v3:
- partially revert v2 changes to avoid overriding capabilities.
Only the bare minimum is overridden: fsuid, fsgid and group_info.
Document the fact the full cred override is unwanted, as it may
represent an unneeded security risk.
Changes in v2:
- capture full struct cred instead of just fsuid/fsgid.
Suggested by Stefan Metzmacher <metze@samba.org>
CC: Stefan Metzmacher <metze@samba.org>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Andy Lutomirski <luto@kernel.org>
CC: Christian Brauner <brauner@kernel.org>
CC: Jan Kara <jack@suse.cz>
CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Aring <alex.aring@gmail.com>
CC: David Laight <David.Laight@ACULAB.COM>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Christian Göttsche <cgzones@googlemail.com>
--
2.44.0
^ permalink raw reply
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Dan Williams @ 2024-04-27 16:22 UTC (permalink / raw)
To: Greg KH, Harold Johnson
Cc: Jonathan Cameron, Dan Williams, linux-cxl, Sreenivas Bagalkote,
Brett Henning, Sumanesh Samanta, linux-kernel, Davidlohr Bueso,
Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, linuxarm,
linux-api, Lorenzo Pieralisi, Natu, Mahesh
In-Reply-To: <2024042708-outscore-dreadful-2c21@gregkh>
Greg KH wrote:
[..]
> So while innovating at the hardware level is fine, follow the ways that
> everyone has done this for other specification types (USB, PCI, etc.)
> and just allow vendor drivers to provide the information. Don't do this
> in crazy userspace drivers which will circumvent the whole reason we
> have standard kernel/user apis in the first place for these types of
> things.
Right, standard kernel/user apis is the requirement.
The suggestion of opaque vendor passthrough tunnels, and every vendor
ships their custom tool to do what should be common flows, is where this
discussion went off the rails.
^ permalink raw reply
* Re: [PATCH v2] usb: gadget: f_uac2: Expose all string descriptors through configfs.
From: Chris Wulff @ 2024-04-27 16:27 UTC (permalink / raw)
To: Pavel Hofman, linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman, James Gruber, Lee Jones,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org
In-Reply-To: <9b40e148-f3eb-f8f5-bf2d-37a0a0629417@ivitera.com>
>From: Pavel Hofman <pavel.hofman@ivitera.com>
>>>> + p_it_name playback input terminal name
>>>> + p_ot_name playback output terminal name
>>>> + p_fu_name playback function unit name
>>>> + p_alt0_name playback alt mode 0 name
>>>> + p_alt1_name playback alt mode 1 name
>>>
>>> Nacked-by: Pavel Hofman <pavel.hofman@ivitera.com>
...
>If the params in the upper level were to stand as defaults for the
>altsettings (and for the existing altsetting 1 if no specific altset
>subdir configs were given), maybe the naming xxx_alt1_xxx could become a
>bit confusing. E.g. p_altx_name or p_alt_non0_name?
I've been prototyping this a bit to see how it will work. My current configfs
structure looks something like:
(all existing properties)
c_it_name
c_it_ch_name
c_fu_name
c_ot_name
p_it_name
p_it_ch_name
p_fu_name
p_ot_name
num_alt_modes (settable to 2..5 in my prototype)
alt.0
c_alt_name
p_alt_name
alt.1 (for alt.1, alt.2, etc.)
c_alt_name
p_alt_name
c_ssize
p_ssize
(Additional properties here for other things that are settable for each alt mode,
but the only one I've implemented in my prototype so far is sample size.)
This brings up a few questions:
Is a property for setting the number of alt modes preferred, or being able to
make directories like some other things (eg, "mkdir alt.5" would add alt mode 5)?
The former is what I started with, but I am leaning towards the latter as it is a bit
more flexible (you could create alt modes of any index, though I'm not entirely
sure why you'd want to.) It does involve a bit more dynamic memory allocation,
but nothing crazy.
And second, should the alt.x directories go back to the defaults if you remove
and re-create them? I'm assuming it makes sense to do that, just putting it out
there since my current prototype doesn't work that way.
I appreciate your thoughts on this.
-- Chris Wulff
^ permalink raw reply
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Sirius @ 2024-04-28 4:25 UTC (permalink / raw)
To: Dan Williams
Cc: Greg KH, Harold Johnson, Jonathan Cameron, linux-cxl,
Sreenivas Bagalkote, Brett Henning, Sumanesh Samanta,
linux-kernel, Davidlohr Bueso, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, linuxarm, linux-api, Lorenzo Pieralisi,
Natu, Mahesh
In-Reply-To: <662d263dd17c7_b6e0294ab@dwillia2-mobl3.amr.corp.intel.com.notmuch>
In days of yore (Sat, 27 Apr 2024), Dan Williams thus quoth:
> Greg KH wrote:
> [..]
> > So while innovating at the hardware level is fine, follow the ways that
> > everyone has done this for other specification types (USB, PCI, etc.)
> > and just allow vendor drivers to provide the information. Don't do this
> > in crazy userspace drivers which will circumvent the whole reason we
> > have standard kernel/user apis in the first place for these types of
> > things.
>
> Right, standard kernel/user apis is the requirement.
>
> The suggestion of opaque vendor passthrough tunnels, and every vendor
> ships their custom tool to do what should be common flows, is where this
> discussion went off the rails.
One aspect of this is Fabric Management (thinking CXL3 here). It is not
desirable that every vendor of CXL hardware require their own
(proprietary) fabric management software. From a user perspective, that is
absolutely horrible. Users can, and will, mix and match CXL hardware
according to their needs (or wallet), and them having to run multiple
fabric management solutions (which in worst case are conflicting with each
other) to manage things is .. suboptimal.
By all means - innovate - but do it in such a way that interoperability
and manageability is the priority. Special sauce vendor lock-in is a
surefire way to kill CXL where it stands - don't do it.
--
Kind regards,
/S
^ 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