* Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: stsp @ 2024-04-23 22:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <4D2A1543-273F-417F-921B-E9F994FBF2E8@amacapital.net>
23.04.2024 19:44, Andy Lutomirski пишет:
> Also, there are lots of ways that f_cred could be relevant: fsuid/fsgid, effective capabilities and security labels. And it gets more complex if this ever gets extended to support connecting or sending to a socket or if someone opens a device node. Does CAP_SYS_ADMIN carry over?
I posted a v3 where I only override
fsuid, fsgid and group_info.
Capabilities and whatever else are
not overridden to avoid security risks.
Does this address your concern?
Note that I think your other concerns
are already addressed, I just added a
bit more of a description now.
^ permalink raw reply
* [PATCH v3 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: Stas Sergeev @ 2024-04-23 22:46 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
This patch-set implements the OA2_INHERIT_CRED 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. 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 more detailed description (including security considerations)
is available in the log messages of individual patches.
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: 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
* [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag
From: Stas Sergeev @ 2024-04-23 22:46 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240423224615.298045-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.
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.
- To avoid leaking creds across exec, this patch requires O_CLOEXEC
flag on a directory.
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/internal.h | 2 +-
fs/namei.c | 61 ++++++++++++++++++++++++++++++++++--
fs/open.c | 2 +-
include/linux/fcntl.h | 2 ++
include/uapi/linux/openat2.h | 3 ++
5 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 7ca738904e34..692b53b19aad 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -169,7 +169,7 @@ static inline void sb_end_ro_state_change(struct super_block *sb)
* open.c
*/
struct open_flags {
- int open_flag;
+ u64 open_flag;
umode_t mode;
int acc_mode;
int intent;
diff --git a/fs/namei.c b/fs/namei.c
index 2fde2c320ae9..f34ad2b296c7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -586,6 +586,9 @@ struct nameidata {
int dfd;
vfsuid_t dir_vfsuid;
umode_t dir_mode;
+ kuid_t dir_open_fsuid;
+ kgid_t dir_open_fsgid;
+ struct group_info *dir_open_groups;
} __randomize_layout;
#define ND_ROOT_PRESET 1
@@ -695,6 +698,8 @@ static void terminate_walk(struct nameidata *nd)
nd->depth = 0;
nd->path.mnt = NULL;
nd->path.dentry = NULL;
+ if (nd->dir_open_groups)
+ put_group_info(nd->dir_open_groups);
}
/* path_put is needed afterwards regardless of success or failure */
@@ -2414,6 +2419,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
get_fs_pwd(current->fs, &nd->path);
nd->inode = nd->path.dentry->d_inode;
}
+ nd->dir_open_fsuid = current_cred()->fsuid;
+ nd->dir_open_fsgid = current_cred()->fsgid;
+ nd->dir_open_groups = get_current_groups();
} else {
/* Caller must check execute permissions on the starting path component */
struct fd f = fdget_raw(nd->dfd);
@@ -2437,6 +2445,10 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
path_get(&nd->path);
nd->inode = nd->path.dentry->d_inode;
}
+ nd->dir_open_fsuid = f.file->f_cred->fsuid;
+ nd->dir_open_fsgid = f.file->f_cred->fsgid;
+ nd->dir_open_groups = get_group_info(
+ f.file->f_cred->group_info);
fdput(f);
}
@@ -3776,6 +3788,29 @@ static int do_o_path(struct nameidata *nd, unsigned flags, struct file *file)
return error;
}
+static const struct cred *openat2_override_creds(struct nameidata *nd)
+{
+ const struct cred *old_cred;
+ struct cred *override_cred;
+
+ override_cred = prepare_creds();
+ if (!override_cred)
+ return NULL;
+
+ override_cred->fsuid = nd->dir_open_fsuid;
+ override_cred->fsgid = nd->dir_open_fsgid;
+ override_cred->group_info = nd->dir_open_groups;
+
+ override_cred->non_rcu = 1;
+
+ old_cred = override_creds(override_cred);
+
+ /* override_cred() gets its own ref */
+ put_cred(override_cred);
+
+ return old_cred;
+}
+
static struct file *path_openat(struct nameidata *nd,
const struct open_flags *op, unsigned flags)
{
@@ -3794,8 +3829,28 @@ static struct file *path_openat(struct nameidata *nd,
error = do_o_path(nd, flags, file);
} else {
const char *s = path_init(nd, flags);
- file = alloc_empty_file(op->open_flag, current_cred());
- error = PTR_ERR_OR_ZERO(file);
+ const struct cred *old_cred = NULL;
+
+ error = 0;
+ if (op->open_flag & OA2_INHERIT_CRED) {
+ /* Make sure to work only with restricted
+ * look-up modes.
+ */
+ if (!(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT)))
+ error = -EPERM;
+ /* Only work with O_CLOEXEC dirs. */
+ if (!get_close_on_exec(nd->dfd))
+ error = -EPERM;
+
+ if (!error)
+ old_cred = openat2_override_creds(nd);
+ }
+ if (!error) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ error = PTR_ERR_OR_ZERO(file);
+ } else {
+ file = ERR_PTR(error);
+ }
if (!error) {
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
@@ -3803,6 +3858,8 @@ static struct file *path_openat(struct nameidata *nd,
}
if (!error)
error = do_open(nd, file, op);
+ if (old_cred)
+ revert_creds(old_cred);
terminate_walk(nd);
if (IS_ERR(file))
return file;
diff --git a/fs/open.c b/fs/open.c
index ee8460c83c77..6be013182a35 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;
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..b71f8b162102 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)
+#define VALID_OPENAT2_FLAGS (VALID_OPEN_FLAGS | OA2_INHERIT_CRED)
+
/* 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..cdd676a10b62 100644
--- a/include/uapi/linux/openat2.h
+++ b/include/uapi/linux/openat2.h
@@ -40,4 +40,7 @@ struct open_how {
return -EAGAIN if that's not
possible. */
+/* openat2-specific flags go to upper 4 bytes. */
+#define OA2_INHERIT_CRED (1ULL << 32)
+
#endif /* _UAPI_LINUX_OPENAT2_H */
--
2.44.0
^ permalink raw reply related
* [PATCH 1/2] fs: reorganize path_openat()
From: Stas Sergeev @ 2024-04-23 22:46 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240423224615.298045-1-stsp2@yandex.ru>
This patch moves the call to alloc_empty_file() below the call to
path_init(). That changes is needed for the next patch, which adds
a cred override for alloc_empty_file(). The needed cred info is only
available after the call to path_init().
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: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
fs/namei.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index c5b2a25be7d0..2fde2c320ae9 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3782,22 +3782,30 @@ static struct file *path_openat(struct nameidata *nd,
struct file *file;
int error;
- file = alloc_empty_file(op->open_flag, current_cred());
- if (IS_ERR(file))
- return file;
-
- if (unlikely(file->f_flags & __O_TMPFILE)) {
+ if (unlikely(op->open_flag & __O_TMPFILE)) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ if (IS_ERR(file))
+ return file;
error = do_tmpfile(nd, flags, op, file);
- } else if (unlikely(file->f_flags & O_PATH)) {
+ } else if (unlikely(op->open_flag & O_PATH)) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ if (IS_ERR(file))
+ return file;
error = do_o_path(nd, flags, file);
} else {
const char *s = path_init(nd, flags);
- while (!(error = link_path_walk(s, nd)) &&
- (s = open_last_lookups(nd, file, op)) != NULL)
- ;
+ file = alloc_empty_file(op->open_flag, current_cred());
+ error = PTR_ERR_OR_ZERO(file);
+ if (!error) {
+ while (!(error = link_path_walk(s, nd)) &&
+ (s = open_last_lookups(nd, file, op)) != NULL)
+ ;
+ }
if (!error)
error = do_open(nd, file, op);
terminate_walk(nd);
+ if (IS_ERR(file))
+ return file;
}
if (likely(!error)) {
if (likely(file->f_mode & FMODE_OPENED))
--
2.44.0
^ permalink raw reply related
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Sreenivas Bagalkote @ 2024-04-23 22:44 UTC (permalink / raw)
To: Jonathan Cameron, Dan Williams
Cc: linux-cxl, 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
In-Reply-To: <CACX_a4XGLgmQC3cqCmDJnrcnfjQRW4EmV8BZTCC=MgzwYwdhXA@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 16895 bytes --]
Can somebody please at least acknowledge that you are getting my emails?
Thank you
Sreeni
Sreenivas Bagalkote <Sreenivas.Bagalkote@broadcom.com>
Product Planning & Management
Broadcom Datacenter Solutions Group
On Mon, Apr 15, 2024 at 2:09 PM Sreenivas Bagalkote <
sreenivas.bagalkote@broadcom.com> wrote:
> Hello,
>
> >> We need guidance from the community.
>
> >> 1. Datacenter customers must be able to manage PCIe switches in-band.>> 2. Management of switches includes getting health, performance, and error telemetry.>> 3. These telemetry functions are not yet part of the CXL standard>> 4. We built the CCI mailboxes into our PCIe switches per CXL spec and developed our management scheme around them.>> >> If the Linux community does not allow a CXL spec-compliant switch to be>> managed via the CXL spec-defined CCI mailbox, then please guide us on>> the right approach. Please tell us how you propose we manage our switches>> in-band.
>
> I am still looking for your guidance. We need to be able to manage our
> switch via the CCI mailbox. We need to use vendor-defined commands per CXL
> spec.
>
> You talked about whitelisting commands (allow-list) which we agreed to.
> Would you please confirm that you will allow the vendor-defined allow-list
> of commands?
>
> Thank you
> Sreeni
>
> On Wed, Apr 10, 2024 at 5:45 AM Jonathan Cameron <
> Jonathan.Cameron@huawei.com> wrote:
>
>> On Fri, 5 Apr 2024 17:04:34 -0700
>> Dan Williams <dan.j.williams@intel.com> wrote:
>>
>> Hi Dan,
>>
>> > Jonathan Cameron wrote:
>> > > Hi All,
>> > >
>> > > This is has come up in a number of discussions both on list and in
>> private,
>> > > so I wanted to lay out a potential set of rules when deciding whether
>> or not
>> > > to provide a user space interface for a particular feature of CXL
>> Fabric
>> > > Management. The intent is to drive discussion, not to simply tell
>> people
>> > > a set of rules. I've brought this to the public lists as it's a
>> Linux kernel
>> > > policy discussion, not a standards one.
>> > >
>> > > Whilst I'm writing the RFC this my attempt to summarize a possible
>> > > position rather than necessarily being my personal view.
>> > >
>> > > It's a straw man - shoot at it!
>> > >
>> > > Not everyone in this discussion is familiar with relevant kernel or
>> CXL concepts
>> > > so I've provided more info than I normally would.
>> >
>> > Thanks for writing this up Jonathan!
>> >
>> > [..]
>> > > 2) Unfiltered userspace use of mailbox for Fabric Management - BMC
>> kernels
>> > >
>> ==========================================================================
>> > >
>> > > (This would just be a kernel option that we'd advise normal server
>> > > distributions not to turn on. Would be enabled by openBMC etc)
>> > >
>> > > This is fine - there is some work to do, but the switch-cci PCI driver
>> > > will hopefully be ready for upstream merge soon. There is no
>> filtering of
>> > > accesses. Think of this as similar to all the damage you can do via
>> > > MCTP from a BMC. Similarly it is likely that much of the complexity
>> > > of the actual commands will be left to user space tooling:
>> > > https://gitlab.com/jic23/cxl-fmapi-tests has some test examples.
>> > >
>> > > Whether Kconfig help text is strong enough to ensure this only gets
>> > > enabled for BMC targeted distros is an open question we can address
>> > > alongside an updated patch set.
>> >
>> > It is not clear to me that this material makes sense to house in
>> > drivers/ vs tools/ or even out-of-tree just for maintenance burden
>> > relief of keeping the universes separated. What does the Linux kernel
>> > project get out of carrying this in mainline alongside the inband code?
>>
>> I'm not sure what you mean by in band. Aim here was to discuss
>> in-band drivers for switch CCI etc. Same reason from a kernel point of
>> view for why we include embedded drivers. I'll interpret in band
>> as host driven and not inband as FM-API stuff.
>>
>> > I do think the mailbox refactoring to support non-CXL use cases is
>> > interesting, but only so far as refactoring is consumed for inband use
>> > cases like RAS API.
>>
>> If I read this right, I disagree with the 'only so far' bit.
>>
>> In all substantial ways we should support BMC use case of the Linux Kernel
>> at a similar level to how we support forms of Linux Distros. It may
>> not be our target market as developers for particular parts of our
>> companies,
>> but we should not block those who want to support it.
>>
>> We should support them in drivers/ - maybe with example userspace code
>> in tools. Linux distros on BMCs is a big market, there are a number
>> of different distros using (and in some cases contributing to) the
>> upstream kernel. Not everyone is using openBMC so there is not one
>> common place where downstream patches could be carried.
>> From a personal point of view, I like that for the same reasons that
>> I like there being multiple Linux sever focused distros. It's a sign
>> of a healthy ecosystem to have diverse options taking the mainline
>> kernel as their starting point.
>>
>> BMCs are just another embedded market, and like other embedded markets
>> we want to encourage upstream first etc.
>> openBMC has a policy on this:
>> https://github.com/openbmc/docs/blob/master/kernel-development.md
>> "The OpenBMC project maintains a kernel tree for use by the project.
>> The tree's general development policy is that code must be upstream
>> first." There are paths to bypass that for openBMC so it's a little
>> more relaxed than some enterprise distros (today, their policies used
>> to look very similar to this) but we should not be telling
>> them they need to carry support downstream. If we are
>> going to tell them that, we need to be able to point at a major
>> sticking point for maintenance burden. So far I don't see the
>> additional complexity as remotely close reaching that bar.
>>
>> So I think we do want switch-cci support and for that matter the
>> equivalent
>> for MHDs in the upstream kernel.
>>
>> One place I think there is some wiggle room is the taint on use of raw
>> commands. Leaving removal of that for BMC kernels as a patch they need
>> to carry downstream doesn't seem too burdensome. I'm sure they'll push
>> back if it is a problem for them! So I think we can kick that question
>> into the future.
>>
>> Addressing maintenance burden, there is a question of where we split
>> the stack. Ignore MHDs for now (I won't go into why in this forum...)
>>
>> The current proposal is (simplified to ignore some sharing in lookup code
>> etc
>> that I can rip out if we think it might be a long term problem)
>>
>> _____________ _____________________
>> | | | |
>> | Switch CCI | | Type 3 Driver stack|
>> |_____________| |_____________________|
>> |___________________________| Whatever GPU etc
>> _______|_______ _______|______
>> | | | |
>> | CXL MBOX | | RAS API etc |
>> |_______________| |______________|
>> |_____________________________|
>> |
>> _________|______
>> | |
>> | MMPT mbox |
>> |________________|
>>
>> Switch CCI Driver: PCI driver doing everything beyond the CXL mbox
>> specific bit.
>> Type 3 Stack: All the normal stack just with the CXL Mailbox specific
>> stuff factored
>> out. Note we can move different amounts of shared logic in
>> here, but
>> in essence it deals with the extra layer on top of the raw
>> MMPT mbox.
>> MMPT Mbox: Mailbox as per the PCI spec.
>> RAS API: Shared RAS API specific infrastructure used by other drivers.
>>
>> If we see a significant maintenance burden, maybe we duplicate the CXL
>> specific
>> MBOX layer - I can see advantages in that as there is some stuff not
>> relevant
>> to the Switch CCI. There will be some duplication of logic however such
>> as background command support (which is CXL only IIUC) We can even use
>> a difference IOCTL number so the two can diverge if needed in the long
>> run.
>>
>> e.g. If it makes it easier to get upstream, we can merrily duplicated code
>> so that only the bit common with RAS API etc is shared (assuming the
>> actually end up with MMPT, not the CXL mailbox which is what their current
>> publicly available spec talks about and I assume is a pref MMPT left
>> over?)
>>
>> _____________ _____________________
>> | | | |
>> | Switch CCI | | Type 3 Driver stack|
>> |_____________| |_____________________|
>> | | Whatever GPU etc
>> _______|_______ _______|_______ ______|_______
>> | | | | | |
>> | CXL MBOX | | CXL MBOX | | RAS API etc |
>> |_______________| |_______________| |______________|
>> |_____________________________|____________________|
>> |
>> ________|______
>> | |
>> | MMPT mbox |
>> |_______________|
>>
>>
>> > > (On to the one that the "debate" is about)
>> > >
>> > > 3) Unfiltered user space use of mailbox for Fabric Management -
>> Distro kernels
>> > >
>> =============================================================================
>> > > (General purpose Linux Server Distro (Redhat, Suse etc))
>> > >
>> > > This is equivalent of RAW command support on CXL Type 3 memory
>> devices.
>> > > You can enable those in a distro kernel build despite the scary config
>> > > help text, but if you use it the kernel is tainted. The result
>> > > of the taint is to add a flag to bug reports and print a big message
>> to say
>> > > that you've used a feature that might result in you shooting yourself
>> > > in the foot.
>> > >
>> > > The taint is there because software is not at first written to deal
>> with
>> > > everything that can happen smoothly (e.g. surprise removal) It's hard
>> > > to survive some of these events, so is never on the initial feature
>> list
>> > > for any bus, so this flag is just to indicate we have entered a world
>> > > where almost all bets are off wrt to stability. We might not know
>> what
>> > > a command does so we can't assess the impact (and no one trusts vendor
>> > > commands to report affects right in the Command Effects Log - which
>> > > in theory tells you if a command can result problems).
>> >
>> > That is a secondary reason that the taint is there. Yes, it helps
>> > upstream not waste their time on bug reports from proprietary use cases,
>> > but the effect of that is to make "raw" command mode unattractive for
>> > deploying solutions at scale. It clarifies that this interface is a
>> > debug-tool that enterprise environment need not worry about.
>> >
>> > The more salient reason for the taint, speaking only for myself as a
>> > Linux kernel community member not for $employer, is to encourage open
>> > collaboration. Take firmware-update for example that is a standard
>> > command with known side effects that is inaccessible via the ioctl()
>> > path. It is placed behind an ABI that is easier to maintain and reason
>> > about. Everyone has the firmware update tool if they have the 'cat'
>> > command. Distros appreciate the fact that they do not need ship yet
>> > another vendor device-update tool, vendors get free tooling and end
>> > users also appreciate one flow for all devices.
>> >
>> > As I alluded here [1], I am not against innovation outside of the
>> > specification, but it needs to be open, and it needs to plausibly become
>> > if not a de jure standard at least a de facto standard.
>> >
>> > [1]:
>> https://lore.kernel.org/all/CAPcyv4gDShAYih5iWabKg_eTHhuHm54vEAei8ZkcmHnPp3B0cw@mail.gmail.com/
>>
>> Agree with all this.
>>
>> >
>> > > A concern was raised about GAE/FAST/LDST tables for CXL Fabrics
>> > > (a r3.1 feature) but, as I understand it, these are intended for a
>> > > host to configure and should not have side effects on other hosts?
>> > > My working assumption is that the kernel driver stack will handle
>> > > these (once we catch up with the current feature backlog!) Currently
>> > > we have no visibility of what the OS driver stack for a fabrics will
>> > > actually look like - the spec is just the starting point for that.
>> > > (patches welcome ;)
>> > >
>> > > The various CXL upstream developers and maintainers may have
>> > > differing views of course, but my current understanding is we want
>> > > to support 1 and 2, but are very resistant to 3!
>> >
>> > 1, yes, 2, need to see the patches, and agree on 3.
>>
>> If we end up with top architecture of the diagrams above, 2 will look
>> pretty
>> similar to last version of the switch-cci patches. So raw commands only
>> + taint.
>> Factoring out MMPT is another layer that doesn't make that much
>> difference in
>> practice to this discussion. Good to have, but the reuse here would be
>> one layer
>> above that.
>>
>> Or we just say go for second proposed architecture and 0 impact on the
>> CXL specific code, just reuse of the MMPT layer. I'd imagine people will
>> get
>> grumpy on code duplication (and we'll spend years rejecting patch sets
>> that
>> try to share the cdoe) but there should be no maintenance burden as
>> a result.
>>
>> >
>> > > General Notes
>> > > =============
>> > >
>> > > One side aspect of why we really don't like unfiltered userspace
>> access to any
>> > > of these devices is that people start building non standard hacks in
>> and we
>> > > lose the ecosystem advantages. Forcing a considered discussion +
>> patches
>> > > to let a particular command be supported, drives standardization.
>> >
>> > Like I said above, I think this is not a side aspect. It is fundamental
>> > to the viability Linux as a project. This project only works because
>> > organizations with competing goals realize they need some common
>> > infrastructure and that there is little to be gained by competing on the
>> > commons.
>> >
>> > >
>> https://lore.kernel.org/linux-cxl/CAPcyv4gDShAYih5iWabKg_eTHhuHm54vEAei8ZkcmHnPp3B0cw@mail.gmail.com/
>> > > provides some history on vendor specific extensions and why in
>> general we
>> > > won't support them upstream.
>> >
>> > Oh, you linked my writeup... I will leave the commentary I added here
>> in case
>> > restating it helps.
>> >
>> > > To address another question raised in an earlier discussion:
>> > > Putting these Fabric Management interfaces behind guard rails of some
>> type
>> > > (e.g. CONFIG_IM_A_BMC_AND_CAN_MAKE_A_MESS) does not encourage the risk
>> > > of non standard interfaces, because we will be even less likely to
>> accept
>> > > those upstream!
>> > >
>> > > If anyone needs more details on any aspect of this please ask.
>> > > There are a lot of things involved and I've only tried to give a
>> fairly
>> > > minimal illustration to drive the discussion. I may well have missed
>> > > something crucial.
>> >
>> > You captured it well, and this is open source so I may have missed
>> > something crucial as well.
>> >
>>
>> Thanks for detailed reply!
>>
>> 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 #1.2: Type: text/html, Size: 22034 bytes --]
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4230 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: stsp @ 2024-04-23 18:05 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <4D2A1543-273F-417F-921B-E9F994FBF2E8@amacapital.net>
23.04.2024 19:44, Andy Lutomirski пишет:
>> On Apr 23, 2024, at 4:02 AM, Stas Sergeev <stsp2@yandex.ru> wrote:
>>
>> This patch-set implements the OA2_INHERIT_CRED 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. 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.
> I like the concept, as it’s a sort of move toward a capability system. But I think that making a dirfd into this sort of capability would need to be much more explicit. Right now, any program could do this entirely by accident, and applying OA2_INHERIT_CRED to an fd fished out of /proc seems hazardous.
Could you please clarify this a bit?
I think if you open someone else's
fd via /proc, then your current creds
would be stored in a struct file, rather
than the creds of the process from
which you open. I don't think creds
can be stolen that way, as I suppose
opening via proc is similar to open
via some symlink.
Or is there some magic going on
so that the process's creds can
actually be leaked this way?
> So perhaps if an open file description for a directory could have something like FMODE_CRED, and if OA2_INHERIT_CRED also blocked .., magic links, symlinks to anywhere above the dirfd (or maybe all symlinks) and absolute path lookups, then this would be okay.
This is already there.
My fault is that I put a short description
in a cover letter and a long description
in a patch itself. I should probably swap
them, as you only read the cover letter.
My patch takes care about possible escape
scenarios by working only with restricted
lookup modes: RESOLVE_BENEATH, RESOLVE_IN_ROOT.
I made sure that symlinks leading outside
of a sandbox, are rejected.
Also note that my patch requires O_CLOEXEC
on a dir_fd to avoid the cred leakage over
exec syscall.
> Also, there are lots of ways that f_cred could be relevant: fsuid/fsgid, effective capabilities and security labels. And it gets more complex if this ever gets extended to support connecting or sending to a socket or if someone opens a device node. Does CAP_SYS_ADMIN carry over?
Hmm, I don't know.
The v1 version of a patch only changed
fsuid/fsgid. Stefan Metzmacher suggested
to use the full creds instead, but I haven't
considered the implications of that change
just yet.
So if using the full creds is too much because
it can carry things like CAP_SYS_ADMIN, then
should I get back to v1 and only change
fsuid/fsgid?
^ permalink raw reply
* Re: [PATCH v2] usb: gadget: f_uac2: Expose all string descriptors through configfs.
From: Chris Wulff @ 2024-04-23 17:22 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: <c9928edb-8b2d-1948-40b8-c16e34cea3e2@ivitera.com>
> From: Pavel Hofman <pavel.hofman@ivitera.com>
> Sent: Tuesday, April 23, 2024 11:38 AM
> > + 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>
>
> I am not sure adding a numbered parameter for every additional alt mode
> is a way to go for the future. I am not that much concerned about UAC1,
> but IMO (at least) in UAC2 the configuration method should be flexible
> for more alt setttings. I can see use cases with many more altsettings.
>
> My proposal for adding more alt settings
> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/35be4668-58d3-894a-72cf-de1afaacae45@ivitera.com/__;!!HBnMciuwfVSXJQ!TYg7j7-fh3eZAzPfiONi2lo54mf2qsWtpG0nwdaQwSqd1nGdKkTDN8o6_lSIWlWPtHoc-2Nz1KCbRhiXJnzXO8Ku1w$
> suggested using lists to existing parameters where each item would
> correspond to the alt setting of the same index (+1). That would allow
> using more altsettings easily, without having to add parameters to the
> source code and adding configfs params. I received no feedback. I do not
> push the param list proposal, but I am convinced an acceptable solution
> should be discussed thoroughly by the UAC2 gadget stakeholders.
>
> I am afraid that once p_alt1_name/c_alt1_name params are accepted, there
> will be no way back because subsequent removal of configfs params could
> be viewed as a regression for users.
I have been thinking about this as well. The alt names are slightly different than the rest of the settings
since they also include alt mode 0. I was thinking p/c_alt1_name could be expanded to the array so
that the entries line up with the other settings and don't have an extra entry for alt 0. Perhaps a different
name would make more sense.
Along those lines, I didn't see any gadget drivers using an array of strings for anything, which is also why
I didn't try to do anything here that merged alt0/1 names into an array. If we were to do an array of strings
I'm not sure what the best separator would be. Maybe ";"? The rates array uses ",".
This patch only exposes the existing strings to make them configurable, but I don't want to do anything
that would preclude a nice interface for extra alt modes.
-- Chris Wulff
^ permalink raw reply
* Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: Andy Lutomirski @ 2024-04-23 16:44 UTC (permalink / raw)
To: Stas Sergeev
Cc: linux-kernel, Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Christian Brauner, Jan Kara, Jeff Layton,
Chuck Lever, Alexander Aring, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240423110148.13114-1-stsp2@yandex.ru>
> On Apr 23, 2024, at 4:02 AM, Stas Sergeev <stsp2@yandex.ru> wrote:
>
> This patch-set implements the OA2_INHERIT_CRED 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. 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.
I like the concept, as it’s a sort of move toward a capability system. But I think that making a dirfd into this sort of capability would need to be much more explicit. Right now, any program could do this entirely by accident, and applying OA2_INHERIT_CRED to an fd fished out of /proc seems hazardous.
So perhaps if an open file description for a directory could have something like FMODE_CRED, and if OA2_INHERIT_CRED also blocked .., magic links, symlinks to anywhere above the dirfd (or maybe all symlinks) and absolute path lookups, then this would be okay.
Also, there are lots of ways that f_cred could be relevant: fsuid/fsgid, effective capabilities and security labels. And it gets more complex if this ever gets extended to support connecting or sending to a socket or if someone opens a device node. Does CAP_SYS_ADMIN carry over?
>
> 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: 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>
>
> Stas Sergeev (2):
> fs: reorganize path_openat()
> openat2: add OA2_INHERIT_CRED flag
>
> fs/internal.h | 2 +-
> fs/namei.c | 52 +++++++++++++++++++++++++++++-------
> fs/open.c | 2 +-
> include/linux/fcntl.h | 2 ++
> include/uapi/linux/openat2.h | 3 +++
> 5 files changed, 50 insertions(+), 11 deletions(-)
>
> --
> 2.44.0
>
>
^ permalink raw reply
* Re: [PATCH v2] usb: gadget: f_uac2: Expose all string descriptors through configfs.
From: Pavel Hofman @ 2024-04-23 15:38 UTC (permalink / raw)
To: Chris Wulff, 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: <CO1PR17MB54195BE778868AFDFE2DCB36E1112@CO1PR17MB5419.namprd17.prod.outlook.com>
On 23. 04. 24 16:09, Chris Wulff wrote:
> This makes all string descriptors configurable for the UAC2 gadget
> so the user can configure names of terminals/controls/alt modes.
>
> Signed-off-by: Chris Wulff <chris.wulff@biamp.com>
> ---
> v2: Improved naming of parameters to be mode user friendly. Added documentation.
> v1: https://lore.kernel.org/linux-usb/CO1PR17MB5419B50F94A0014647542931E10D2@CO1PR17MB5419.namprd17.prod.outlook.com/
>
> .../ABI/testing/configfs-usb-gadget-uac2 | 13 +++
> Documentation/usb/gadget-testing.rst | 13 +++
> drivers/usb/gadget/function/f_uac2.c | 80 +++++++++++++++----
> drivers/usb/gadget/function/u_uac2.h | 17 +++-
> 4 files changed, 105 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> index a2bf4fd82a5b..250f8eeb8eab 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> @@ -35,6 +35,19 @@ Description:
> req_number the number of pre-allocated requests
> for both capture and playback
> function_name name of the interface
> + if_ctrl_name topology control name
> + clksrc_in_name input clock name
> + clksrc_out_name output clock name
> + 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>
I am not sure adding a numbered parameter for every additional alt mode
is a way to go for the future. I am not that much concerned about UAC1,
but IMO (at least) in UAC2 the configuration method should be flexible
for more alt setttings. I can see use cases with many more altsettings.
My proposal for adding more alt settings
https://lore.kernel.org/linux-usb/35be4668-58d3-894a-72cf-de1afaacae45@ivitera.com/
suggested using lists to existing parameters where each item would
correspond to the alt setting of the same index (+1). That would allow
using more altsettings easily, without having to add parameters to the
source code and adding configfs params. I received no feedback. I do not
push the param list proposal, but I am convinced an acceptable solution
should be discussed thoroughly by the UAC2 gadget stakeholders.
I am afraid that once p_alt1_name/c_alt1_name params are accepted, there
will be no way back because subsequent removal of configfs params could
be viewed as a regression for users.
Thanks a lot for considering,
Pavel.
> + c_it_name capture input terminal name
> + c_ot_name capture output terminal name
> + c_fu_name capture functional unit name
> + c_alt0_name capture alt mode 0 name
> + c_alt1_name capture alt mode 1 name
> c_terminal_type code of the capture terminal type
> p_terminal_type code of the playback terminal type> ===================== =======================================
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index b086c7ab72f0..1a11d3b3bb88 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -765,6 +765,19 @@ The uac2 function provides these attributes in its function directory:
> req_number the number of pre-allocated request for both capture
> and playback
> function_name name of the interface
> + if_ctrl_name topology control name
> + clksrc_in_name input clock name
> + clksrc_out_name output clock name
> + 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
> + c_it_name capture input terminal name
> + c_ot_name capture output terminal name
> + c_fu_name capture functional unit name
> + c_alt0_name capture alt mode 0 name
> + c_alt1_name capture alt mode 1 name
> c_terminal_type code of the capture terminal type
> p_terminal_type code of the playback terminal type
> ================ ====================================================
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index 383f6854cfec..5ca7ecdb6e60 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -104,25 +104,10 @@ enum {
> STR_AS_OUT_ALT1,
> STR_AS_IN_ALT0,
> STR_AS_IN_ALT1,
> + NUM_STR_DESCRIPTORS,
> };
>
> -static struct usb_string strings_fn[] = {
> - /* [STR_ASSOC].s = DYNAMIC, */
> - [STR_IF_CTRL].s = "Topology Control",
> - [STR_CLKSRC_IN].s = "Input Clock",
> - [STR_CLKSRC_OUT].s = "Output Clock",
> - [STR_USB_IT].s = "USBH Out",
> - [STR_IO_IT].s = "USBD Out",
> - [STR_USB_OT].s = "USBH In",
> - [STR_IO_OT].s = "USBD In",
> - [STR_FU_IN].s = "Capture Volume",
> - [STR_FU_OUT].s = "Playback Volume",
> - [STR_AS_OUT_ALT0].s = "Playback Inactive",
> - [STR_AS_OUT_ALT1].s = "Playback Active",
> - [STR_AS_IN_ALT0].s = "Capture Inactive",
> - [STR_AS_IN_ALT1].s = "Capture Active",
> - { },
> -};
> +static struct usb_string strings_fn[NUM_STR_DESCRIPTORS + 1] = {};
>
> static const char *const speed_names[] = {
> [USB_SPEED_UNKNOWN] = "UNKNOWN",
> @@ -1049,6 +1034,21 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
> return ret;
>
> strings_fn[STR_ASSOC].s = uac2_opts->function_name;
> + strings_fn[STR_IF_CTRL].s = uac2_opts->if_ctrl_name;
> + strings_fn[STR_CLKSRC_IN].s = uac2_opts->clksrc_in_name;
> + strings_fn[STR_CLKSRC_OUT].s = uac2_opts->clksrc_out_name;
> +
> + strings_fn[STR_USB_IT].s = uac2_opts->p_it_name;
> + strings_fn[STR_IO_OT].s = uac2_opts->p_ot_name;
> + strings_fn[STR_FU_OUT].s = uac2_opts->p_fu_name;
> + strings_fn[STR_AS_OUT_ALT0].s = uac2_opts->p_alt0_name;
> + strings_fn[STR_AS_OUT_ALT1].s = uac2_opts->p_alt1_name;
> +
> + strings_fn[STR_IO_IT].s = uac2_opts->c_it_name;
> + strings_fn[STR_USB_OT].s = uac2_opts->c_ot_name;
> + strings_fn[STR_FU_IN].s = uac2_opts->c_fu_name;
> + strings_fn[STR_AS_IN_ALT0].s = uac2_opts->c_alt0_name;
> + strings_fn[STR_AS_IN_ALT1].s = uac2_opts->c_alt1_name;
>
> us = usb_gstrings_attach(cdev, fn_strings, ARRAY_SIZE(strings_fn));
> if (IS_ERR(us))
> @@ -2097,10 +2097,26 @@ UAC2_ATTRIBUTE(s16, c_volume_max);
> UAC2_ATTRIBUTE(s16, c_volume_res);
> UAC2_ATTRIBUTE(u32, fb_max);
> UAC2_ATTRIBUTE_STRING(function_name);
> +UAC2_ATTRIBUTE_STRING(if_ctrl_name);
> +UAC2_ATTRIBUTE_STRING(clksrc_in_name);
> +UAC2_ATTRIBUTE_STRING(clksrc_out_name);
> +
> +UAC2_ATTRIBUTE_STRING(p_it_name);
> +UAC2_ATTRIBUTE_STRING(p_ot_name);
> +UAC2_ATTRIBUTE_STRING(p_fu_name);
> +UAC2_ATTRIBUTE_STRING(p_alt0_name);
> +UAC2_ATTRIBUTE_STRING(p_alt1_name);
> +
> +UAC2_ATTRIBUTE_STRING(c_it_name);
> +UAC2_ATTRIBUTE_STRING(c_ot_name);
> +UAC2_ATTRIBUTE_STRING(c_fu_name);
> +UAC2_ATTRIBUTE_STRING(c_alt0_name);
> +UAC2_ATTRIBUTE_STRING(c_alt1_name);
>
> UAC2_ATTRIBUTE(s16, p_terminal_type);
> UAC2_ATTRIBUTE(s16, c_terminal_type);
>
> +
> static struct configfs_attribute *f_uac2_attrs[] = {
> &f_uac2_opts_attr_p_chmask,
> &f_uac2_opts_attr_p_srate,
> @@ -2127,6 +2143,21 @@ static struct configfs_attribute *f_uac2_attrs[] = {
> &f_uac2_opts_attr_c_volume_res,
>
> &f_uac2_opts_attr_function_name,
> + &f_uac2_opts_attr_if_ctrl_name,
> + &f_uac2_opts_attr_clksrc_in_name,
> + &f_uac2_opts_attr_clksrc_out_name,
> +
> + &f_uac2_opts_attr_p_it_name,
> + &f_uac2_opts_attr_p_ot_name,
> + &f_uac2_opts_attr_p_fu_name,
> + &f_uac2_opts_attr_p_alt0_name,
> + &f_uac2_opts_attr_p_alt1_name,
> +
> + &f_uac2_opts_attr_c_it_name,
> + &f_uac2_opts_attr_c_ot_name,
> + &f_uac2_opts_attr_c_fu_name,
> + &f_uac2_opts_attr_c_alt0_name,
> + &f_uac2_opts_attr_c_alt1_name,
>
> &f_uac2_opts_attr_p_terminal_type,
> &f_uac2_opts_attr_c_terminal_type,
> @@ -2188,6 +2219,21 @@ static struct usb_function_instance *afunc_alloc_inst(void)
> opts->fb_max = FBACK_FAST_MAX;
>
> scnprintf(opts->function_name, sizeof(opts->function_name), "Source/Sink");
> + scnprintf(opts->if_ctrl_name, sizeof(opts->if_ctrl_name), "Topology Control");
> + scnprintf(opts->clksrc_in_name, sizeof(opts->clksrc_in_name), "Input Clock");
> + scnprintf(opts->clksrc_out_name, sizeof(opts->clksrc_out_name), "Output Clock");
> +
> + scnprintf(opts->p_it_name, sizeof(opts->p_it_name), "USBH Out");
> + scnprintf(opts->p_ot_name, sizeof(opts->p_ot_name), "USBD In");
> + scnprintf(opts->p_fu_name, sizeof(opts->p_fu_name), "Playback Volume");
> + scnprintf(opts->p_alt0_name, sizeof(opts->p_alt0_name), "Playback Inactive");
> + scnprintf(opts->p_alt1_name, sizeof(opts->p_alt1_name), "Playback Active");
> +
> + scnprintf(opts->c_it_name, sizeof(opts->c_it_name), "USBD Out");
> + scnprintf(opts->c_ot_name, sizeof(opts->c_ot_name), "USBH In");
> + scnprintf(opts->c_fu_name, sizeof(opts->c_fu_name), "Capture Volume");
> + scnprintf(opts->c_alt0_name, sizeof(opts->c_alt0_name), "Capture Inactive");
> + scnprintf(opts->c_alt1_name, sizeof(opts->c_alt1_name), "Capture Active");
>
> opts->p_terminal_type = UAC2_DEF_P_TERM_TYPE;
> opts->c_terminal_type = UAC2_DEF_C_TERM_TYPE;
> diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
> index 5e81bdd6c5fb..e697d35a1759 100644
> --- a/drivers/usb/gadget/function/u_uac2.h
> +++ b/drivers/usb/gadget/function/u_uac2.h
> @@ -68,7 +68,22 @@ struct f_uac2_opts {
> int fb_max;
> bool bound;
>
> - char function_name[32];
> + char function_name[USB_MAX_STRING_LEN];
> + char if_ctrl_name[USB_MAX_STRING_LEN];
> + char clksrc_in_name[USB_MAX_STRING_LEN];
> + char clksrc_out_name[USB_MAX_STRING_LEN];
> +
> + char p_it_name[USB_MAX_STRING_LEN];
> + char p_ot_name[USB_MAX_STRING_LEN];
> + char p_fu_name[USB_MAX_STRING_LEN];
> + char p_alt0_name[USB_MAX_STRING_LEN];
> + char p_alt1_name[USB_MAX_STRING_LEN];
> +
> + char c_it_name[USB_MAX_STRING_LEN];
> + char c_ot_name[USB_MAX_STRING_LEN];
> + char c_fu_name[USB_MAX_STRING_LEN];
> + char c_alt0_name[USB_MAX_STRING_LEN];
> + char c_alt1_name[USB_MAX_STRING_LEN];
>
> s16 p_terminal_type;
> s16 c_terminal_type;
^ permalink raw reply
* [PATCH v2] usb: gadget: f_uac2: Expose all string descriptors through configfs.
From: Chris Wulff @ 2024-04-23 14:09 UTC (permalink / raw)
To: linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman, James Gruber, Lee Jones,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Chris Wulff
This makes all string descriptors configurable for the UAC2 gadget
so the user can configure names of terminals/controls/alt modes.
Signed-off-by: Chris Wulff <chris.wulff@biamp.com>
---
v2: Improved naming of parameters to be mode user friendly. Added documentation.
v1: https://lore.kernel.org/linux-usb/CO1PR17MB5419B50F94A0014647542931E10D2@CO1PR17MB5419.namprd17.prod.outlook.com/
.../ABI/testing/configfs-usb-gadget-uac2 | 13 +++
Documentation/usb/gadget-testing.rst | 13 +++
drivers/usb/gadget/function/f_uac2.c | 80 +++++++++++++++----
drivers/usb/gadget/function/u_uac2.h | 17 +++-
4 files changed, 105 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
index a2bf4fd82a5b..250f8eeb8eab 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
@@ -35,6 +35,19 @@ Description:
req_number the number of pre-allocated requests
for both capture and playback
function_name name of the interface
+ if_ctrl_name topology control name
+ clksrc_in_name input clock name
+ clksrc_out_name output clock name
+ 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
+ c_it_name capture input terminal name
+ c_ot_name capture output terminal name
+ c_fu_name capture functional unit name
+ c_alt0_name capture alt mode 0 name
+ c_alt1_name capture alt mode 1 name
c_terminal_type code of the capture terminal type
p_terminal_type code of the playback terminal type
===================== =======================================
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index b086c7ab72f0..1a11d3b3bb88 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -765,6 +765,19 @@ The uac2 function provides these attributes in its function directory:
req_number the number of pre-allocated request for both capture
and playback
function_name name of the interface
+ if_ctrl_name topology control name
+ clksrc_in_name input clock name
+ clksrc_out_name output clock name
+ 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
+ c_it_name capture input terminal name
+ c_ot_name capture output terminal name
+ c_fu_name capture functional unit name
+ c_alt0_name capture alt mode 0 name
+ c_alt1_name capture alt mode 1 name
c_terminal_type code of the capture terminal type
p_terminal_type code of the playback terminal type
================ ====================================================
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 383f6854cfec..5ca7ecdb6e60 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -104,25 +104,10 @@ enum {
STR_AS_OUT_ALT1,
STR_AS_IN_ALT0,
STR_AS_IN_ALT1,
+ NUM_STR_DESCRIPTORS,
};
-static struct usb_string strings_fn[] = {
- /* [STR_ASSOC].s = DYNAMIC, */
- [STR_IF_CTRL].s = "Topology Control",
- [STR_CLKSRC_IN].s = "Input Clock",
- [STR_CLKSRC_OUT].s = "Output Clock",
- [STR_USB_IT].s = "USBH Out",
- [STR_IO_IT].s = "USBD Out",
- [STR_USB_OT].s = "USBH In",
- [STR_IO_OT].s = "USBD In",
- [STR_FU_IN].s = "Capture Volume",
- [STR_FU_OUT].s = "Playback Volume",
- [STR_AS_OUT_ALT0].s = "Playback Inactive",
- [STR_AS_OUT_ALT1].s = "Playback Active",
- [STR_AS_IN_ALT0].s = "Capture Inactive",
- [STR_AS_IN_ALT1].s = "Capture Active",
- { },
-};
+static struct usb_string strings_fn[NUM_STR_DESCRIPTORS + 1] = {};
static const char *const speed_names[] = {
[USB_SPEED_UNKNOWN] = "UNKNOWN",
@@ -1049,6 +1034,21 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
return ret;
strings_fn[STR_ASSOC].s = uac2_opts->function_name;
+ strings_fn[STR_IF_CTRL].s = uac2_opts->if_ctrl_name;
+ strings_fn[STR_CLKSRC_IN].s = uac2_opts->clksrc_in_name;
+ strings_fn[STR_CLKSRC_OUT].s = uac2_opts->clksrc_out_name;
+
+ strings_fn[STR_USB_IT].s = uac2_opts->p_it_name;
+ strings_fn[STR_IO_OT].s = uac2_opts->p_ot_name;
+ strings_fn[STR_FU_OUT].s = uac2_opts->p_fu_name;
+ strings_fn[STR_AS_OUT_ALT0].s = uac2_opts->p_alt0_name;
+ strings_fn[STR_AS_OUT_ALT1].s = uac2_opts->p_alt1_name;
+
+ strings_fn[STR_IO_IT].s = uac2_opts->c_it_name;
+ strings_fn[STR_USB_OT].s = uac2_opts->c_ot_name;
+ strings_fn[STR_FU_IN].s = uac2_opts->c_fu_name;
+ strings_fn[STR_AS_IN_ALT0].s = uac2_opts->c_alt0_name;
+ strings_fn[STR_AS_IN_ALT1].s = uac2_opts->c_alt1_name;
us = usb_gstrings_attach(cdev, fn_strings, ARRAY_SIZE(strings_fn));
if (IS_ERR(us))
@@ -2097,10 +2097,26 @@ UAC2_ATTRIBUTE(s16, c_volume_max);
UAC2_ATTRIBUTE(s16, c_volume_res);
UAC2_ATTRIBUTE(u32, fb_max);
UAC2_ATTRIBUTE_STRING(function_name);
+UAC2_ATTRIBUTE_STRING(if_ctrl_name);
+UAC2_ATTRIBUTE_STRING(clksrc_in_name);
+UAC2_ATTRIBUTE_STRING(clksrc_out_name);
+
+UAC2_ATTRIBUTE_STRING(p_it_name);
+UAC2_ATTRIBUTE_STRING(p_ot_name);
+UAC2_ATTRIBUTE_STRING(p_fu_name);
+UAC2_ATTRIBUTE_STRING(p_alt0_name);
+UAC2_ATTRIBUTE_STRING(p_alt1_name);
+
+UAC2_ATTRIBUTE_STRING(c_it_name);
+UAC2_ATTRIBUTE_STRING(c_ot_name);
+UAC2_ATTRIBUTE_STRING(c_fu_name);
+UAC2_ATTRIBUTE_STRING(c_alt0_name);
+UAC2_ATTRIBUTE_STRING(c_alt1_name);
UAC2_ATTRIBUTE(s16, p_terminal_type);
UAC2_ATTRIBUTE(s16, c_terminal_type);
+
static struct configfs_attribute *f_uac2_attrs[] = {
&f_uac2_opts_attr_p_chmask,
&f_uac2_opts_attr_p_srate,
@@ -2127,6 +2143,21 @@ static struct configfs_attribute *f_uac2_attrs[] = {
&f_uac2_opts_attr_c_volume_res,
&f_uac2_opts_attr_function_name,
+ &f_uac2_opts_attr_if_ctrl_name,
+ &f_uac2_opts_attr_clksrc_in_name,
+ &f_uac2_opts_attr_clksrc_out_name,
+
+ &f_uac2_opts_attr_p_it_name,
+ &f_uac2_opts_attr_p_ot_name,
+ &f_uac2_opts_attr_p_fu_name,
+ &f_uac2_opts_attr_p_alt0_name,
+ &f_uac2_opts_attr_p_alt1_name,
+
+ &f_uac2_opts_attr_c_it_name,
+ &f_uac2_opts_attr_c_ot_name,
+ &f_uac2_opts_attr_c_fu_name,
+ &f_uac2_opts_attr_c_alt0_name,
+ &f_uac2_opts_attr_c_alt1_name,
&f_uac2_opts_attr_p_terminal_type,
&f_uac2_opts_attr_c_terminal_type,
@@ -2188,6 +2219,21 @@ static struct usb_function_instance *afunc_alloc_inst(void)
opts->fb_max = FBACK_FAST_MAX;
scnprintf(opts->function_name, sizeof(opts->function_name), "Source/Sink");
+ scnprintf(opts->if_ctrl_name, sizeof(opts->if_ctrl_name), "Topology Control");
+ scnprintf(opts->clksrc_in_name, sizeof(opts->clksrc_in_name), "Input Clock");
+ scnprintf(opts->clksrc_out_name, sizeof(opts->clksrc_out_name), "Output Clock");
+
+ scnprintf(opts->p_it_name, sizeof(opts->p_it_name), "USBH Out");
+ scnprintf(opts->p_ot_name, sizeof(opts->p_ot_name), "USBD In");
+ scnprintf(opts->p_fu_name, sizeof(opts->p_fu_name), "Playback Volume");
+ scnprintf(opts->p_alt0_name, sizeof(opts->p_alt0_name), "Playback Inactive");
+ scnprintf(opts->p_alt1_name, sizeof(opts->p_alt1_name), "Playback Active");
+
+ scnprintf(opts->c_it_name, sizeof(opts->c_it_name), "USBD Out");
+ scnprintf(opts->c_ot_name, sizeof(opts->c_ot_name), "USBH In");
+ scnprintf(opts->c_fu_name, sizeof(opts->c_fu_name), "Capture Volume");
+ scnprintf(opts->c_alt0_name, sizeof(opts->c_alt0_name), "Capture Inactive");
+ scnprintf(opts->c_alt1_name, sizeof(opts->c_alt1_name), "Capture Active");
opts->p_terminal_type = UAC2_DEF_P_TERM_TYPE;
opts->c_terminal_type = UAC2_DEF_C_TERM_TYPE;
diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
index 5e81bdd6c5fb..e697d35a1759 100644
--- a/drivers/usb/gadget/function/u_uac2.h
+++ b/drivers/usb/gadget/function/u_uac2.h
@@ -68,7 +68,22 @@ struct f_uac2_opts {
int fb_max;
bool bound;
- char function_name[32];
+ char function_name[USB_MAX_STRING_LEN];
+ char if_ctrl_name[USB_MAX_STRING_LEN];
+ char clksrc_in_name[USB_MAX_STRING_LEN];
+ char clksrc_out_name[USB_MAX_STRING_LEN];
+
+ char p_it_name[USB_MAX_STRING_LEN];
+ char p_ot_name[USB_MAX_STRING_LEN];
+ char p_fu_name[USB_MAX_STRING_LEN];
+ char p_alt0_name[USB_MAX_STRING_LEN];
+ char p_alt1_name[USB_MAX_STRING_LEN];
+
+ char c_it_name[USB_MAX_STRING_LEN];
+ char c_ot_name[USB_MAX_STRING_LEN];
+ char c_fu_name[USB_MAX_STRING_LEN];
+ char c_alt0_name[USB_MAX_STRING_LEN];
+ char c_alt1_name[USB_MAX_STRING_LEN];
s16 p_terminal_type;
s16 c_terminal_type;
--
2.34.1
^ permalink raw reply related
* [PATCH v2] usb: gadget: f_uac1: Expose all string descriptors through configfs.
From: Chris Wulff @ 2024-04-23 14:09 UTC (permalink / raw)
To: linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman, Perr Zhang, Lee Jones,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Chris Wulff
This makes all string descriptors configurable for the UAC1 gadget
so the user can configure names of terminals/controls/alt modes.
Signed-off-by: Chris Wulff <chris.wulff@biamp.com>
---
v2: Improved naming of parameters to be mode user friendly. Added documentation.
v1: https://lore.kernel.org/linux-usb/CO1PR17MB54198F836C3ED1B4FA5F14A9E10D2@CO1PR17MB5419.namprd17.prod.outlook.com/
.../ABI/testing/configfs-usb-gadget-uac1 | 12 +++
Documentation/usb/gadget-testing.rst | 12 +++
drivers/usb/gadget/function/f_uac1.c | 75 +++++++++++++++----
drivers/usb/gadget/function/u_uac1.h | 16 +++-
4 files changed, 98 insertions(+), 17 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac1 b/Documentation/ABI/testing/configfs-usb-gadget-uac1
index c4ba92f004c3..cf93b98b274d 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac1
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac1
@@ -30,4 +30,16 @@ Description:
req_number the number of pre-allocated requests
for both capture and playback
function_name name of the interface
+ p_it_name playback input terminal name
+ p_it_ch_name playback channels name
+ p_ot_name playback output terminal name
+ p_fu_name playback functional unit name
+ p_alt0_name playback alt mode 0 name
+ p_alt1_name playback alt mode 1 name
+ c_it_name capture input terminal name
+ c_it_ch_name capture channels name
+ c_ot_name capture output terminal name
+ c_fu_name capture functional unit name
+ c_alt0_name capture alt mode 0 name
+ c_alt1_name capture alt mode 1 name
===================== =======================================
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index b086c7ab72f0..a89b49e639c3 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -957,6 +957,18 @@ The uac1 function provides these attributes in its function directory:
req_number the number of pre-allocated requests for both capture
and playback
function_name name of the interface
+ p_it_name playback input terminal name
+ p_it_ch_name playback channels name
+ p_ot_name playback output terminal name
+ p_fu_name playback functional unit name
+ p_alt0_name playback alt mode 0 name
+ p_alt1_name playback alt mode 1 name
+ c_it_name capture input terminal name
+ c_it_ch_name capture channels name
+ c_ot_name capture output terminal name
+ c_fu_name capture functional unit name
+ c_alt0_name capture alt mode 0 name
+ c_alt1_name capture alt mode 1 name
================ ====================================================
The attributes have sane default values.
diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
index 7de74a3dd392..f7dd6a2466b0 100644
--- a/drivers/usb/gadget/function/f_uac1.c
+++ b/drivers/usb/gadget/function/f_uac1.c
@@ -377,24 +377,10 @@ enum {
STR_AS_OUT_IF_ALT1,
STR_AS_IN_IF_ALT0,
STR_AS_IN_IF_ALT1,
+ NUM_STR_DESCRIPTORS,
};
-static struct usb_string strings_uac1[] = {
- /* [STR_AC_IF].s = DYNAMIC, */
- [STR_USB_OUT_IT].s = "Playback Input terminal",
- [STR_USB_OUT_IT_CH_NAMES].s = "Playback Channels",
- [STR_IO_OUT_OT].s = "Playback Output terminal",
- [STR_IO_IN_IT].s = "Capture Input terminal",
- [STR_IO_IN_IT_CH_NAMES].s = "Capture Channels",
- [STR_USB_IN_OT].s = "Capture Output terminal",
- [STR_FU_IN].s = "Capture Volume",
- [STR_FU_OUT].s = "Playback Volume",
- [STR_AS_OUT_IF_ALT0].s = "Playback Inactive",
- [STR_AS_OUT_IF_ALT1].s = "Playback Active",
- [STR_AS_IN_IF_ALT0].s = "Capture Inactive",
- [STR_AS_IN_IF_ALT1].s = "Capture Active",
- { },
-};
+static struct usb_string strings_uac1[NUM_STR_DESCRIPTORS + 1] = {};
static struct usb_gadget_strings str_uac1 = {
.language = 0x0409, /* en-us */
@@ -1265,6 +1251,20 @@ static int f_audio_bind(struct usb_configuration *c, struct usb_function *f)
strings_uac1[STR_AC_IF].s = audio_opts->function_name;
+ strings_uac1[STR_USB_OUT_IT].s = audio_opts->p_it_name;
+ strings_uac1[STR_USB_OUT_IT_CH_NAMES].s = audio_opts->p_it_ch_name;
+ strings_uac1[STR_IO_OUT_OT].s = audio_opts->p_ot_name;
+ strings_uac1[STR_FU_OUT].s = audio_opts->p_fu_name;
+ strings_uac1[STR_AS_OUT_IF_ALT0].s = audio_opts->p_alt0_name;
+ strings_uac1[STR_AS_OUT_IF_ALT1].s = audio_opts->p_alt1_name;
+
+ strings_uac1[STR_IO_IN_IT].s = audio_opts->c_it_name;
+ strings_uac1[STR_IO_IN_IT_CH_NAMES].s = audio_opts->c_it_ch_name;
+ strings_uac1[STR_USB_IN_OT].s = audio_opts->c_ot_name;
+ strings_uac1[STR_FU_IN].s = audio_opts->c_fu_name;
+ strings_uac1[STR_AS_IN_IF_ALT0].s = audio_opts->c_alt0_name;
+ strings_uac1[STR_AS_IN_IF_ALT1].s = audio_opts->c_alt1_name;
+
us = usb_gstrings_attach(cdev, uac1_strings, ARRAY_SIZE(strings_uac1));
if (IS_ERR(us))
return PTR_ERR(us);
@@ -1681,8 +1681,23 @@ UAC1_ATTRIBUTE(bool, c_volume_present);
UAC1_ATTRIBUTE(s16, c_volume_min);
UAC1_ATTRIBUTE(s16, c_volume_max);
UAC1_ATTRIBUTE(s16, c_volume_res);
+
UAC1_ATTRIBUTE_STRING(function_name);
+UAC1_ATTRIBUTE_STRING(p_it_name);
+UAC1_ATTRIBUTE_STRING(p_it_ch_name);
+UAC1_ATTRIBUTE_STRING(p_ot_name);
+UAC1_ATTRIBUTE_STRING(p_fu_name);
+UAC1_ATTRIBUTE_STRING(p_alt0_name);
+UAC1_ATTRIBUTE_STRING(p_alt1_name);
+
+UAC1_ATTRIBUTE_STRING(c_it_name);
+UAC1_ATTRIBUTE_STRING(c_it_ch_name);
+UAC1_ATTRIBUTE_STRING(c_ot_name);
+UAC1_ATTRIBUTE_STRING(c_fu_name);
+UAC1_ATTRIBUTE_STRING(c_alt0_name);
+UAC1_ATTRIBUTE_STRING(c_alt1_name);
+
static struct configfs_attribute *f_uac1_attrs[] = {
&f_uac1_opts_attr_c_chmask,
&f_uac1_opts_attr_c_srate,
@@ -1706,6 +1721,20 @@ static struct configfs_attribute *f_uac1_attrs[] = {
&f_uac1_opts_attr_function_name,
+ &f_uac1_opts_attr_p_it_name,
+ &f_uac1_opts_attr_p_it_ch_name,
+ &f_uac1_opts_attr_p_ot_name,
+ &f_uac1_opts_attr_p_fu_name,
+ &f_uac1_opts_attr_p_alt0_name,
+ &f_uac1_opts_attr_p_alt1_name,
+
+ &f_uac1_opts_attr_c_it_name,
+ &f_uac1_opts_attr_c_it_ch_name,
+ &f_uac1_opts_attr_c_ot_name,
+ &f_uac1_opts_attr_c_fu_name,
+ &f_uac1_opts_attr_c_alt0_name,
+ &f_uac1_opts_attr_c_alt1_name,
+
NULL,
};
@@ -1760,6 +1789,20 @@ static struct usb_function_instance *f_audio_alloc_inst(void)
scnprintf(opts->function_name, sizeof(opts->function_name), "AC Interface");
+ scnprintf(opts->p_it_name, sizeof(opts->p_it_name), "Playback Input terminal");
+ scnprintf(opts->p_it_ch_name, sizeof(opts->p_it_ch_name), "Playback Channels");
+ scnprintf(opts->p_ot_name, sizeof(opts->p_ot_name), "Playback Output terminal");
+ scnprintf(opts->p_fu_name, sizeof(opts->p_fu_name), "Playback Volume");
+ scnprintf(opts->p_alt0_name, sizeof(opts->p_alt0_name), "Playback Inactive");
+ scnprintf(opts->p_alt1_name, sizeof(opts->p_alt1_name), "Playback Active");
+
+ scnprintf(opts->c_it_name, sizeof(opts->c_it_name), "Capture Input terminal");
+ scnprintf(opts->c_it_ch_name, sizeof(opts->c_it_ch_name), "Capture Channels");
+ scnprintf(opts->c_ot_name, sizeof(opts->c_ot_name), "Capture Output terminal");
+ scnprintf(opts->c_fu_name, sizeof(opts->c_fu_name), "Capture Volume");
+ scnprintf(opts->c_alt0_name, sizeof(opts->c_alt0_name), "Capture Inactive");
+ scnprintf(opts->c_alt1_name, sizeof(opts->c_alt1_name), "Capture Active");
+
return &opts->func_inst;
}
diff --git a/drivers/usb/gadget/function/u_uac1.h b/drivers/usb/gadget/function/u_uac1.h
index f7a616760e31..67784de9782b 100644
--- a/drivers/usb/gadget/function/u_uac1.h
+++ b/drivers/usb/gadget/function/u_uac1.h
@@ -52,7 +52,21 @@ struct f_uac1_opts {
int req_number;
unsigned bound:1;
- char function_name[32];
+ char function_name[USB_MAX_STRING_LEN];
+
+ char p_it_name[USB_MAX_STRING_LEN];
+ char p_it_ch_name[USB_MAX_STRING_LEN];
+ char p_ot_name[USB_MAX_STRING_LEN];
+ char p_fu_name[USB_MAX_STRING_LEN];
+ char p_alt0_name[USB_MAX_STRING_LEN];
+ char p_alt1_name[USB_MAX_STRING_LEN];
+
+ char c_it_name[USB_MAX_STRING_LEN];
+ char c_it_ch_name[USB_MAX_STRING_LEN];
+ char c_ot_name[USB_MAX_STRING_LEN];
+ char c_fu_name[USB_MAX_STRING_LEN];
+ char c_alt0_name[USB_MAX_STRING_LEN];
+ char c_alt1_name[USB_MAX_STRING_LEN];
struct mutex lock;
int refcnt;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: stsp @ 2024-04-23 11:02 UTC (permalink / raw)
To: Stefan Metzmacher, linux-kernel
Cc: Eric Biederman, Alexander Viro, Andy Lutomirski,
Christian Brauner, Jan Kara, Jeff Layton, Chuck Lever,
Alexander Aring, linux-fsdevel, Paolo Bonzini,
Christian Göttsche, Linux API Mailing List
In-Reply-To: <a24f8d8c-2e7e-41ff-a640-134501ba4fa2@samba.org>
23.04.2024 13:58, Stefan Metzmacher пишет:
>
> I guess this is something that should cc linux-api@vger.kernel.org ...
Done, thanks.
^ permalink raw reply
* [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag
From: Stas Sergeev @ 2024-04-23 11:01 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240423110148.13114-1-stsp2@yandex.ru>
This flag performs the open operation with the credentials that
were in effect when dir_fd was opened.
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:
To avoid sandboxing escape, this patch makes sure the restricted
lookup modes are used. Namely, RESOLVE_BENEATH or RESOLVE_IN_ROOT.
To avoid leaking creds across exec, this patch requires O_CLOEXEC
flag on a directory.
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/internal.h | 2 +-
fs/namei.c | 30 ++++++++++++++++++++++++++++--
fs/open.c | 2 +-
include/linux/fcntl.h | 2 ++
include/uapi/linux/openat2.h | 3 +++
5 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 7ca738904e34..692b53b19aad 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -169,7 +169,7 @@ static inline void sb_end_ro_state_change(struct super_block *sb)
* open.c
*/
struct open_flags {
- int open_flag;
+ u64 open_flag;
umode_t mode;
int acc_mode;
int intent;
diff --git a/fs/namei.c b/fs/namei.c
index 2fde2c320ae9..0e0f2e32ef02 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -586,6 +586,7 @@ struct nameidata {
int dfd;
vfsuid_t dir_vfsuid;
umode_t dir_mode;
+ const struct cred *dir_open_cred;
} __randomize_layout;
#define ND_ROOT_PRESET 1
@@ -695,6 +696,7 @@ static void terminate_walk(struct nameidata *nd)
nd->depth = 0;
nd->path.mnt = NULL;
nd->path.dentry = NULL;
+ put_cred(nd->dir_open_cred);
}
/* path_put is needed afterwards regardless of success or failure */
@@ -2414,6 +2416,7 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
get_fs_pwd(current->fs, &nd->path);
nd->inode = nd->path.dentry->d_inode;
}
+ nd->dir_open_cred = get_current_cred();
} else {
/* Caller must check execute permissions on the starting path component */
struct fd f = fdget_raw(nd->dfd);
@@ -2437,6 +2440,7 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
path_get(&nd->path);
nd->inode = nd->path.dentry->d_inode;
}
+ nd->dir_open_cred = get_cred(f.file->f_cred);
fdput(f);
}
@@ -3794,8 +3798,28 @@ static struct file *path_openat(struct nameidata *nd,
error = do_o_path(nd, flags, file);
} else {
const char *s = path_init(nd, flags);
- file = alloc_empty_file(op->open_flag, current_cred());
- error = PTR_ERR_OR_ZERO(file);
+ const struct cred *old_cred = NULL;
+
+ error = 0;
+ if (op->open_flag & OA2_INHERIT_CRED) {
+ /* Make sure to work only with restricted
+ * look-up modes.
+ */
+ if (!(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT)))
+ error = -EPERM;
+ /* Only work with O_CLOEXEC dirs. */
+ if (!get_close_on_exec(nd->dfd))
+ error = -EPERM;
+
+ if (!error)
+ old_cred = override_creds(nd->dir_open_cred);
+ }
+ if (!error) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ error = PTR_ERR_OR_ZERO(file);
+ } else {
+ file = ERR_PTR(error);
+ }
if (!error) {
while (!(error = link_path_walk(s, nd)) &&
(s = open_last_lookups(nd, file, op)) != NULL)
@@ -3803,6 +3827,8 @@ static struct file *path_openat(struct nameidata *nd,
}
if (!error)
error = do_open(nd, file, op);
+ if (old_cred)
+ revert_creds(old_cred);
terminate_walk(nd);
if (IS_ERR(file))
return file;
diff --git a/fs/open.c b/fs/open.c
index ee8460c83c77..6be013182a35 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;
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..b71f8b162102 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)
+#define VALID_OPENAT2_FLAGS (VALID_OPEN_FLAGS | OA2_INHERIT_CRED)
+
/* 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..cdd676a10b62 100644
--- a/include/uapi/linux/openat2.h
+++ b/include/uapi/linux/openat2.h
@@ -40,4 +40,7 @@ struct open_how {
return -EAGAIN if that's not
possible. */
+/* openat2-specific flags go to upper 4 bytes. */
+#define OA2_INHERIT_CRED (1ULL << 32)
+
#endif /* _UAPI_LINUX_OPENAT2_H */
--
2.44.0
^ permalink raw reply related
* [PATCH 1/2] fs: reorganize path_openat()
From: Stas Sergeev @ 2024-04-23 11:01 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <20240423110148.13114-1-stsp2@yandex.ru>
This patch moves the call to alloc_empty_file() below the call to
path_init(). That changes is needed for the next patch, which adds
a cred override for alloc_empty_file(). The needed cred info is only
available after the call to path_init().
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: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
fs/namei.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index c5b2a25be7d0..2fde2c320ae9 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3782,22 +3782,30 @@ static struct file *path_openat(struct nameidata *nd,
struct file *file;
int error;
- file = alloc_empty_file(op->open_flag, current_cred());
- if (IS_ERR(file))
- return file;
-
- if (unlikely(file->f_flags & __O_TMPFILE)) {
+ if (unlikely(op->open_flag & __O_TMPFILE)) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ if (IS_ERR(file))
+ return file;
error = do_tmpfile(nd, flags, op, file);
- } else if (unlikely(file->f_flags & O_PATH)) {
+ } else if (unlikely(op->open_flag & O_PATH)) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ if (IS_ERR(file))
+ return file;
error = do_o_path(nd, flags, file);
} else {
const char *s = path_init(nd, flags);
- while (!(error = link_path_walk(s, nd)) &&
- (s = open_last_lookups(nd, file, op)) != NULL)
- ;
+ file = alloc_empty_file(op->open_flag, current_cred());
+ error = PTR_ERR_OR_ZERO(file);
+ if (!error) {
+ while (!(error = link_path_walk(s, nd)) &&
+ (s = open_last_lookups(nd, file, op)) != NULL)
+ ;
+ }
if (!error)
error = do_open(nd, file, op);
terminate_walk(nd);
+ if (IS_ERR(file))
+ return file;
}
if (likely(!error)) {
if (likely(file->f_mode & FMODE_OPENED))
--
2.44.0
^ permalink raw reply related
* [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: Stas Sergeev @ 2024-04-23 11:01 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, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
This patch-set implements the OA2_INHERIT_CRED 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. 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.
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: 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>
Stas Sergeev (2):
fs: reorganize path_openat()
openat2: add OA2_INHERIT_CRED flag
fs/internal.h | 2 +-
fs/namei.c | 52 +++++++++++++++++++++++++++++-------
fs/open.c | 2 +-
include/linux/fcntl.h | 2 ++
include/uapi/linux/openat2.h | 3 +++
5 files changed, 50 insertions(+), 11 deletions(-)
--
2.44.0
^ permalink raw reply
* Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
From: Stefan Metzmacher @ 2024-04-23 10:58 UTC (permalink / raw)
To: Stas Sergeev, linux-kernel
Cc: Eric Biederman, Alexander Viro, Andy Lutomirski,
Christian Brauner, Jan Kara, Jeff Layton, Chuck Lever,
Alexander Aring, linux-fsdevel, Paolo Bonzini,
Christian Göttsche, Linux API Mailing List
In-Reply-To: <20240423104824.10464-1-stsp2@yandex.ru>
Am 23.04.24 um 12:48 schrieb Stas Sergeev:
> This patch-set implements the OA2_INHERIT_CRED 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. 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.
>
> 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: linux-fsdevel@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
I guess this is something that should cc linux-api@vger.kernel.org ...
metze
^ permalink raw reply
* Re: [PATCHv3 bpf-next 0/7] uprobe: uretprobe speed up
From: Jiri Olsa @ 2024-04-22 20:25 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, 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: <20240423000943.478ccf1e735a63c6c1b4c66b@kernel.org>
On Tue, Apr 23, 2024 at 12:09:43AM +0900, Masami Hiramatsu wrote:
> Hi Jiri,
>
> On Sun, 21 Apr 2024 21:41:59 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > hi,
> > as part of the effort on speeding up the uprobes [0] coming with
> > return uprobe optimization by using syscall instead of the trap
> > on the uretprobe trampoline.
> >
> > The speed up depends on instruction type that uprobe is installed
> > and depends on specific HW type, please check patch 1 for details.
> >
> > Patches 1-6 are based on bpf-next/master, but path 1 and 2 are
> > apply-able on linux-trace.git tree probes/for-next branch.
> > Patch 7 is based on man-pages master.
>
> Thanks for updated! I reviewed the series and just except for the
> manpage, it looks good to me.
>
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> for the series.
> If Linux API maintainers are OK, I can pick this in probes/for-next.
great, thanks
> (BTW, who will pick the manpage patch?)
ugh, I cc-ed linux-api but not linux-man@vger.kernel.org
I'll add that for new version
jirka
>
> Thank you,
>
> >
> > v3 changes:
> > - added source ip check if the uretprobe syscall is called from
> > trampoline and sending SIGILL to process if it's not
> > - keep x86 compat process to use standard breakpoint
> > - split syscall wiring into separate change
> > - ran ltp and syzkaller locally, no issues found [Masami]
> > - building uprobe_compat binary in selftests which breaks
> > CI atm because of missing 32-bit delve packages, I will
> > need to fix that in separate changes once this is acked
> > - added man page change
> > - there were several changes so I removed acks [Oleg Andrii]
> >
> > Also available at:
> > https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > uretprobe_syscall
> >
> > thanks,
> > jirka
> >
> >
> > Notes to check list items in Documentation/process/adding-syscalls.rst:
> >
> > - System Call Alternatives
> > New syscall seems like the best way in here, becase we need
> > just to quickly enter kernel with no extra arguments processing,
> > which we'd need to do if we decided to use another syscall.
> >
> > - Designing the API: Planning for Extension
> > The uretprobe syscall is very specific and most likely won't be
> > extended in the future.
> >
> > At the moment it does not take any arguments and even if it does
> > in future, it's allowed to be called only from trampoline prepared
> > by kernel, so there'll be no broken user.
> >
> > - Designing the API: Other Considerations
> > N/A because uretprobe syscall does not return reference to kernel
> > object.
> >
> > - Proposing the API
> > Wiring up of the uretprobe system call si in separate change,
> > selftests and man page changes are part of the patchset.
> >
> > - Generic System Call Implementation
> > There's no CONFIG option for the new functionality because it
> > keeps the same behaviour from the user POV.
> >
> > - x86 System Call Implementation
> > It's 64-bit syscall only.
> >
> > - Compatibility System Calls (Generic)
> > N/A uretprobe syscall has no arguments and is not supported
> > for compat processes.
> >
> > - Compatibility System Calls (x86)
> > N/A uretprobe syscall is not supported for compat processes.
> >
> > - System Calls Returning Elsewhere
> > N/A.
> >
> > - Other Details
> > N/A.
> >
> > - Testing
> > Adding new bpf selftests and ran ltp on top of this change.
> >
> > - Man Page
> > Attached.
> >
> > - Do not call System Calls in the Kernel
> > N/A.
> >
> >
> > [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> > ---
> > Jiri Olsa (6):
> > uprobe: Wire up uretprobe system call
> > uprobe: Add uretprobe syscall to speed up return probe
> > selftests/bpf: Add uretprobe syscall test for regs integrity
> > selftests/bpf: Add uretprobe syscall test for regs changes
> > selftests/bpf: Add uretprobe syscall call from user space test
> > selftests/bpf: Add uretprobe compat test
> >
> > arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> > arch/x86/kernel/uprobes.c | 115 ++++++++++++++++++++++++++++++
> > include/linux/syscalls.h | 2 +
> > include/linux/uprobes.h | 3 +
> > include/uapi/asm-generic/unistd.h | 5 +-
> > kernel/events/uprobes.c | 24 +++++--
> > kernel/sys_ni.c | 2 +
> > tools/include/linux/compiler.h | 4 ++
> > tools/testing/selftests/bpf/.gitignore | 1 +
> > tools/testing/selftests/bpf/Makefile | 6 +-
> > tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 +++++++++++++++++++++++++++++++-
> > tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
> > tools/testing/selftests/bpf/progs/uprobe_syscall_call.c | 15 ++++
> > tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c | 13 ++++
> > 15 files changed, 681 insertions(+), 10 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> >
> >
> > Jiri Olsa (1):
> > man2: Add uretprobe syscall page
> >
> > man2/uretprobe.2 | 40 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 man2/uretprobe.2
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 7/7] man2: Add uretprobe syscall page
From: Jiri Olsa @ 2024-04-22 20:15 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, 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: <20240423000729.f1d58443100c3994afca0a7f@kernel.org>
On Tue, Apr 23, 2024 at 12:07:29AM +0900, Masami Hiramatsu wrote:
> On Sun, 21 Apr 2024 21:42:06 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > Adding man page for new uretprobe syscall.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > man2/uretprobe.2 | 40 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 man2/uretprobe.2
> >
> > diff --git a/man2/uretprobe.2 b/man2/uretprobe.2
> > new file mode 100644
> > index 000000000000..c0343a88bb57
> > --- /dev/null
> > +++ b/man2/uretprobe.2
> > @@ -0,0 +1,40 @@
> > +.\" Copyright (C) 2024, Jiri Olsa <jolsa@kernel.org>
> > +.\"
> > +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> > +.\"
> > +.TH uretprobe 2 (date) "Linux man-pages (unreleased)"
> > +.SH NAME
> > +uretprobe \- execute pending return uprobes
> > +.SH SYNOPSIS
> > +.nf
> > +.B int uretprobe(void)
> > +.fi
> > +.SH DESCRIPTION
> > +On x86_64 architecture the kernel is using uretprobe syscall to trigger
> > +uprobe return probe consumers instead of using standard breakpoint instruction.
> > +The reason is that it's much faster to do syscall than breakpoint trap
> > +on x86_64 architecture.
>
> Do we specify the supported architecture as this? Currently it is supported
> only on x86-64, but it could be extended later, right?
yes, that's the idea, but I can't really speak other than x86 ;-)
so not sure abour other archs details
>
> This should be just noted as NOTES. Something like "This syscall is initially
> introduced on x86-64 because a syscall is faster than a breakpoint trap on it.
> But this will be extended to the architectures whose syscall is faster than
> breakpoint trap."
's/will be extended/might be will be extended/' seems better to me,
other than that it looks ok
thanks,
jirka
>
> Thank you,
>
> > +
> > +The uretprobe syscall is not supposed to be called directly by user, it's allowed
> > +to be invoked only through user space trampoline provided by kernel.
> > +When called from outside of this trampoline, the calling process will receive
> > +.BR SIGILL .
> > +
> > +.SH RETURN VALUE
> > +.BR uretprobe()
> > +return value is specific for given architecture.
> > +
> > +.SH VERSIONS
> > +This syscall is not specified in POSIX,
> > +and details of its behavior vary across systems.
> > +.SH STANDARDS
> > +None.
> > +.SH NOTES
> > +.BR uretprobe()
> > +exists only to allow the invocation of return uprobe consumers.
> > +It should
> > +.B never
> > +be called directly.
> > +Details of the arguments (if any) passed to
> > +.BR uretprobe ()
> > +and the return value are specific for given architecture.
> > --
> > 2.44.0
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 1/7] uprobe: Wire up uretprobe system call
From: Oleg Nesterov @ 2024-04-22 15:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, 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 04/21, Jiri Olsa wrote:
>
> 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(-)
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe
From: Oleg Nesterov @ 2024-04-22 15:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, 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 04/21, Jiri Olsa wrote:
>
> arch/x86/kernel/uprobes.c | 115 ++++++++++++++++++++++++++++++++++++++
> include/linux/uprobes.h | 3 +
> kernel/events/uprobes.c | 24 +++++---
> 3 files changed, 135 insertions(+), 7 deletions(-)
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 0/7] uprobe: uretprobe speed up
From: Masami Hiramatsu @ 2024-04-22 15:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, 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-1-jolsa@kernel.org>
Hi Jiri,
On Sun, 21 Apr 2024 21:41:59 +0200
Jiri Olsa <jolsa@kernel.org> wrote:
> hi,
> as part of the effort on speeding up the uprobes [0] coming with
> return uprobe optimization by using syscall instead of the trap
> on the uretprobe trampoline.
>
> The speed up depends on instruction type that uprobe is installed
> and depends on specific HW type, please check patch 1 for details.
>
> Patches 1-6 are based on bpf-next/master, but path 1 and 2 are
> apply-able on linux-trace.git tree probes/for-next branch.
> Patch 7 is based on man-pages master.
Thanks for updated! I reviewed the series and just except for the
manpage, it looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
for the series.
If Linux API maintainers are OK, I can pick this in probes/for-next.
(BTW, who will pick the manpage patch?)
Thank you,
>
> v3 changes:
> - added source ip check if the uretprobe syscall is called from
> trampoline and sending SIGILL to process if it's not
> - keep x86 compat process to use standard breakpoint
> - split syscall wiring into separate change
> - ran ltp and syzkaller locally, no issues found [Masami]
> - building uprobe_compat binary in selftests which breaks
> CI atm because of missing 32-bit delve packages, I will
> need to fix that in separate changes once this is acked
> - added man page change
> - there were several changes so I removed acks [Oleg Andrii]
>
> Also available at:
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> uretprobe_syscall
>
> thanks,
> jirka
>
>
> Notes to check list items in Documentation/process/adding-syscalls.rst:
>
> - System Call Alternatives
> New syscall seems like the best way in here, becase we need
> just to quickly enter kernel with no extra arguments processing,
> which we'd need to do if we decided to use another syscall.
>
> - Designing the API: Planning for Extension
> The uretprobe syscall is very specific and most likely won't be
> extended in the future.
>
> At the moment it does not take any arguments and even if it does
> in future, it's allowed to be called only from trampoline prepared
> by kernel, so there'll be no broken user.
>
> - Designing the API: Other Considerations
> N/A because uretprobe syscall does not return reference to kernel
> object.
>
> - Proposing the API
> Wiring up of the uretprobe system call si in separate change,
> selftests and man page changes are part of the patchset.
>
> - Generic System Call Implementation
> There's no CONFIG option for the new functionality because it
> keeps the same behaviour from the user POV.
>
> - x86 System Call Implementation
> It's 64-bit syscall only.
>
> - Compatibility System Calls (Generic)
> N/A uretprobe syscall has no arguments and is not supported
> for compat processes.
>
> - Compatibility System Calls (x86)
> N/A uretprobe syscall is not supported for compat processes.
>
> - System Calls Returning Elsewhere
> N/A.
>
> - Other Details
> N/A.
>
> - Testing
> Adding new bpf selftests and ran ltp on top of this change.
>
> - Man Page
> Attached.
>
> - Do not call System Calls in the Kernel
> N/A.
>
>
> [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> ---
> Jiri Olsa (6):
> uprobe: Wire up uretprobe system call
> uprobe: Add uretprobe syscall to speed up return probe
> selftests/bpf: Add uretprobe syscall test for regs integrity
> selftests/bpf: Add uretprobe syscall test for regs changes
> selftests/bpf: Add uretprobe syscall call from user space test
> selftests/bpf: Add uretprobe compat test
>
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> arch/x86/kernel/uprobes.c | 115 ++++++++++++++++++++++++++++++
> include/linux/syscalls.h | 2 +
> include/linux/uprobes.h | 3 +
> include/uapi/asm-generic/unistd.h | 5 +-
> kernel/events/uprobes.c | 24 +++++--
> kernel/sys_ni.c | 2 +
> tools/include/linux/compiler.h | 4 ++
> tools/testing/selftests/bpf/.gitignore | 1 +
> tools/testing/selftests/bpf/Makefile | 6 +-
> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 +++++++++++++++++++++++++++++++-
> tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
> tools/testing/selftests/bpf/progs/uprobe_syscall_call.c | 15 ++++
> tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c | 13 ++++
> 15 files changed, 681 insertions(+), 10 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
>
>
> Jiri Olsa (1):
> man2: Add uretprobe syscall page
>
> man2/uretprobe.2 | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 man2/uretprobe.2
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 7/7] man2: Add uretprobe syscall page
From: Masami Hiramatsu @ 2024-04-22 15:07 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, 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-8-jolsa@kernel.org>
On Sun, 21 Apr 2024 21:42:06 +0200
Jiri Olsa <jolsa@kernel.org> wrote:
> Adding man page for new uretprobe syscall.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> man2/uretprobe.2 | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 man2/uretprobe.2
>
> diff --git a/man2/uretprobe.2 b/man2/uretprobe.2
> new file mode 100644
> index 000000000000..c0343a88bb57
> --- /dev/null
> +++ b/man2/uretprobe.2
> @@ -0,0 +1,40 @@
> +.\" Copyright (C) 2024, Jiri Olsa <jolsa@kernel.org>
> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.\"
> +.TH uretprobe 2 (date) "Linux man-pages (unreleased)"
> +.SH NAME
> +uretprobe \- execute pending return uprobes
> +.SH SYNOPSIS
> +.nf
> +.B int uretprobe(void)
> +.fi
> +.SH DESCRIPTION
> +On x86_64 architecture the kernel is using uretprobe syscall to trigger
> +uprobe return probe consumers instead of using standard breakpoint instruction.
> +The reason is that it's much faster to do syscall than breakpoint trap
> +on x86_64 architecture.
Do we specify the supported architecture as this? Currently it is supported
only on x86-64, but it could be extended later, right?
This should be just noted as NOTES. Something like "This syscall is initially
introduced on x86-64 because a syscall is faster than a breakpoint trap on it.
But this will be extended to the architectures whose syscall is faster than
breakpoint trap."
Thank you,
> +
> +The uretprobe syscall is not supposed to be called directly by user, it's allowed
> +to be invoked only through user space trampoline provided by kernel.
> +When called from outside of this trampoline, the calling process will receive
> +.BR SIGILL .
> +
> +.SH RETURN VALUE
> +.BR uretprobe()
> +return value is specific for given architecture.
> +
> +.SH VERSIONS
> +This syscall is not specified in POSIX,
> +and details of its behavior vary across systems.
> +.SH STANDARDS
> +None.
> +.SH NOTES
> +.BR uretprobe()
> +exists only to allow the invocation of return uprobe consumers.
> +It should
> +.B never
> +be called directly.
> +Details of the arguments (if any) passed to
> +.BR uretprobe ()
> +and the return value are specific for given architecture.
> --
> 2.44.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH 7/7] man2: Add uretprobe syscall page
From: Jiri Olsa @ 2024-04-21 19:42 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: 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-1-jolsa@kernel.org>
Adding man page for new uretprobe syscall.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
man2/uretprobe.2 | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 man2/uretprobe.2
diff --git a/man2/uretprobe.2 b/man2/uretprobe.2
new file mode 100644
index 000000000000..c0343a88bb57
--- /dev/null
+++ b/man2/uretprobe.2
@@ -0,0 +1,40 @@
+.\" Copyright (C) 2024, Jiri Olsa <jolsa@kernel.org>
+.\"
+.\" SPDX-License-Identifier: Linux-man-pages-copyleft
+.\"
+.TH uretprobe 2 (date) "Linux man-pages (unreleased)"
+.SH NAME
+uretprobe \- execute pending return uprobes
+.SH SYNOPSIS
+.nf
+.B int uretprobe(void)
+.fi
+.SH DESCRIPTION
+On x86_64 architecture the kernel is using uretprobe syscall to trigger
+uprobe return probe consumers instead of using standard breakpoint instruction.
+The reason is that it's much faster to do syscall than breakpoint trap
+on x86_64 architecture.
+
+The uretprobe syscall is not supposed to be called directly by user, it's allowed
+to be invoked only through user space trampoline provided by kernel.
+When called from outside of this trampoline, the calling process will receive
+.BR SIGILL .
+
+.SH RETURN VALUE
+.BR uretprobe()
+return value is specific for given architecture.
+
+.SH VERSIONS
+This syscall is not specified in POSIX,
+and details of its behavior vary across systems.
+.SH STANDARDS
+None.
+.SH NOTES
+.BR uretprobe()
+exists only to allow the invocation of return uprobe consumers.
+It should
+.B never
+be called directly.
+Details of the arguments (if any) passed to
+.BR uretprobe ()
+and the return value are specific for given architecture.
--
2.44.0
^ permalink raw reply related
* [PATCHv3 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
From: Jiri Olsa @ 2024-04-21 19:42 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: 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-1-jolsa@kernel.org>
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
# 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");
+ 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)
+{
+ 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 related
* [PATCHv3 bpf-next 5/7] selftests/bpf: Add uretprobe syscall call from user space test
From: Jiri Olsa @ 2024-04-21 19:42 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: 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-1-jolsa@kernel.org>
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
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 1a50cd35205d..9233210a4c33 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -7,7 +7,10 @@
#include <unistd.h>
#include <asm/ptrace.h>
#include <linux/compiler.h>
+#include <linux/stringify.h>
+#include <sys/wait.h>
#include "uprobe_syscall.skel.h"
+#include "uprobe_syscall_call.skel.h"
__naked unsigned long uretprobe_regs_trigger(void)
{
@@ -209,6 +212,85 @@ static void test_uretprobe_regs_change(void)
}
}
+#ifndef __NR_uretprobe
+#define __NR_uretprobe 462
+#endif
+
+__naked unsigned long uretprobe_syscall_call_1(void)
+{
+ /*
+ * Pretend we are uretprobe trampoline to trigger the return
+ * probe invocation in order to verify we get SIGILL.
+ */
+ asm volatile (
+ "pushq %rax\n"
+ "pushq %rcx\n"
+ "pushq %r11\n"
+ "movq $" __stringify(__NR_uretprobe) ", %rax\n"
+ "syscall\n"
+ "popq %r11\n"
+ "popq %rcx\n"
+ "retq\n"
+ );
+}
+
+__naked unsigned long uretprobe_syscall_call(void)
+{
+ asm volatile (
+ "call uretprobe_syscall_call_1\n"
+ "retq\n"
+ );
+}
+
+static void __test_uretprobe_syscall_call(void)
+{
+ struct uprobe_syscall_call *skel = NULL;
+ int err;
+
+ skel = uprobe_syscall_call__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "uprobe_syscall_call__open_and_load"))
+ goto cleanup;
+
+ err = uprobe_syscall_call__attach(skel);
+ if (!ASSERT_OK(err, "uprobe_syscall_call__attach"))
+ goto cleanup;
+
+ uretprobe_syscall_call();
+
+cleanup:
+ uprobe_syscall_call__destroy(skel);
+}
+
+static void trace_pipe_cb(const char *str, void *data)
+{
+ if (strstr(str, "uretprobe called") != NULL)
+ (*(int *)data)++;
+}
+
+static void test_uretprobe_syscall_call(void)
+{
+ int pid, status, found = 0;
+
+ pid = fork();
+ if (!ASSERT_GE(pid, 0, "fork"))
+ return;
+
+ if (pid == 0) {
+ __test_uretprobe_syscall_call();
+ _exit(0);
+ }
+
+ waitpid(pid, &status, 0);
+
+ /* verify the child got killed with SIGILL */
+ ASSERT_EQ(WIFSIGNALED(status), 1, "WIFSIGNALED");
+ ASSERT_EQ(WTERMSIG(status), SIGILL, "WTERMSIG");
+
+ /* verify the uretprobe program wasn't called */
+ ASSERT_OK(read_trace_pipe_iter(trace_pipe_cb, &found, 1000),
+ "read_trace_pipe_iter");
+ ASSERT_EQ(found, 0, "found");
+}
#else
static void test_uretprobe_regs_equal(void)
{
@@ -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)
+{
+ 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");
+ return 0;
+}
--
2.44.0
^ permalink raw reply related
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