* Re: [PATCHv3 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
From: Jiri Olsa @ 2024-04-29 7:39 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <CAEf4BzYU-y+vptqXpuALYecJJgPt+CTcbo+=Q9QXnu4vNwem+g@mail.gmail.com>
On Fri, Apr 26, 2024 at 11:06:53AM -0700, Andrii Nakryiko wrote:
> On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding test that adds return uprobe inside 32 bit task
> > and verify the return uprobe and attached bpf programs
> > get properly executed.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/testing/selftests/bpf/.gitignore | 1 +
> > tools/testing/selftests/bpf/Makefile | 6 ++-
> > .../selftests/bpf/prog_tests/uprobe_syscall.c | 40 +++++++++++++++++++
> > .../bpf/progs/uprobe_syscall_compat.c | 13 ++++++
> > 4 files changed, 59 insertions(+), 1 deletion(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> >
> > diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> > index f1aebabfb017..69d71223c0dd 100644
> > --- a/tools/testing/selftests/bpf/.gitignore
> > +++ b/tools/testing/selftests/bpf/.gitignore
> > @@ -45,6 +45,7 @@ test_cpp
> > /veristat
> > /sign-file
> > /uprobe_multi
> > +/uprobe_compat
> > *.ko
> > *.tmp
> > xskxceiver
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index edc73f8f5aef..d170b63eca62 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -134,7 +134,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> > xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> > xdp_features bpf_test_no_cfi.ko
> >
> > -TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
> > +TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi uprobe_compat
>
> you need to add uprobe_compat to TRUNNER_EXTRA_FILES as well, no?
ah right
> > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > index 9233210a4c33..3770254d893b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > @@ -11,6 +11,7 @@
> > #include <sys/wait.h>
> > #include "uprobe_syscall.skel.h"
> > #include "uprobe_syscall_call.skel.h"
> > +#include "uprobe_syscall_compat.skel.h"
> >
> > __naked unsigned long uretprobe_regs_trigger(void)
> > {
> > @@ -291,6 +292,35 @@ static void test_uretprobe_syscall_call(void)
> > "read_trace_pipe_iter");
> > ASSERT_EQ(found, 0, "found");
> > }
> > +
> > +static void trace_pipe_compat_cb(const char *str, void *data)
> > +{
> > + if (strstr(str, "uretprobe compat") != NULL)
> > + (*(int *)data)++;
> > +}
> > +
> > +static void test_uretprobe_compat(void)
> > +{
> > + struct uprobe_syscall_compat *skel = NULL;
> > + int err, found = 0;
> > +
> > + skel = uprobe_syscall_compat__open_and_load();
> > + if (!ASSERT_OK_PTR(skel, "uprobe_syscall_compat__open_and_load"))
> > + goto cleanup;
> > +
> > + err = uprobe_syscall_compat__attach(skel);
> > + if (!ASSERT_OK(err, "uprobe_syscall_compat__attach"))
> > + goto cleanup;
> > +
> > + system("./uprobe_compat");
> > +
> > + ASSERT_OK(read_trace_pipe_iter(trace_pipe_compat_cb, &found, 1000),
> > + "read_trace_pipe_iter");
>
> why so complicated? can't you just set global variable that it was called
hm, we execute separate uprobe_compat (32bit) process that triggers the bpf
program, so we can't use global variable.. using the trace_pipe was the only
thing that was easy to do
jirka
>
> > + ASSERT_EQ(found, 1, "found");
> > +
> > +cleanup:
> > + uprobe_syscall_compat__destroy(skel);
> > +}
> > #else
> > static void test_uretprobe_regs_equal(void)
> > {
> > @@ -306,6 +336,11 @@ static void test_uretprobe_syscall_call(void)
> > {
> > test__skip();
> > }
> > +
> > +static void test_uretprobe_compat(void)
> > +{
> > + test__skip();
> > +}
> > #endif
> >
> > void test_uprobe_syscall(void)
> > @@ -320,3 +355,8 @@ void serial_test_uprobe_syscall_call(void)
> > {
> > test_uretprobe_syscall_call();
> > }
> > +
> > +void serial_test_uprobe_syscall_compat(void)
>
> and then no need for serial_test?
>
> > +{
> > + test_uretprobe_compat();
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > new file mode 100644
> > index 000000000000..f8adde7f08e2
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("uretprobe.multi/./uprobe_compat:main")
> > +int uretprobe_compat(struct pt_regs *ctx)
> > +{
> > + bpf_printk("uretprobe compat\n");
> > + return 0;
> > +}
> > --
> > 2.44.0
> >
^ permalink raw reply
* Re: [PATCH v5 0/3] implement OA2_CRED_INHERIT flag for openat2()
From: Christian Brauner @ 2024-04-29 9:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stas Sergeev, Aleksa Sarai, Serge E. Hallyn, linux-kernel,
Stefan Metzmacher, Eric Biederman, Alexander Viro,
Andy Lutomirski, Jan Kara, Jeff Layton, Chuck Lever,
Alexander Aring, David Laight, linux-fsdevel, linux-api,
Paolo Bonzini, Christian Göttsche
In-Reply-To: <CALCETrUL3zXAX94CpcQYwj1omwO+=-1Li+J7Bw2kpAw4d7nsyw@mail.gmail.com>
On Sun, Apr 28, 2024 at 09:41:20AM -0700, Andy Lutomirski wrote:
> > On Apr 26, 2024, at 6:39 AM, Stas Sergeev <stsp2@yandex.ru> wrote:
> > This patch-set implements the OA2_CRED_INHERIT flag for openat2() syscall.
> > It is needed to perform an open operation with the creds that were in
> > effect when the dir_fd was opened, if the dir was opened with O_CRED_ALLOW
> > flag. This allows the process to pre-open some dirs and switch eUID
> > (and other UIDs/GIDs) to the less-privileged user, while still retaining
> > the possibility to open/create files within the pre-opened directory set.
> >
>
> I’ve been contemplating this, and I want to propose a different solution.
>
> First, the problem Stas is solving is quite narrow and doesn’t
> actually need kernel support: if I want to write a user program that
> sandboxes itself, I have at least three solutions already. I can make
> a userns and a mountns; I can use landlock; and I can have a separate
> process that brokers filesystem access using SCM_RIGHTS.
>
> But what if I want to run a container, where the container can access
> a specific host directory, and the contained application is not aware
> of the exact technology being used? I recently started using
> containers in anger in a production setting, and “anger” was
> definitely the right word: binding part of a filesystem in is
> *miserable*. Getting the DAC rules right is nasty. LSMs are worse.
Nowadays it's extremely simple due tue open_tree(OPEN_TREE_CLONE) and
move_mount(). I rewrote the bind-mount logic in systemd based on that
and util-linux uses that as well now.
https://brauner.io/2023/02/28/mounting-into-mount-namespaces.html
> Podman’s “bind,relabel” feature is IMO utterly disgusting. I think I
> actually gave up on making one of my use cases work on a Fedora
> system.
>
> Here’s what I wanted to do, logically, in production: pick a host
> directory, pick a host *principal* (UID, GID, label, etc), and have
> the *entire container* access the directory as that principal. This is
> what happens automatically if I run the whole container as a userns
> with only a single UID mapped, but I don’t really want to do that for
> a whole variety and of reasons.
You're describing idmapped mounts for the most part which are upstream
and are used in exactly that way by a lot of userspace.
>
> So maybe reimagining Stas’ feature a bit can actually solve this
> problem. Instead of a special dirfd, what if there was a special
> subtree (in the sense of open_tree) that captures a set of creds and
> does all opens inside the subtree using those creds?
That would mean override creds in the VFS layer when accessing a
specific subtree which is a terrible idea imho. Not just because it will
quickly become a potential dos when you do that with a lot of subtrees
it will also have complex interactions with overlayfs.
>
> This isn’t a fully formed proposal, but I *think* it should be
> generally fairly safe for even an unprivileged user to clone a subtree
> with a specific flag set to do this. Maybe a capability would be
> needed (CAP_CAPTURE_CREDS?), but it would be nice to allow delegating
> this to a daemon if a privilege is needed, and getting the API right
> might be a bit tricky.
>
> Then two different things could be done:
>
> 1. The subtree could be used unmounted or via /proc magic links. This
> would be for programs that are aware of this interface.
>
> 2. The subtree could be mounted, and accessed through the mount would
> use the captured creds.
>
> (Hmm. What would a new open_tree() pointing at this special subtree do?)
>
>
> With all this done, if userspace wired it up, a container user could
> do something like:
>
> —bind-capture-creds source=dest
>
> And the contained program would access source *as the user who started
> the container*, and this would just work without relabeling or
> fiddling with owner uids or gids or ACLs, and it would continue to
> work even if the container has multiple dynamically allocated subuids
> mapped (e.g. one for “root” and one for the actual application).
>
> Bonus points for the ability to revoke the creds in an already opened
> subtree. Or even for the creds to automatically revoke themselves when
> the opener exits (or maybe when a specific cred-pinning fd goes away).
>
> (This should work for single files as well as for directories.)
>
> New LSM hooks or extensions of existing hooks might be needed to make
> LSMs comfortable with this.
>
> What do you all think?
I think the problem you're describing is already mostly solved.
^ permalink raw reply
* Re: RFC: Restricting userspace interfaces for CXL fabric management
From: Jonathan Cameron @ 2024-04-29 12:18 UTC (permalink / raw)
To: Harold Johnson
Cc: Dan Williams, linux-cxl, Sreenivas Bagalkote, Brett Henning,
Sumanesh Samanta, linux-kernel, Davidlohr Bueso, Dave Jiang,
Alison Schofield, Vishal Verma, Ira Weiny, linuxarm, linux-api,
Lorenzo Pieralisi, Natu, Mahesh, gregkh
In-Reply-To: <6351024b5d6206c4e9a8cd98d1a09d43@mail.gmail.com>
On Fri, 26 Apr 2024 14:25:29 -0500
Harold Johnson <harold.johnson@broadcom.com> wrote:
> Perhaps a bit more color on a few specifics might be helpful.
>
> I think that there will always be a class of vendor specific APIs/Opcodes
> that are related to an implementation of a standard instead of the
> standard itself. I've been party to discussion on not creating CXL
> defined API/Opcodes that get into the realm of specifying an
> implementation. There are also a class of data that can be collected from
> a specific implementation that is helpful for debug, for health
> monitoring, and perhaps performance monitoring where the implementation
> matters and therefore are not easily abstracted to a standard.
Hi Harold,
Let's divert into a few specifics to give some routes to implementing these.
Some of them are extensions of things already well handled. Tweaks
and extensions in the 'spirit' of the existing spec are both places where
adding some richness to the spec is probably not too difficult and where
there may be some flexibility.
In some cases the definitions in the specification almost certainly
came after your design.
>
> A few examples:
> a) Temperature monitoring of a component or internal chip die
> temperatures. Could CXL define a standard OpCode to gather temperatures,
> yes it could; but is this really part of CXL? Then how many temperature
> elements and what does each element mean? This enters into the
> implementation and therefore is vendor specific. Unless the CXL spec
> starts to define the implementation, something along the lines of "thou
> shall have an average die temperature, rather than specific temperatures
> across a die", etc.
There is a general temperature monitoring and trip thresholds etc in the
spec via get Health Info and event logs. That covers a single value,
so not as extensive as what you refer to but it's a start. Whilst you
are right that the specification should not mandate N temperatures in
specific locations, it would be a reasonable request to allow for a
wider set of monitors with some level of description. The intent
being that generic software can discover what is there and present
that info for logging / monitoring.
Whilst the exact meaning will vary by device, some broad scoping is
easy enough (memory controller vs various FRUs perhaps) and that is useful
to providing generic users space software. hmwon has long handled this
sort of data from whole systems where very similar aspects of 'what does
this monitor actually mean' apply. Sometimes that does need device
specific mapping files - so there is precedence that may be helpful here.
>
> b) Error counters, metrics, internal counters, etc. Could CXL define a
> set of common error counters, absolutely. PCIe has done some of this.
> However, a specific implementation may have counters and error reporting
> that are meaningful only to a specific design and a specific
> implementation rather than a "least common denominator" approach of a
> standard body.
>
> c) Performance counters, metric, indicators, etc. Performance can be very
> implementation specific and tweaking performance is likely to be
> implementation specific. Yes, generic and a least common denominator
> elements could be created, but are likely to limiting in realizing the
> maximum performance of an implementation.
These two are somewhat related.
This selection falls into two broad categories.
- Opaque logging and reporting needed just for debug. There are patches
on list for Vendor Debug Log. They haven't merged yet though I think,
so good to get your input on that series. Intent of that feature is
opaque logs. There will never be a useful general tool for that stuff,
conversely no general userspace tools will use them as a result.
It's likely vendor engineer only territory.
- Counters that useful at runtime.
The CXL PMU spec is rich and flexible. In common with CPU PMUs the perf driver
allows for direct specification of events to count. The same issue with
implementation specific counters occurs in CPUs. Whilst you can keep
the meaning of events opaque, if you do you loose out on useable general
purpose software (e.g. perftool). Alternatively some of this can be
pushed into perf tool description files.
The advantage of pushing counters in to the main specification is that
they work out of the box as the CPMU driver will report those directly
(no need for perf tool to work with raw event codes).
At the moment that driver implements part of the CPMU spec. If there are
features you need (free running counters come to mind) then shout
/ patches welcome. We decided to play wait and see with the CPMU driver as
it wasn't clear if the full flexibility was needed for real devices.
>
> d) Logs, errors and debug information. In addition to spec defined
> logging of CXL topology errors, specific designs will have logs, crash
> dumps, debug data that is very specific to a implementation. There are
> likely to be cases where a product that conforms to a specification like
> CXL, may have features that don't directly have anything to do with CXL,
> but where a standards based management interface can be used to configure,
> manage, and collect data for a non-CXL feature.
There are standard interfaces defined for some of this stuff as well.
Last I checked the discussion revolved around component state dump
only being safe to use if the background command abort was supported,
as that was needed to ensure the kernel was not locked out for an indefinite
amount of time.
If you are looking at other standards being run to configure CXL devices
excellent, but those standards should be accessed using whatever the
appropriate kernel interfaces. Could you give some examples for this one?
>
> e) Innovation. I believe that innovation should be encouraged. There may
> be designs that support CXL, but that also incorporate unique and
> innovative features or functions that might service a niche market. The
> AI space is ripe for innovation and perhaps specialized features that may
> not make sense for the overall CXL specification.
Agreed - but those may need specific drivers.
>
> I think that in most cases Vendor specific opcodes are not used to
> circumvent the standards, but are used when the standards group has no
> interested in driving into the standard certain features that are clearly
> either implementation specific or are vendor specific additions that have
> a specific appeal to a select class of customer, but yet are not relevant
> to a specific standard.
>
> At the end of the day, customer want products that solve a specific
> problem. Sometimes vendor can address market segments or niches that a
> standard group has no interest in supporting. It can also take months,
> and in some cases years to reach an agreement on what standardized feature
> should look like. I also believe that there can be competitive reasons
> why there might be a group that wants to slow down a vendor's
> implementation for fear of losing market share.
Whilst I appreciate the way this can slow down adoption / kernel support
it's a path that is still worth it in the end.
As Dan and others have put it, there are other routes than the main
CXL standard. Those should allow tighter collaboration on smaller topics
to define common standards.
Thanks,
Jonathan
>
> Thanks
> Harold Johnson
>
>
> -----Original Message-----
> From: Jonathan Cameron [mailto:Jonathan.Cameron@Huawei.com]
> Sent: Friday, April 26, 2024 11:54 AM
> To: Dan Williams
> Cc: linux-cxl@vger.kernel.org; Sreenivas Bagalkote; Brett Henning; Harold
> Johnson; Sumanesh Samanta; linux-kernel@vger.kernel.org; Davidlohr Bueso;
> Dave Jiang; Alison Schofield; Vishal Verma; Ira Weiny;
> linuxarm@huawei.com; linux-api@vger.kernel.org; Lorenzo Pieralisi; Natu,
> Mahesh; gregkh@linuxfoundation.org
> Subject: Re: RFC: Restricting userspace interfaces for CXL fabric
> management
>
> On Fri, 26 Apr 2024 09:16:44 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > Jonathan Cameron wrote:
> > [..]
> > > To give people an incentive to play the standards game we have to
> > > provide an alternative. Userspace libraries will provide some
> incentive
> > > to standardize if we have enough vendors (we don't today - so they
> will
> > > do their own libraries), but it is a lot easier to encourage if we
> > > exercise control over the interface.
> >
> > Yes, and I expect you and I are not far off on what can be done
> > here.
> >
> > However, lets cut to a sentiment hanging over this discussion. Referring
> > to vendor specific commands:
> >
> > "CXL spec has them for a reason and they need to be supported."
> >
> > ...that is an aggressive "vendor specific first" sentiment that
> > generates an aggressive "userspace drivers" reaction, because the best
> > way to get around community discussions about what ABI makes sense is
> > userspace drivers.
> >
> > Now, if we can step back to where this discussion started, where typical
> > Linux collaboration shines, and where I think you and I are more aligned
> > than this thread would indicate, is "vendor specific last". Lets
> > carefully consider the vendor specific commands that are candidates to
> > be de facto cross vendor semantics if not de jure standards.
> >
>
> Agreed. I'd go a little further and say I generally have much more warm
> and
> fuzzy feelings when what is a vendor defined command (today) maps to more
> or less the same bit of code for a proposed standards ECN.
>
> IP rules prevent us commenting on specific proposals, but there will be
> things we review quicker and with a lighter touch vs others where we
> ask lots of annoying questions about generality of the feature etc.
> Given the effort we are putting in on the kernel side we all want CXL
> to succeed and will do our best to encourage activities that make that
> more likely. There are other standards bodies available... which may
> make more sense for some features.
>
> Command interfaces are not a good place to compete and maintain secrecy.
> If vendors want to do that, then they don't get the pony of upstream
> support. They get to convince distros to do a custom kernel build for
> them:
> Good luck with that, some of those folk are 'blunt' in their responses to
> such requests.
>
> My proposal is we go forward with a bunch of the CXL spec defined commands
> to show the 'how' and consider specific proposals for upstream support
> of vendor defined commands on a case by case basis (so pretty much
> what you say above). Maybe after a few are done we can formalize some
> rules of thumb help vendors makes such proposals, though maybe some
> will figure out it is a better and longer term solution to do 'standards
> first development'.
>
> I think we do need to look at the safety filtering of tunneled
> commands but don't see that as a particularly tricky addition -
> for the simple non destructive commands at least.
>
> Jonathan
>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 5/7] selftests/bpf: Add uretprobe syscall call from user space test
From: Andrii Nakryiko @ 2024-04-29 16:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <Zi9NPfII8I7nWz6O@krava>
On Mon, Apr 29, 2024 at 12:33 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Apr 26, 2024 at 11:03:29AM -0700, Andrii Nakryiko wrote:
> > On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding test to verify that when called from outside of the
> > > trampoline provided by kernel, the uretprobe syscall will cause
> > > calling process to receive SIGILL signal and the attached bpf
> > > program is no executed.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > .../selftests/bpf/prog_tests/uprobe_syscall.c | 92 +++++++++++++++++++
> > > .../selftests/bpf/progs/uprobe_syscall_call.c | 15 +++
> > > 2 files changed, 107 insertions(+)
> > > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> > >
> >
> > See nits below, but overall LGTM
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > [...]
> >
> > > @@ -219,6 +301,11 @@ static void test_uretprobe_regs_change(void)
> > > {
> > > test__skip();
> > > }
> > > +
> > > +static void test_uretprobe_syscall_call(void)
> > > +{
> > > + test__skip();
> > > +}
> > > #endif
> > >
> > > void test_uprobe_syscall(void)
> > > @@ -228,3 +315,8 @@ void test_uprobe_syscall(void)
> > > if (test__start_subtest("uretprobe_regs_change"))
> > > test_uretprobe_regs_change();
> > > }
> > > +
> > > +void serial_test_uprobe_syscall_call(void)
> >
> > does it need to be serial? non-serial are still run sequentially
> > within a process (there is no multi-threading), it's more about some
> > global effects on system.
>
> plz see below
>
> >
> > > +{
> > > + test_uretprobe_syscall_call();
> > > +}
> > > diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> > > new file mode 100644
> > > index 000000000000..5ea03bb47198
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_call.c
> > > @@ -0,0 +1,15 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include "vmlinux.h"
> > > +#include <bpf/bpf_helpers.h>
> > > +#include <string.h>
> > > +
> > > +struct pt_regs regs;
> > > +
> > > +char _license[] SEC("license") = "GPL";
> > > +
> > > +SEC("uretprobe//proc/self/exe:uretprobe_syscall_call")
> > > +int uretprobe(struct pt_regs *regs)
> > > +{
> > > + bpf_printk("uretprobe called");
> >
> > debugging leftover? we probably don't want to pollute trace_pipe from test
>
> the reason for this is to make sure the bpf program was not executed,
>
> the test makes sure the child gets killed with SIGILL and also that
> the bpf program was not executed by checking the trace_pipe and
> making sure nothing was received
>
> the trace_pipe reading is also why it's serial
you could have attached BPF program from parent process and use a
global variable (and thus eliminate all the trace_pipe system-wide
dependency), but ok, it's fine by me the way this is done
>
> jirka
>
> >
> > > + return 0;
> > > +}
> > > --
> > > 2.44.0
> > >
^ permalink raw reply
* Re: [PATCHv3 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
From: Andrii Nakryiko @ 2024-04-29 16:44 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-kernel, linux-trace-kernel, linux-api, x86, bpf, Song Liu,
Yonghong Song, John Fastabend, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov (AMD), Ingo Molnar, Andy Lutomirski
In-Reply-To: <Zi9OwCwluxTo-Azd@krava>
On Mon, Apr 29, 2024 at 12:39 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Apr 26, 2024 at 11:06:53AM -0700, Andrii Nakryiko wrote:
> > On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding test that adds return uprobe inside 32 bit task
> > > and verify the return uprobe and attached bpf programs
> > > get properly executed.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > tools/testing/selftests/bpf/.gitignore | 1 +
> > > tools/testing/selftests/bpf/Makefile | 6 ++-
> > > .../selftests/bpf/prog_tests/uprobe_syscall.c | 40 +++++++++++++++++++
> > > .../bpf/progs/uprobe_syscall_compat.c | 13 ++++++
> > > 4 files changed, 59 insertions(+), 1 deletion(-)
> > > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> > > index f1aebabfb017..69d71223c0dd 100644
> > > --- a/tools/testing/selftests/bpf/.gitignore
> > > +++ b/tools/testing/selftests/bpf/.gitignore
> > > @@ -45,6 +45,7 @@ test_cpp
> > > /veristat
> > > /sign-file
> > > /uprobe_multi
> > > +/uprobe_compat
> > > *.ko
> > > *.tmp
> > > xskxceiver
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index edc73f8f5aef..d170b63eca62 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -134,7 +134,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> > > xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> > > xdp_features bpf_test_no_cfi.ko
> > >
> > > -TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
> > > +TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi uprobe_compat
> >
> > you need to add uprobe_compat to TRUNNER_EXTRA_FILES as well, no?
>
> ah right
>
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > > index 9233210a4c33..3770254d893b 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > > @@ -11,6 +11,7 @@
> > > #include <sys/wait.h>
> > > #include "uprobe_syscall.skel.h"
> > > #include "uprobe_syscall_call.skel.h"
> > > +#include "uprobe_syscall_compat.skel.h"
> > >
> > > __naked unsigned long uretprobe_regs_trigger(void)
> > > {
> > > @@ -291,6 +292,35 @@ static void test_uretprobe_syscall_call(void)
> > > "read_trace_pipe_iter");
> > > ASSERT_EQ(found, 0, "found");
> > > }
> > > +
> > > +static void trace_pipe_compat_cb(const char *str, void *data)
> > > +{
> > > + if (strstr(str, "uretprobe compat") != NULL)
> > > + (*(int *)data)++;
> > > +}
> > > +
> > > +static void test_uretprobe_compat(void)
> > > +{
> > > + struct uprobe_syscall_compat *skel = NULL;
> > > + int err, found = 0;
> > > +
> > > + skel = uprobe_syscall_compat__open_and_load();
> > > + if (!ASSERT_OK_PTR(skel, "uprobe_syscall_compat__open_and_load"))
> > > + goto cleanup;
> > > +
> > > + err = uprobe_syscall_compat__attach(skel);
> > > + if (!ASSERT_OK(err, "uprobe_syscall_compat__attach"))
> > > + goto cleanup;
> > > +
> > > + system("./uprobe_compat");
> > > +
> > > + ASSERT_OK(read_trace_pipe_iter(trace_pipe_compat_cb, &found, 1000),
> > > + "read_trace_pipe_iter");
> >
> > why so complicated? can't you just set global variable that it was called
>
> hm, we execute separate uprobe_compat (32bit) process that triggers the bpf
> program, so we can't use global variable.. using the trace_pipe was the only
> thing that was easy to do
you need child process to trigger uprobe, but you could have installed
BPF program from parent process (you'd need to make child wait for
parent to be ready, with normal pipe() like we do in other places).
I think generally the less work forked child process does, the better.
All those ASSERT() failures won't produce any output in child process,
unless you run tests in verbose mode, because we haven't implemented
some form of sending all the logs back to the parent process and so
they are completely lost. But that's a separate topic.
Either way, consider using pipe() to coordinate waiting from child on
parent being ready, but otherwise do all the BPF-related heavy lifting
from parent (you can attach BPF programs to specific PID using
bpf_program__attach_uprobe() easily, it's not declarative, but simple
enough).
>
> jirka
>
> >
> > > + ASSERT_EQ(found, 1, "found");
> > > +
> > > +cleanup:
> > > + uprobe_syscall_compat__destroy(skel);
> > > +}
> > > #else
> > > static void test_uretprobe_regs_equal(void)
> > > {
> > > @@ -306,6 +336,11 @@ static void test_uretprobe_syscall_call(void)
> > > {
> > > test__skip();
> > > }
> > > +
> > > +static void test_uretprobe_compat(void)
> > > +{
> > > + test__skip();
> > > +}
> > > #endif
> > >
> > > void test_uprobe_syscall(void)
> > > @@ -320,3 +355,8 @@ void serial_test_uprobe_syscall_call(void)
> > > {
> > > test_uretprobe_syscall_call();
> > > }
> > > +
> > > +void serial_test_uprobe_syscall_compat(void)
> >
> > and then no need for serial_test?
> >
> > > +{
> > > + test_uretprobe_compat();
> > > +}
> > > diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > > new file mode 100644
> > > index 000000000000..f8adde7f08e2
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > > @@ -0,0 +1,13 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/bpf.h>
> > > +#include <bpf/bpf_helpers.h>
> > > +#include <bpf/bpf_tracing.h>
> > > +
> > > +char _license[] SEC("license") = "GPL";
> > > +
> > > +SEC("uretprobe.multi/./uprobe_compat:main")
> > > +int uretprobe_compat(struct pt_regs *ctx)
> > > +{
> > > + bpf_printk("uretprobe compat\n");
> > > + return 0;
> > > +}
> > > --
> > > 2.44.0
> > >
^ permalink raw reply
* Re: [PATCH v3 2/2] fs/xattr: add *at family syscalls
From: Jan Kara @ 2024-04-30 10:09 UTC (permalink / raw)
To: cgzones
Cc: x86, linux-alpha, linux-kernel, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, audit, linux-arch, linux-api,
linux-security-module, selinux, Richard Henderson,
Ivan Kokshaysky, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, Eric Paris,
Arnd Bergmann, Jens Axboe, Pavel Begunkov, Peter Zijlstra,
Sohil Mehta, Palmer Dabbelt, Miklos Szeredi, Nhat Pham,
Casey Schaufler, Florian Fainelli, Kees Cook, Rick Edgecombe,
Mark Rutland, io-uring
In-Reply-To: <20240426162042.191916-1-cgoettsche@seltendoof.de>
On Fri 26-04-24 18:20:14, Christian Göttsche wrote:
> From: Christian Göttsche <cgzones@googlemail.com>
>
> Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
> removexattrat(). Those can be used to operate on extended attributes,
> especially security related ones, either relative to a pinned directory
> or on a file descriptor without read access, avoiding a
> /proc/<pid>/fd/<fd> detour, requiring a mounted procfs.
>
> One use case will be setfiles(8) setting SELinux file contexts
> ("security.selinux") without race conditions and without a file
> descriptor opened with read access requiring SELinux read permission.
>
> Use the do_{name}at() pattern from fs/open.c.
>
> Pass the value of the extended attribute, its length, and for
> setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
> struct xattr_args to not exceed six syscall arguments and not
> merging the AT_* and XATTR_* flags.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The patch looks good to me. Just a few nits below:
> -static int path_setxattr(const char __user *pathname,
> +static int do_setxattrat(int dfd, const char __user *pathname, unsigned int at_flags,
Can we please stay within 80 columns (happens in multiple places in the
patch)? I don't insist but it makes things easier to read in some setups so
I prefer it.
> @@ -852,13 +908,21 @@ listxattr(struct dentry *d, char __user *list, size_t size)
> return error;
> }
>
> -static ssize_t path_listxattr(const char __user *pathname, char __user *list,
> - size_t size, unsigned int lookup_flags)
> +static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user *list,
> + size_t size, int flags)
So I like how in previous syscalls you have 'at_flags', 'lookup_flags', and
'xattr_flags'. That makes things much easier to digest. Can you please stay
with that convention here as well and call this argument 'at_flags'? Also I
think the argument ordering like "dfd, pathname, at_flags, list, size" is
more consistent with other syscalls you define.
> @@ -870,16 +934,22 @@ static ssize_t path_listxattr(const char __user *pathname, char __user *list,
> return error;
> }
>
> +SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname, char __user *, list,
> + size_t, size, int, flags)
> +{
> + return do_listxattrat(dfd, pathname, list, size, flags);
> +}
> +
Same comment as above - "flags" -> "at_flags" and reorder args please.
> @@ -917,13 +987,21 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d,
> return vfs_removexattr(idmap, d, kname);
> }
>
> -static int path_removexattr(const char __user *pathname,
> - const char __user *name, unsigned int lookup_flags)
> +static int do_removexattrat(int dfd, const char __user *pathname,
> + const char __user *name, int flags)
> {
Same comment as above - "flags" -> "at_flags" and reorder args please.
> @@ -939,16 +1017,22 @@ static int path_removexattr(const char __user *pathname,
> return error;
> }
>
> +SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname,
> + const char __user *, name, int, flags)
> +{
Same comment as above - "flags" -> "at_flags" and reorder args please.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH] fs/xattr: unify *at syscalls
From: Christian Göttsche @ 2024-04-30 15:19 UTC (permalink / raw)
To: brauner
Cc: Christian Göttsche, Jan Kara, Alexander Viro, Jan Kara,
Arnd Bergmann, Thomas Gleixner, Kees Cook, Geert Uytterhoeven,
Casey Schaufler, peterz@infradead.org, Sohil Mehta,
Miklos Szeredi, Mark Rutland, linux-fsdevel, linux-kernel,
linux-api
From: Christian Göttsche <cgzones@googlemail.com>
Use the same parameter ordering for all four newly added *xattrat
syscalls:
dirfd, pathname, at_flags, ...
Also consistently use unsigned int as the type for at_flags.
Suggested-by: Jan Kara <jack@suse.com>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
fs/xattr.c | 36 +++++++++++++++++++-----------------
include/linux/syscalls.h | 8 +++++---
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 45603e74c632..454304046d7d 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -931,17 +931,18 @@ listxattr(struct dentry *d, char __user *list, size_t size)
return error;
}
-static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user *list,
- size_t size, int flags)
+static ssize_t do_listxattrat(int dfd, const char __user *pathname,
+ unsigned int at_flags,
+ char __user *list, size_t size)
{
struct path path;
ssize_t error = 0;
int lookup_flags;
- if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
return -EINVAL;
- if (flags & AT_EMPTY_PATH && vfs_empty_path(dfd, pathname)) {
+ if (at_flags & AT_EMPTY_PATH && vfs_empty_path(dfd, pathname)) {
CLASS(fd, f)(dfd);
if (!f.file)
@@ -965,22 +966,23 @@ static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user
return error;
}
-SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname, char __user *, list,
- size_t, size, int, flags)
+SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
+ unsigned int, at_flags,
+ char __user *, list, size_t, size)
{
- return do_listxattrat(dfd, pathname, list, size, flags);
+ return do_listxattrat(dfd, pathname, at_flags, list, size);
}
SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
size_t, size)
{
- return do_listxattrat(AT_FDCWD, pathname, list, size, 0);
+ return do_listxattrat(AT_FDCWD, pathname, 0, list, size);
}
SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
size_t, size)
{
- return do_listxattrat(AT_FDCWD, pathname, list, size, AT_SYMLINK_NOFOLLOW);
+ return do_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
}
SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
@@ -1019,17 +1021,17 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d,
}
static int do_removexattrat(int dfd, const char __user *pathname,
- const char __user *name, int flags)
+ unsigned int at_flags, const char __user *name)
{
struct path path;
int error;
int lookup_flags;
- if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
return -EINVAL;
- lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
- if (flags & AT_EMPTY_PATH)
+ lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+ if (at_flags & AT_EMPTY_PATH)
lookup_flags |= LOOKUP_EMPTY;
retry:
error = user_path_at(dfd, pathname, lookup_flags, &path);
@@ -1049,21 +1051,21 @@ static int do_removexattrat(int dfd, const char __user *pathname,
}
SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname,
- const char __user *, name, int, flags)
+ unsigned int, at_flags, const char __user *, name)
{
- return do_removexattrat(dfd, pathname, name, flags);
+ return do_removexattrat(dfd, pathname, at_flags, name);
}
SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
const char __user *, name)
{
- return do_removexattrat(AT_FDCWD, pathname, name, 0);
+ return do_removexattrat(AT_FDCWD, pathname, 0, name);
}
SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
const char __user *, name)
{
- return do_removexattrat(AT_FDCWD, pathname, name, AT_SYMLINK_NOFOLLOW);
+ return do_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
}
SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e06fffc48535..ca3cba698602 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -356,15 +356,17 @@ asmlinkage long sys_fgetxattr(int fd, const char __user *name,
void __user *value, size_t size);
asmlinkage long sys_listxattr(const char __user *path, char __user *list,
size_t size);
-asmlinkage long sys_listxattrat(int dfd, const char __user *path, char __user *list,
- size_t size, int flags);
+asmlinkage long sys_listxattrat(int dfd, const char __user *path,
+ unsigned int at_flags,
+ char __user *list, size_t size);
asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
size_t size);
asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
asmlinkage long sys_removexattr(const char __user *path,
const char __user *name);
asmlinkage long sys_removexattrat(int dfd, const char __user *path,
- const char __user *name, int flags);
+ unsigned int at_flags,
+ const char __user *name);
asmlinkage long sys_lremovexattr(const char __user *path,
const char __user *name);
asmlinkage long sys_fremovexattr(int fd, const char __user *name);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 0/1] Change failover behavior for DIRECT writes in ext4/block fops
From: Jeremy Bongio @ 2024-05-01 23:15 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, linux-fsdevel, linux-api, linux-block, Jeremy Bongio
From: Jeremy Bongio <jbongio@google.com>
In kernel 6.9, for an O_DIRECT write:
xfs - Will fallback to a sync, buffered write for -ENOTBLK (for reflink CoW)
ext2/3/4 - will fallback to a sync, buffered write for short writes.
If iomap returns -ENOTBLK, write will return status of 0.
block fops - will fallback to a sync, buffered write for short writes.
zonefs - Will fallback to a sync, buffered write for -ENOTBLK.
Will return the bytes written for a short write, no fallback.
Relevant commit:
60263d5889e6d "iomap: fall back to buffered writes for invalidation failures"
In most cases, I think users would be surprised if an O_DIRECT write request
silently resulted in a buffered request.
The iomap_dio_rw() return code -ENOTBLK means page invalidation failed before
submitting the bio.
Is falling back to buffered IO for short writes or -ENOTBLK desirable in ext4
or block fops?
Jeremy Bongio (1):
Remove buffered failover for ext4 and block fops direct writes.
block/fops.c | 3 ---
fs/ext4/file.c | 27 ---------------------------
2 files changed, 30 deletions(-)
--
2.44.0.769.g3c40516874-goog
^ permalink raw reply
* [RFC PATCH 1/1] Remove buffered failover for ext4 and block fops direct writes.
From: Jeremy Bongio @ 2024-05-01 23:15 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, linux-fsdevel, linux-api, linux-block, Jeremy Bongio
In-Reply-To: <20240501231533.3128797-1-bongiojp@gmail.com>
From: Jeremy Bongio <jbongio@google.com>
ext4 and block fops would both failover to syncronous, buffered writes if
the direct IO results in a short write where only a portion of the request
was completed.
This patch changes the behavior to simply return the number of bytes
written if the direct write is short.
---
block/fops.c | 3 ---
fs/ext4/file.c | 27 ---------------------------
2 files changed, 30 deletions(-)
diff --git a/block/fops.c b/block/fops.c
index 0cf8cf72cdfa..d32574ba9d71 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -704,9 +704,6 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (iocb->ki_flags & IOCB_DIRECT) {
ret = blkdev_direct_write(iocb, from);
- if (ret >= 0 && iov_iter_count(from))
- ret = direct_write_fallback(iocb, from, ret,
- blkdev_buffered_write(iocb, from));
} else {
ret = blkdev_buffered_write(iocb, from);
}
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 54d6ff22585c..d0760452a11f 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -595,32 +595,6 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
else
inode_unlock(inode);
- if (ret >= 0 && iov_iter_count(from)) {
- ssize_t err;
- loff_t endbyte;
-
- offset = iocb->ki_pos;
- err = ext4_buffered_write_iter(iocb, from);
- if (err < 0)
- return err;
-
- /*
- * We need to ensure that the pages within the page cache for
- * the range covered by this I/O are written to disk and
- * invalidated. This is in attempt to preserve the expected
- * direct I/O semantics in the case we fallback to buffered I/O
- * to complete off the I/O request.
- */
- ret += err;
- endbyte = offset + err - 1;
- err = filemap_write_and_wait_range(iocb->ki_filp->f_mapping,
- offset, endbyte);
- if (!err)
- invalidate_mapping_pages(iocb->ki_filp->f_mapping,
- offset >> PAGE_SHIFT,
- endbyte >> PAGE_SHIFT);
- }
-
return ret;
}
@@ -958,4 +932,3 @@ const struct inode_operations ext4_file_inode_operations = {
.fileattr_get = ext4_fileattr_get,
.fileattr_set = ext4_fileattr_set,
};
-
--
2.44.0.769.g3c40516874-goog
^ permalink raw reply related
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: André Almeida @ 2024-05-01 23:44 UTC (permalink / raw)
To: Christian Brauner
Cc: Mathieu Desnoyers, Peter Zijlstra, Thomas Gleixner, linux-kernel,
Paul E . McKenney, Boqun Feng, H . Peter Anvin, Paul Turner,
linux-api, Florian Weimer, David.Laight, carlos, Peter Oskolkov,
Alexander Mikhalitsyn, Chris Kennelly, Ingo Molnar, Darren Hart,
Davidlohr Bueso, libc-alpha, Steven Rostedt, Jonathan Corbet,
Noah Goldstein, Daniel Colascione, longman, kernel-dev
In-Reply-To: <20240426-gaumen-zweibeinig-3490b06e86c2@brauner>
Hi Christian,
Em 26/04/2024 07:26, Christian Brauner escreveu:
> On Thu, Apr 25, 2024 at 05:43:31PM -0300, André Almeida wrote:
>> Hi,
>>
>> In the last LPC, Mathieu Desnoyers and I presented[0] a proposal to extend the
>> rseq interface to be able to implement spin locks in userspace correctly. Thomas
>> Gleixner agreed that this is something that Linux could improve, but asked for
>> an alternative proposal first: a futex operation that allows to spin a user
>> lock inside the kernel. This patchset implements a prototype of this idea for
>> further discussion.
>>
>> With FUTEX2_SPIN flag set during a futex_wait(), the futex value is expected to
>> be the PID of the lock owner. Then, the kernel gets the task_struct of the
>> corresponding PID, and checks if it's running. It spins until the futex
>> is awaken, the task is scheduled out or if a timeout happens. If the lock owner
>> is scheduled out at any time, then the syscall follows the normal path of
>> sleeping as usual.
>>
>> If the futex is awaken and we are spinning, we can return to userspace quickly,
>> avoid the scheduling out and in again to wake from a futex_wait(), thus
>> speeding up the wait operation.
>>
>> I didn't manage to find a good mechanism to prevent race conditions between
>> setting *futex = PID in userspace and doing find_get_task_by_vpid(PID) in kernel
>> space, giving that there's enough room for the original PID owner exit and such
>> PID to be relocated to another unrelated task in the system. I didn't performed
>
> One option would be to also allow pidfds. Starting with v6.9 they can be
> used to reference individual threads.
>
> So for the really fast case where you have multiple threads and you
> somehow may really do care about the impact of the atomic_long_inc() on
> pidfd_file->f_count during fdget() (for the single-threaded case the
> increment is elided), callers can pass the TID. But in cases where the
> inc and put aren't a performance sensitive, you can use pidfds.
>
Thank you very much for making the effort here, much appreciated :)
While I agree that pidfds would fix the PID race conditions, I will move
this interface to support TIDs instead, as noted by Florian and Peter.
With TID the race conditions are diminished I reckon?
^ permalink raw reply
* Re: [RFC PATCH 1/1] Remove buffered failover for ext4 and block fops direct writes.
From: Christoph Hellwig @ 2024-05-02 5:45 UTC (permalink / raw)
To: Jeremy Bongio
Cc: Ted Tso, linux-ext4, linux-fsdevel, linux-api, linux-block,
Jeremy Bongio
In-Reply-To: <20240501231533.3128797-2-bongiojp@gmail.com>
On Wed, May 01, 2024 at 04:15:33PM -0700, Jeremy Bongio wrote:
> From: Jeremy Bongio <jbongio@google.com>
>
> ext4 and block fops would both failover to syncronous, buffered writes if
> the direct IO results in a short write where only a portion of the request
> was completed.
>
> This patch changes the behavior to simply return the number of bytes
> written if the direct write is short.
Please don't combine ext4 and block changes in a single patch. Please
also explain why you want to change things.
AFAIK this is simply the historic behavior of the old direct I/O code
that's been around forever. I think the XFS semantics make a lot more
sense, but people might rely on this one way or another.
^ permalink raw reply
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Christian Brauner @ 2024-05-02 8:45 UTC (permalink / raw)
To: André Almeida
Cc: Mathieu Desnoyers, Peter Zijlstra, Thomas Gleixner, linux-kernel,
Paul E . McKenney, Boqun Feng, H . Peter Anvin, Paul Turner,
linux-api, Florian Weimer, David.Laight, carlos, Peter Oskolkov,
Alexander Mikhalitsyn, Chris Kennelly, Ingo Molnar, Darren Hart,
Davidlohr Bueso, libc-alpha, Steven Rostedt, Jonathan Corbet,
Noah Goldstein, Daniel Colascione, longman, kernel-dev
In-Reply-To: <f052ff72-72c9-4b83-9285-2cd9d52e5f72@igalia.com>
On Wed, May 01, 2024 at 08:44:36PM -0300, André Almeida wrote:
> Hi Christian,
>
> Em 26/04/2024 07:26, Christian Brauner escreveu:
> > On Thu, Apr 25, 2024 at 05:43:31PM -0300, André Almeida wrote:
> > > Hi,
> > >
> > > In the last LPC, Mathieu Desnoyers and I presented[0] a proposal to extend the
> > > rseq interface to be able to implement spin locks in userspace correctly. Thomas
> > > Gleixner agreed that this is something that Linux could improve, but asked for
> > > an alternative proposal first: a futex operation that allows to spin a user
> > > lock inside the kernel. This patchset implements a prototype of this idea for
> > > further discussion.
> > >
> > > With FUTEX2_SPIN flag set during a futex_wait(), the futex value is expected to
> > > be the PID of the lock owner. Then, the kernel gets the task_struct of the
> > > corresponding PID, and checks if it's running. It spins until the futex
> > > is awaken, the task is scheduled out or if a timeout happens. If the lock owner
> > > is scheduled out at any time, then the syscall follows the normal path of
> > > sleeping as usual.
> > >
> > > If the futex is awaken and we are spinning, we can return to userspace quickly,
> > > avoid the scheduling out and in again to wake from a futex_wait(), thus
> > > speeding up the wait operation.
> > >
> > > I didn't manage to find a good mechanism to prevent race conditions between
> > > setting *futex = PID in userspace and doing find_get_task_by_vpid(PID) in kernel
> > > space, giving that there's enough room for the original PID owner exit and such
> > > PID to be relocated to another unrelated task in the system. I didn't performed
> >
> > One option would be to also allow pidfds. Starting with v6.9 they can be
> > used to reference individual threads.
> >
> > So for the really fast case where you have multiple threads and you
> > somehow may really do care about the impact of the atomic_long_inc() on
> > pidfd_file->f_count during fdget() (for the single-threaded case the
> > increment is elided), callers can pass the TID. But in cases where the
> > inc and put aren't a performance sensitive, you can use pidfds.
> >
>
> Thank you very much for making the effort here, much appreciated :)
>
> While I agree that pidfds would fix the PID race conditions, I will move
> this interface to support TIDs instead, as noted by Florian and Peter. With
> TID the race conditions are diminished I reckon?
Unless I'm missing something the question here is PID (as in TGID aka
thread-group leader id gotten via getpid()) vs TID (thread specific id
gotten via gettid()). You want the thread-specific id as you want to
interact with the futex state of a specific thread not the thread-group
leader.
Aside from that TIDs are subject to the same race conditions that PIDs
are. They are allocated from the same pool (see alloc_pid()).
^ permalink raw reply
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Florian Weimer @ 2024-05-02 9:51 UTC (permalink / raw)
To: Christian Brauner
Cc: André Almeida, Mathieu Desnoyers, Peter Zijlstra,
Thomas Gleixner, linux-kernel, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, David.Laight, carlos,
Peter Oskolkov, Alexander Mikhalitsyn, Chris Kennelly,
Ingo Molnar, Darren Hart, Davidlohr Bueso, libc-alpha,
Steven Rostedt, Jonathan Corbet, Noah Goldstein,
Daniel Colascione, longman, kernel-dev
In-Reply-To: <20240502-gezeichnet-besonderen-d277879cd669@brauner>
* Christian Brauner:
> Unless I'm missing something the question here is PID (as in TGID aka
> thread-group leader id gotten via getpid()) vs TID (thread specific id
> gotten via gettid()). You want the thread-specific id as you want to
> interact with the futex state of a specific thread not the thread-group
> leader.
>
> Aside from that TIDs are subject to the same race conditions that PIDs
> are. They are allocated from the same pool (see alloc_pid()).
For most mutex types (but not robust mutexes), it is undefined in
userspace if a thread exits while it has locked a mutex. Such a usage
condition would ensure that the race doesn't happen, I believe.
From a glibc perspective, we typically cannot use long-term file
descriptors (that are kept open across function calls) because some
applications do not expect them, or even close them behind our back.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Christian Brauner @ 2024-05-02 10:14 UTC (permalink / raw)
To: Florian Weimer
Cc: André Almeida, Mathieu Desnoyers, Peter Zijlstra,
Thomas Gleixner, linux-kernel, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, David.Laight, carlos,
Peter Oskolkov, Alexander Mikhalitsyn, Chris Kennelly,
Ingo Molnar, Darren Hart, Davidlohr Bueso, libc-alpha,
Steven Rostedt, Jonathan Corbet, Noah Goldstein,
Daniel Colascione, longman, kernel-dev
In-Reply-To: <8734r0o81v.fsf@oldenburg.str.redhat.com>
On Thu, May 02, 2024 at 11:51:56AM +0200, Florian Weimer wrote:
> * Christian Brauner:
>
> > Unless I'm missing something the question here is PID (as in TGID aka
> > thread-group leader id gotten via getpid()) vs TID (thread specific id
> > gotten via gettid()). You want the thread-specific id as you want to
> > interact with the futex state of a specific thread not the thread-group
> > leader.
> >
> > Aside from that TIDs are subject to the same race conditions that PIDs
> > are. They are allocated from the same pool (see alloc_pid()).
>
> For most mutex types (but not robust mutexes), it is undefined in
> userspace if a thread exits while it has locked a mutex. Such a usage
> condition would ensure that the race doesn't happen, I believe.
The argument is a bit shaky imho because the race not being able to
happen is predicated on no one being careless enough to exit with a
mutex held. That doesn't do anything against someone doing it on
purpose.
>
> From a glibc perspective, we typically cannot use long-term file
> descriptors (that are kept open across function calls) because some
> applications do not expect them, or even close them behind our back.
Yeah, good point. Note, I suggested it as an extension not as a
replacement for the TID. I still think it would be a useful extension in
general.
^ permalink raw reply
* Re: [PATCH] fs/xattr: unify *at syscalls
From: Jan Kara @ 2024-05-02 10:37 UTC (permalink / raw)
To: cgzones
Cc: brauner, Jan Kara, Alexander Viro, Jan Kara, Arnd Bergmann,
Thomas Gleixner, Kees Cook, Geert Uytterhoeven, Casey Schaufler,
peterz@infradead.org, Sohil Mehta, Miklos Szeredi, Mark Rutland,
linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <20240430151917.30036-1-cgoettsche@seltendoof.de>
On Tue 30-04-24 17:19:14, Christian Göttsche wrote:
> From: Christian Göttsche <cgzones@googlemail.com>
>
> Use the same parameter ordering for all four newly added *xattrat
> syscalls:
>
> dirfd, pathname, at_flags, ...
>
> Also consistently use unsigned int as the type for at_flags.
>
> Suggested-by: Jan Kara <jack@suse.com>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Thanks! The change looks good to me. Christian, do you plan to fold this
into the series you've taken to your tree?
Honza
> ---
> fs/xattr.c | 36 +++++++++++++++++++-----------------
> include/linux/syscalls.h | 8 +++++---
> 2 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 45603e74c632..454304046d7d 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -931,17 +931,18 @@ listxattr(struct dentry *d, char __user *list, size_t size)
> return error;
> }
>
> -static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user *list,
> - size_t size, int flags)
> +static ssize_t do_listxattrat(int dfd, const char __user *pathname,
> + unsigned int at_flags,
> + char __user *list, size_t size)
> {
> struct path path;
> ssize_t error = 0;
> int lookup_flags;
>
> - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> return -EINVAL;
>
> - if (flags & AT_EMPTY_PATH && vfs_empty_path(dfd, pathname)) {
> + if (at_flags & AT_EMPTY_PATH && vfs_empty_path(dfd, pathname)) {
> CLASS(fd, f)(dfd);
>
> if (!f.file)
> @@ -965,22 +966,23 @@ static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user
> return error;
> }
>
> -SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname, char __user *, list,
> - size_t, size, int, flags)
> +SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
> + unsigned int, at_flags,
> + char __user *, list, size_t, size)
> {
> - return do_listxattrat(dfd, pathname, list, size, flags);
> + return do_listxattrat(dfd, pathname, at_flags, list, size);
> }
>
> SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
> size_t, size)
> {
> - return do_listxattrat(AT_FDCWD, pathname, list, size, 0);
> + return do_listxattrat(AT_FDCWD, pathname, 0, list, size);
> }
>
> SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
> size_t, size)
> {
> - return do_listxattrat(AT_FDCWD, pathname, list, size, AT_SYMLINK_NOFOLLOW);
> + return do_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
> }
>
> SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
> @@ -1019,17 +1021,17 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d,
> }
>
> static int do_removexattrat(int dfd, const char __user *pathname,
> - const char __user *name, int flags)
> + unsigned int at_flags, const char __user *name)
> {
> struct path path;
> int error;
> int lookup_flags;
>
> - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> return -EINVAL;
>
> - lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> - if (flags & AT_EMPTY_PATH)
> + lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> + if (at_flags & AT_EMPTY_PATH)
> lookup_flags |= LOOKUP_EMPTY;
> retry:
> error = user_path_at(dfd, pathname, lookup_flags, &path);
> @@ -1049,21 +1051,21 @@ static int do_removexattrat(int dfd, const char __user *pathname,
> }
>
> SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname,
> - const char __user *, name, int, flags)
> + unsigned int, at_flags, const char __user *, name)
> {
> - return do_removexattrat(dfd, pathname, name, flags);
> + return do_removexattrat(dfd, pathname, at_flags, name);
> }
>
> SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
> const char __user *, name)
> {
> - return do_removexattrat(AT_FDCWD, pathname, name, 0);
> + return do_removexattrat(AT_FDCWD, pathname, 0, name);
> }
>
> SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
> const char __user *, name)
> {
> - return do_removexattrat(AT_FDCWD, pathname, name, AT_SYMLINK_NOFOLLOW);
> + return do_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
> }
>
> SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e06fffc48535..ca3cba698602 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -356,15 +356,17 @@ asmlinkage long sys_fgetxattr(int fd, const char __user *name,
> void __user *value, size_t size);
> asmlinkage long sys_listxattr(const char __user *path, char __user *list,
> size_t size);
> -asmlinkage long sys_listxattrat(int dfd, const char __user *path, char __user *list,
> - size_t size, int flags);
> +asmlinkage long sys_listxattrat(int dfd, const char __user *path,
> + unsigned int at_flags,
> + char __user *list, size_t size);
> asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
> size_t size);
> asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
> asmlinkage long sys_removexattr(const char __user *path,
> const char __user *name);
> asmlinkage long sys_removexattrat(int dfd, const char __user *path,
> - const char __user *name, int flags);
> + unsigned int at_flags,
> + const char __user *name);
> asmlinkage long sys_lremovexattr(const char __user *path,
> const char __user *name);
> asmlinkage long sys_fremovexattr(int fd, const char __user *name);
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [RFC PATCH 0/1] Add FUTEX_SPIN operation
From: Florian Weimer @ 2024-05-02 10:39 UTC (permalink / raw)
To: Christian Brauner
Cc: André Almeida, Mathieu Desnoyers, Peter Zijlstra,
Thomas Gleixner, linux-kernel, Paul E . McKenney, Boqun Feng,
H . Peter Anvin, Paul Turner, linux-api, David.Laight, carlos,
Peter Oskolkov, Alexander Mikhalitsyn, Chris Kennelly,
Ingo Molnar, Darren Hart, Davidlohr Bueso, libc-alpha,
Steven Rostedt, Jonathan Corbet, Noah Goldstein,
Daniel Colascione, longman, kernel-dev
In-Reply-To: <20240502-sporen-pirschen-039688cd9efe@brauner>
* Christian Brauner:
>> From a glibc perspective, we typically cannot use long-term file
>> descriptors (that are kept open across function calls) because some
>> applications do not expect them, or even close them behind our back.
>
> Yeah, good point. Note, I suggested it as an extension not as a
> replacement for the TID. I still think it would be a useful extension in
> general.
Applications will need a way to determine when it is safe to close the
pidfd, though. If we automate this in glibc (in the same way we handle
thread stack deallocation for example), I think we are essentially back
to square one, except that pidfd collisions are much more likely than
TID collisions, especially on systems that have adjusted kernel.pid_max.
(File descriptor allocation is designed to maximize collisions, after
all.)
Thanks,
Florian
^ permalink raw reply
* [PATCHv4 bpf-next 0/7] uprobe: uretprobe speed up
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
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.
v4 changes:
- added acks [Oleg,Andrii,Masami]
- reworded the man page and adding more info to NOTE section [Masami]
- rewrote bpf tests not to use trace_pipe [Andrii]
- cc-ed linux-man list
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 | 7 +-
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 ++++++++++++++++++++++++++++-
tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 382 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c | 17 +++++
14 files changed, 691 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_executed.c
Jiri Olsa (1):
man2: Add uretprobe syscall page
man2/uretprobe.2 | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 man2/uretprobe.2
^ permalink raw reply
* [PATCHv4 bpf-next 1/7] uprobe: Wire up uretprobe system call
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-1-jolsa@kernel.org>
Wiring up uretprobe system call, which comes in following changes.
We need to do the wiring before, because the uretprobe implementation
needs the syscall number.
Note at the moment uretprobe syscall is supported only for native
64-bit process.
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 2 ++
include/uapi/asm-generic/unistd.h | 5 ++++-
kernel/sys_ni.c | 2 ++
4 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f4147f..af0a33ab06ee 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -383,6 +383,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 64 uretprobe sys_uretprobe
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e619ac10cd23..5318e0e76799 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -972,6 +972,8 @@ asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags);
/* x86 */
asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int on);
+asmlinkage long sys_uretprobe(void);
+
/* pciconfig: alpha, arm, arm64, ia64, sparc */
asmlinkage long sys_pciconfig_read(unsigned long bus, unsigned long dfn,
unsigned long off, unsigned long len,
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 75f00965ab15..8a747cd1d735 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -842,8 +842,11 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+#define __NR_uretprobe 462
+__SYSCALL(__NR_uretprobe, sys_uretprobe)
+
#undef __NR_syscalls
-#define __NR_syscalls 462
+#define __NR_syscalls 463
/*
* 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index faad00cce269..be6195e0d078 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -391,3 +391,5 @@ COND_SYSCALL(setuid16);
/* restartable sequence */
COND_SYSCALL(rseq);
+
+COND_SYSCALL(uretprobe);
--
2.44.0
^ permalink raw reply related
* [PATCHv4 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-1-jolsa@kernel.org>
Adding uretprobe syscall instead of trap to speed up return probe.
At the moment the uretprobe setup/path is:
- install entry uprobe
- when the uprobe is hit, it overwrites probed function's return address
on stack with address of the trampoline that contains breakpoint
instruction
- the breakpoint trap code handles the uretprobe consumers execution and
jumps back to original return address
This patch replaces the above trampoline's breakpoint instruction with new
ureprobe syscall call. This syscall does exactly the same job as the trap
with some more extra work:
- syscall trampoline must save original value for rax/r11/rcx registers
on stack - rax is set to syscall number and r11/rcx are changed and
used by syscall instruction
- the syscall code reads the original values of those registers and
restore those values in task's pt_regs area
- only caller from trampoline exposed in '[uprobes]' is allowed,
the process will receive SIGILL signal otherwise
Even with some extra work, using the uretprobes syscall shows speed
improvement (compared to using standard breakpoint):
On Intel (11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz)
current:
uretprobe-nop : 1.498 ± 0.000M/s
uretprobe-push : 1.448 ± 0.001M/s
uretprobe-ret : 0.816 ± 0.001M/s
with the fix:
uretprobe-nop : 1.969 ± 0.002M/s < 31% speed up
uretprobe-push : 1.910 ± 0.000M/s < 31% speed up
uretprobe-ret : 0.934 ± 0.000M/s < 14% speed up
On Amd (AMD Ryzen 7 5700U)
current:
uretprobe-nop : 0.778 ± 0.001M/s
uretprobe-push : 0.744 ± 0.001M/s
uretprobe-ret : 0.540 ± 0.001M/s
with the fix:
uretprobe-nop : 0.860 ± 0.001M/s < 10% speed up
uretprobe-push : 0.818 ± 0.001M/s < 10% speed up
uretprobe-ret : 0.578 ± 0.000M/s < 7% speed up
The performance test spawns a thread that runs loop which triggers
uprobe with attached bpf program that increments the counter that
gets printed in results above.
The uprobe (and uretprobe) kind is determined by which instruction
is being patched with breakpoint instruction. That's also important
for uretprobes, because uprobe is installed for each uretprobe.
The performance test is part of bpf selftests:
tools/testing/selftests/bpf/run_bench_uprobes.sh
Note at the moment uretprobe syscall is supported only for native
64-bit process, compat process still uses standard breakpoint.
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/kernel/uprobes.c | 115 ++++++++++++++++++++++++++++++++++++++
include/linux/uprobes.h | 3 +
kernel/events/uprobes.c | 24 +++++---
3 files changed, 135 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 6c07f6daaa22..81e6ee95784d 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -12,6 +12,7 @@
#include <linux/ptrace.h>
#include <linux/uprobes.h>
#include <linux/uaccess.h>
+#include <linux/syscalls.h>
#include <linux/kdebug.h>
#include <asm/processor.h>
@@ -308,6 +309,120 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
}
#ifdef CONFIG_X86_64
+
+asm (
+ ".pushsection .rodata\n"
+ ".global uretprobe_syscall_entry\n"
+ "uretprobe_syscall_entry:\n"
+ "pushq %rax\n"
+ "pushq %rcx\n"
+ "pushq %r11\n"
+ "movq $" __stringify(__NR_uretprobe) ", %rax\n"
+ "syscall\n"
+ ".global uretprobe_syscall_check\n"
+ "uretprobe_syscall_check:\n"
+ "popq %r11\n"
+ "popq %rcx\n"
+
+ /* The uretprobe syscall replaces stored %rax value with final
+ * return address, so we don't restore %rax in here and just
+ * call ret.
+ */
+ "retq\n"
+ ".global uretprobe_syscall_end\n"
+ "uretprobe_syscall_end:\n"
+ ".popsection\n"
+);
+
+extern u8 uretprobe_syscall_entry[];
+extern u8 uretprobe_syscall_check[];
+extern u8 uretprobe_syscall_end[];
+
+void *arch_uprobe_trampoline(unsigned long *psize)
+{
+ static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
+ struct pt_regs *regs = task_pt_regs(current);
+
+ /*
+ * At the moment the uretprobe syscall trampoline is supported
+ * only for native 64-bit process, the compat process still uses
+ * standard breakpoint.
+ */
+ if (user_64bit_mode(regs)) {
+ *psize = uretprobe_syscall_end - uretprobe_syscall_entry;
+ return uretprobe_syscall_entry;
+ }
+
+ *psize = UPROBE_SWBP_INSN_SIZE;
+ return &insn;
+}
+
+static unsigned long trampoline_check_ip(void)
+{
+ unsigned long tramp = uprobe_get_trampoline_vaddr();
+
+ return tramp + (uretprobe_syscall_check - uretprobe_syscall_entry);
+}
+
+SYSCALL_DEFINE0(uretprobe)
+{
+ struct pt_regs *regs = task_pt_regs(current);
+ unsigned long err, ip, sp, r11_cx_ax[3];
+
+ if (regs->ip != trampoline_check_ip())
+ goto sigill;
+
+ err = copy_from_user(r11_cx_ax, (void __user *)regs->sp, sizeof(r11_cx_ax));
+ if (err)
+ goto sigill;
+
+ /* expose the "right" values of r11/cx/ax/sp to uprobe_consumer/s */
+ regs->r11 = r11_cx_ax[0];
+ regs->cx = r11_cx_ax[1];
+ regs->ax = r11_cx_ax[2];
+ regs->sp += sizeof(r11_cx_ax);
+ regs->orig_ax = -1;
+
+ ip = regs->ip;
+ sp = regs->sp;
+
+ uprobe_handle_trampoline(regs);
+
+ /*
+ * uprobe_consumer has changed sp, we can do nothing,
+ * just return via iret
+ */
+ if (regs->sp != sp)
+ return regs->ax;
+ regs->sp -= sizeof(r11_cx_ax);
+
+ /* for the case uprobe_consumer has changed r11/cx */
+ r11_cx_ax[0] = regs->r11;
+ r11_cx_ax[1] = regs->cx;
+
+ /*
+ * ax register is passed through as return value, so we can use
+ * its space on stack for ip value and jump to it through the
+ * trampoline's ret instruction
+ */
+ r11_cx_ax[2] = regs->ip;
+ regs->ip = ip;
+
+ err = copy_to_user((void __user *)regs->sp, r11_cx_ax, sizeof(r11_cx_ax));
+ if (err)
+ goto sigill;
+
+ /* ensure sysret, see do_syscall_64() */
+ regs->r11 = regs->flags;
+ regs->cx = regs->ip;
+
+ return regs->ax;
+
+sigill:
+ force_sig(SIGILL);
+ return -1;
+}
+
/*
* If arch_uprobe->insn doesn't use rip-relative addressing, return
* immediately. Otherwise, rewrite the instruction so that it accesses
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index f46e0ca0169c..b503fafb7fb3 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -138,6 +138,9 @@ extern bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check c
extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len);
+extern void uprobe_handle_trampoline(struct pt_regs *regs);
+extern void *arch_uprobe_trampoline(unsigned long *psize);
+extern unsigned long uprobe_get_trampoline_vaddr(void);
#else /* !CONFIG_UPROBES */
struct uprobes_state {
};
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index e4834d23e1d1..c550449d66be 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1474,11 +1474,20 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
return ret;
}
+void * __weak arch_uprobe_trampoline(unsigned long *psize)
+{
+ static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
+
+ *psize = UPROBE_SWBP_INSN_SIZE;
+ return &insn;
+}
+
static struct xol_area *__create_xol_area(unsigned long vaddr)
{
struct mm_struct *mm = current->mm;
- uprobe_opcode_t insn = UPROBE_SWBP_INSN;
+ unsigned long insns_size;
struct xol_area *area;
+ void *insns;
area = kmalloc(sizeof(*area), GFP_KERNEL);
if (unlikely(!area))
@@ -1502,7 +1511,8 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
/* Reserve the 1st slot for get_trampoline_vaddr() */
set_bit(0, area->bitmap);
atomic_set(&area->slot_count, 1);
- arch_uprobe_copy_ixol(area->pages[0], 0, &insn, UPROBE_SWBP_INSN_SIZE);
+ insns = arch_uprobe_trampoline(&insns_size);
+ arch_uprobe_copy_ixol(area->pages[0], 0, insns, insns_size);
if (!xol_add_vma(mm, area))
return area;
@@ -1827,7 +1837,7 @@ void uprobe_copy_process(struct task_struct *t, unsigned long flags)
*
* Returns -1 in case the xol_area is not allocated.
*/
-static unsigned long get_trampoline_vaddr(void)
+unsigned long uprobe_get_trampoline_vaddr(void)
{
struct xol_area *area;
unsigned long trampoline_vaddr = -1;
@@ -1878,7 +1888,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
if (!ri)
return;
- trampoline_vaddr = get_trampoline_vaddr();
+ trampoline_vaddr = uprobe_get_trampoline_vaddr();
orig_ret_vaddr = arch_uretprobe_hijack_return_addr(trampoline_vaddr, regs);
if (orig_ret_vaddr == -1)
goto fail;
@@ -2123,7 +2133,7 @@ static struct return_instance *find_next_ret_chain(struct return_instance *ri)
return ri;
}
-static void handle_trampoline(struct pt_regs *regs)
+void uprobe_handle_trampoline(struct pt_regs *regs)
{
struct uprobe_task *utask;
struct return_instance *ri, *next;
@@ -2187,8 +2197,8 @@ static void handle_swbp(struct pt_regs *regs)
int is_swbp;
bp_vaddr = uprobe_get_swbp_addr(regs);
- if (bp_vaddr == get_trampoline_vaddr())
- return handle_trampoline(regs);
+ if (bp_vaddr == uprobe_get_trampoline_vaddr())
+ return uprobe_handle_trampoline(regs);
uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
if (!uprobe) {
--
2.44.0
^ permalink raw reply related
* [PATCHv4 bpf-next 3/7] selftests/bpf: Add uretprobe syscall test for regs integrity
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-1-jolsa@kernel.org>
Add uretprobe syscall test that compares register values before
and after the uretprobe is hit. It also compares the register
values seen from attached bpf program.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/include/linux/compiler.h | 4 +
.../selftests/bpf/prog_tests/uprobe_syscall.c | 163 ++++++++++++++++++
.../selftests/bpf/progs/uprobe_syscall.c | 15 ++
3 files changed, 182 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 8a63a9913495..6f7f22ac9da5 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -62,6 +62,10 @@
#define __nocf_check __attribute__((nocf_check))
#endif
+#ifndef __naked
+#define __naked __attribute__((__naked__))
+#endif
+
/* Are two types/vars the same type (ignoring qualifiers)? */
#ifndef __same_type
# define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
new file mode 100644
index 000000000000..311ac19d8992
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#ifdef __x86_64__
+
+#include <unistd.h>
+#include <asm/ptrace.h>
+#include <linux/compiler.h>
+#include "uprobe_syscall.skel.h"
+
+__naked unsigned long uretprobe_regs_trigger(void)
+{
+ asm volatile (
+ "movq $0xdeadbeef, %rax\n"
+ "ret\n"
+ );
+}
+
+__naked void uretprobe_regs(struct pt_regs *before, struct pt_regs *after)
+{
+ asm volatile (
+ "movq %r15, 0(%rdi)\n"
+ "movq %r14, 8(%rdi)\n"
+ "movq %r13, 16(%rdi)\n"
+ "movq %r12, 24(%rdi)\n"
+ "movq %rbp, 32(%rdi)\n"
+ "movq %rbx, 40(%rdi)\n"
+ "movq %r11, 48(%rdi)\n"
+ "movq %r10, 56(%rdi)\n"
+ "movq %r9, 64(%rdi)\n"
+ "movq %r8, 72(%rdi)\n"
+ "movq %rax, 80(%rdi)\n"
+ "movq %rcx, 88(%rdi)\n"
+ "movq %rdx, 96(%rdi)\n"
+ "movq %rsi, 104(%rdi)\n"
+ "movq %rdi, 112(%rdi)\n"
+ "movq $0, 120(%rdi)\n" /* orig_rax */
+ "movq $0, 128(%rdi)\n" /* rip */
+ "movq $0, 136(%rdi)\n" /* cs */
+ "pushf\n"
+ "pop %rax\n"
+ "movq %rax, 144(%rdi)\n" /* eflags */
+ "movq %rsp, 152(%rdi)\n" /* rsp */
+ "movq $0, 160(%rdi)\n" /* ss */
+
+ /* save 2nd argument */
+ "pushq %rsi\n"
+ "call uretprobe_regs_trigger\n"
+
+ /* save return value and load 2nd argument pointer to rax */
+ "pushq %rax\n"
+ "movq 8(%rsp), %rax\n"
+
+ "movq %r15, 0(%rax)\n"
+ "movq %r14, 8(%rax)\n"
+ "movq %r13, 16(%rax)\n"
+ "movq %r12, 24(%rax)\n"
+ "movq %rbp, 32(%rax)\n"
+ "movq %rbx, 40(%rax)\n"
+ "movq %r11, 48(%rax)\n"
+ "movq %r10, 56(%rax)\n"
+ "movq %r9, 64(%rax)\n"
+ "movq %r8, 72(%rax)\n"
+ "movq %rcx, 88(%rax)\n"
+ "movq %rdx, 96(%rax)\n"
+ "movq %rsi, 104(%rax)\n"
+ "movq %rdi, 112(%rax)\n"
+ "movq $0, 120(%rax)\n" /* orig_rax */
+ "movq $0, 128(%rax)\n" /* rip */
+ "movq $0, 136(%rax)\n" /* cs */
+
+ /* restore return value and 2nd argument */
+ "pop %rax\n"
+ "pop %rsi\n"
+
+ "movq %rax, 80(%rsi)\n"
+
+ "pushf\n"
+ "pop %rax\n"
+
+ "movq %rax, 144(%rsi)\n" /* eflags */
+ "movq %rsp, 152(%rsi)\n" /* rsp */
+ "movq $0, 160(%rsi)\n" /* ss */
+ "ret\n"
+);
+}
+
+static void test_uretprobe_regs_equal(void)
+{
+ struct uprobe_syscall *skel = NULL;
+ struct pt_regs before = {}, after = {};
+ unsigned long *pb = (unsigned long *) &before;
+ unsigned long *pa = (unsigned long *) &after;
+ unsigned long *pp;
+ unsigned int i, cnt;
+ int err;
+
+ skel = uprobe_syscall__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "uprobe_syscall__open_and_load"))
+ goto cleanup;
+
+ err = uprobe_syscall__attach(skel);
+ if (!ASSERT_OK(err, "uprobe_syscall__attach"))
+ goto cleanup;
+
+ uretprobe_regs(&before, &after);
+
+ pp = (unsigned long *) &skel->bss->regs;
+ cnt = sizeof(before)/sizeof(*pb);
+
+ for (i = 0; i < cnt; i++) {
+ unsigned int offset = i * sizeof(unsigned long);
+
+ /*
+ * Check register before and after uretprobe_regs_trigger call
+ * that triggers the uretprobe.
+ */
+ switch (offset) {
+ case offsetof(struct pt_regs, rax):
+ ASSERT_EQ(pa[i], 0xdeadbeef, "return value");
+ break;
+ default:
+ if (!ASSERT_EQ(pb[i], pa[i], "register before-after value check"))
+ fprintf(stdout, "failed register offset %u\n", offset);
+ }
+
+ /*
+ * Check register seen from bpf program and register after
+ * uretprobe_regs_trigger call
+ */
+ switch (offset) {
+ /*
+ * These values will be different (not set in uretprobe_regs),
+ * we don't care.
+ */
+ case offsetof(struct pt_regs, orig_rax):
+ case offsetof(struct pt_regs, rip):
+ case offsetof(struct pt_regs, cs):
+ case offsetof(struct pt_regs, rsp):
+ case offsetof(struct pt_regs, ss):
+ break;
+ default:
+ if (!ASSERT_EQ(pp[i], pa[i], "register prog-after value check"))
+ fprintf(stdout, "failed register offset %u\n", offset);
+ }
+ }
+
+cleanup:
+ uprobe_syscall__destroy(skel);
+}
+#else
+static void test_uretprobe_regs_equal(void)
+{
+ test__skip();
+}
+#endif
+
+void test_uprobe_syscall(void)
+{
+ if (test__start_subtest("uretprobe_regs_equal"))
+ test_uretprobe_regs_equal();
+}
diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall.c b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
new file mode 100644
index 000000000000..8a4fa6c7ef59
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/uprobe_syscall.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_regs_trigger")
+int uretprobe(struct pt_regs *ctx)
+{
+ __builtin_memcpy(®s, ctx, sizeof(regs));
+ return 0;
+}
--
2.44.0
^ permalink raw reply related
* [PATCHv4 bpf-next 4/7] selftests/bpf: Add uretprobe syscall test for regs changes
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-1-jolsa@kernel.org>
Adding test that creates uprobe consumer on uretprobe which changes some
of the registers. Making sure the changed registers are propagated to the
user space when the ureptobe syscall trampoline is used on x86_64.
To be able to do this, adding support to bpf_testmod to create uprobe via
new attribute file:
/sys/kernel/bpf_testmod_uprobe
This file is expecting file offset and creates related uprobe on current
process exe file and removes existing uprobe if offset is 0. The can be
only single uprobe at any time.
The uprobe has specific consumer that changes registers used in ureprobe
syscall trampoline and which are later checked in the test.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 123 +++++++++++++++++-
.../selftests/bpf/prog_tests/uprobe_syscall.c | 67 ++++++++++
2 files changed, 189 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index eb2b78552ca2..27a12d125b9f 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -10,6 +10,7 @@
#include <linux/percpu-defs.h>
#include <linux/sysfs.h>
#include <linux/tracepoint.h>
+#include <linux/namei.h>
#include "bpf_testmod.h"
#include "bpf_testmod_kfunc.h"
@@ -343,6 +344,119 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
.write = bpf_testmod_test_write,
};
+/* bpf_testmod_uprobe sysfs attribute is so far enabled for x86_64 only,
+ * please see test_uretprobe_regs_change test
+ */
+#ifdef __x86_64__
+
+static int
+uprobe_ret_handler(struct uprobe_consumer *self, unsigned long func,
+ struct pt_regs *regs)
+
+{
+ regs->ax = 0x12345678deadbeef;
+ regs->cx = 0x87654321feebdaed;
+ regs->r11 = (u64) -1;
+ return true;
+}
+
+struct testmod_uprobe {
+ struct path path;
+ loff_t offset;
+ struct uprobe_consumer consumer;
+};
+
+static DEFINE_MUTEX(testmod_uprobe_mutex);
+
+static struct testmod_uprobe uprobe = {
+ .consumer.ret_handler = uprobe_ret_handler,
+};
+
+static int testmod_register_uprobe(loff_t offset)
+{
+ int err = -EBUSY;
+
+ if (uprobe.offset)
+ return -EBUSY;
+
+ mutex_lock(&testmod_uprobe_mutex);
+
+ if (uprobe.offset)
+ goto out;
+
+ err = kern_path("/proc/self/exe", LOOKUP_FOLLOW, &uprobe.path);
+ if (err)
+ goto out;
+
+ err = uprobe_register_refctr(d_real_inode(uprobe.path.dentry),
+ offset, 0, &uprobe.consumer);
+ if (err)
+ path_put(&uprobe.path);
+ else
+ uprobe.offset = offset;
+
+out:
+ mutex_unlock(&testmod_uprobe_mutex);
+ return err;
+}
+
+static void testmod_unregister_uprobe(void)
+{
+ mutex_lock(&testmod_uprobe_mutex);
+
+ if (uprobe.offset) {
+ uprobe_unregister(d_real_inode(uprobe.path.dentry),
+ uprobe.offset, &uprobe.consumer);
+ uprobe.offset = 0;
+ }
+
+ mutex_unlock(&testmod_uprobe_mutex);
+}
+
+static ssize_t
+bpf_testmod_uprobe_write(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t len)
+{
+ unsigned long offset;
+ int err;
+
+ if (kstrtoul(buf, 0, &offset))
+ return -EINVAL;
+
+ if (offset)
+ err = testmod_register_uprobe(offset);
+ else
+ testmod_unregister_uprobe();
+
+ return err ?: strlen(buf);
+}
+
+static struct bin_attribute bin_attr_bpf_testmod_uprobe_file __ro_after_init = {
+ .attr = { .name = "bpf_testmod_uprobe", .mode = 0666, },
+ .write = bpf_testmod_uprobe_write,
+};
+
+static int register_bpf_testmod_uprobe(void)
+{
+ return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_uprobe_file);
+}
+
+static void unregister_bpf_testmod_uprobe(void)
+{
+ testmod_unregister_uprobe();
+ sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_uprobe_file);
+}
+
+#else
+static int register_bpf_testmod_uprobe(void)
+{
+ return 0;
+}
+
+static void unregister_bpf_testmod_uprobe(void) { }
+#endif
+
BTF_KFUNCS_START(bpf_testmod_common_kfunc_ids)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
@@ -655,7 +769,13 @@ static int bpf_testmod_init(void)
return ret;
if (bpf_fentry_test1(0) < 0)
return -EINVAL;
- return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
+ ret = sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
+ if (ret < 0)
+ return ret;
+ ret = register_bpf_testmod_uprobe();
+ if (ret < 0)
+ return ret;
+ return 0;
}
static void bpf_testmod_exit(void)
@@ -669,6 +789,7 @@ static void bpf_testmod_exit(void)
msleep(20);
sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
+ unregister_bpf_testmod_uprobe();
}
module_init(bpf_testmod_init);
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 311ac19d8992..1a50cd35205d 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -149,15 +149,82 @@ static void test_uretprobe_regs_equal(void)
cleanup:
uprobe_syscall__destroy(skel);
}
+
+#define BPF_TESTMOD_UPROBE_TEST_FILE "/sys/kernel/bpf_testmod_uprobe"
+
+static int write_bpf_testmod_uprobe(unsigned long offset)
+{
+ size_t n, ret;
+ char buf[30];
+ int fd;
+
+ n = sprintf(buf, "%lu", offset);
+
+ fd = open(BPF_TESTMOD_UPROBE_TEST_FILE, O_WRONLY);
+ if (fd < 0)
+ return -errno;
+
+ ret = write(fd, buf, n);
+ close(fd);
+ return ret != n ? (int) ret : 0;
+}
+
+static void test_uretprobe_regs_change(void)
+{
+ struct pt_regs before = {}, after = {};
+ unsigned long *pb = (unsigned long *) &before;
+ unsigned long *pa = (unsigned long *) &after;
+ unsigned long cnt = sizeof(before)/sizeof(*pb);
+ unsigned int i, err, offset;
+
+ offset = get_uprobe_offset(uretprobe_regs_trigger);
+
+ err = write_bpf_testmod_uprobe(offset);
+ if (!ASSERT_OK(err, "register_uprobe"))
+ return;
+
+ uretprobe_regs(&before, &after);
+
+ err = write_bpf_testmod_uprobe(0);
+ if (!ASSERT_OK(err, "unregister_uprobe"))
+ return;
+
+ for (i = 0; i < cnt; i++) {
+ unsigned int offset = i * sizeof(unsigned long);
+
+ switch (offset) {
+ case offsetof(struct pt_regs, rax):
+ ASSERT_EQ(pa[i], 0x12345678deadbeef, "rax");
+ break;
+ case offsetof(struct pt_regs, rcx):
+ ASSERT_EQ(pa[i], 0x87654321feebdaed, "rcx");
+ break;
+ case offsetof(struct pt_regs, r11):
+ ASSERT_EQ(pa[i], (__u64) -1, "r11");
+ break;
+ default:
+ if (!ASSERT_EQ(pa[i], pb[i], "register before-after value check"))
+ fprintf(stdout, "failed register offset %u\n", offset);
+ }
+ }
+}
+
#else
static void test_uretprobe_regs_equal(void)
{
test__skip();
}
+
+static void test_uretprobe_regs_change(void)
+{
+ test__skip();
+}
#endif
void test_uprobe_syscall(void)
{
if (test__start_subtest("uretprobe_regs_equal"))
test_uretprobe_regs_equal();
+ if (test__start_subtest("uretprobe_regs_change"))
+ test_uretprobe_regs_change();
}
--
2.44.0
^ permalink raw reply related
* [PATCHv4 bpf-next 5/7] selftests/bpf: Add uretprobe syscall call from user space test
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-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 not executed.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/uprobe_syscall.c | 95 +++++++++++++++++++
.../bpf/progs/uprobe_syscall_executed.c | 17 ++++
2 files changed, 112 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_executed.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..c6fdb8c59ea3 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_executed.skel.h"
__naked unsigned long uretprobe_regs_trigger(void)
{
@@ -209,6 +212,91 @@ 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)
+{
+ LIBBPF_OPTS(bpf_uprobe_multi_opts, opts,
+ .retprobe = true,
+ );
+ struct uprobe_syscall_executed *skel;
+ int pid, status, err, go[2], c;
+
+ if (pipe(go))
+ return;
+
+ skel = uprobe_syscall_executed__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "uprobe_syscall_executed__open_and_load"))
+ goto cleanup;
+
+ pid = fork();
+ if (!ASSERT_GE(pid, 0, "fork"))
+ goto cleanup;
+
+ /* child */
+ if (pid == 0) {
+ close(go[1]);
+
+ /* wait for parent's kick */
+ err = read(go[0], &c, 1);
+ if (err != 1)
+ exit(-1);
+
+ uretprobe_syscall_call();
+ _exit(0);
+ }
+
+ skel->links.test = bpf_program__attach_uprobe_multi(skel->progs.test, pid,
+ "/proc/self/exe",
+ "uretprobe_syscall_call", &opts);
+ if (!ASSERT_OK_PTR(skel->links.test, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ /* kick the child */
+ write(go[1], &c, 1);
+ err = waitpid(pid, &status, 0);
+ ASSERT_EQ(err, pid, "waitpid");
+
+ /* 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_EQ(skel->bss->executed, 0, "executed");
+
+cleanup:
+ uprobe_syscall_executed__destroy(skel);
+ close(go[1]);
+ close(go[0]);
+}
#else
static void test_uretprobe_regs_equal(void)
{
@@ -219,6 +307,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)
@@ -227,4 +320,6 @@ void test_uprobe_syscall(void)
test_uretprobe_regs_equal();
if (test__start_subtest("uretprobe_regs_change"))
test_uretprobe_regs_change();
+ if (test__start_subtest("uretprobe_syscall_call"))
+ test_uretprobe_syscall_call();
}
diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c
new file mode 100644
index 000000000000..0d7f1a7db2e2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c
@@ -0,0 +1,17 @@
+// 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";
+
+int executed = 0;
+
+SEC("uretprobe.multi")
+int test(struct pt_regs *regs)
+{
+ executed = 1;
+ return 0;
+}
--
2.44.0
^ permalink raw reply related
* [PATCHv4 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-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.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 7 ++-
.../selftests/bpf/prog_tests/uprobe_syscall.c | 60 +++++++++++++++++++
3 files changed, 67 insertions(+), 1 deletion(-)
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 82247aeef857..a94352162290 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -133,7 +133,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
ifneq ($(V),1)
submake_extras := feature_display=0
@@ -631,6 +631,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/xdp_synproxy \
$(OUTPUT)/sign-file \
$(OUTPUT)/uprobe_multi \
+ $(OUTPUT)/uprobe_compat \
ima_setup.sh \
verify_sig_setup.sh \
$(wildcard progs/btf_dump_test_case_*.c) \
@@ -752,6 +753,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 c6fdb8c59ea3..bfea9a0368a4 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -5,6 +5,7 @@
#ifdef __x86_64__
#include <unistd.h>
+#include <stdlib.h>
#include <asm/ptrace.h>
#include <linux/compiler.h>
#include <linux/stringify.h>
@@ -297,6 +298,58 @@ static void test_uretprobe_syscall_call(void)
close(go[1]);
close(go[0]);
}
+
+static void test_uretprobe_compat(void)
+{
+ LIBBPF_OPTS(bpf_uprobe_multi_opts, opts,
+ .retprobe = true,
+ );
+ struct uprobe_syscall_executed *skel;
+ int err, go[2], pid, c, status;
+
+ if (pipe(go))
+ return;
+
+ skel = uprobe_syscall_executed__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "uprobe_syscall_executed__open_and_load"))
+ goto cleanup;
+
+ pid = fork();
+ if (pid < 0)
+ goto cleanup;
+
+ /* child */
+ if (pid == 0) {
+ close(go[1]);
+
+ /* wait for parent's kick */
+ err = read(go[0], &c, 1);
+ if (err != 1)
+ exit(-1);
+ execl("./uprobe_compat", "./uprobe_compat", NULL);
+ exit(-1);
+ }
+
+ skel->links.test = bpf_program__attach_uprobe_multi(skel->progs.test, pid,
+ "./uprobe_compat", "main", &opts);
+ if (!ASSERT_OK_PTR(skel->links.test, "bpf_program__attach_uprobe_multi"))
+ goto cleanup;
+
+ /* kick the child */
+ write(go[1], &c, 1);
+ err = waitpid(pid, &status, 0);
+ ASSERT_EQ(err, pid, "waitpid");
+
+ /* verify the child exited normally and the bpf program got executed */
+ ASSERT_EQ(WIFEXITED(status), 1, "WIFEXITED");
+ ASSERT_EQ(WEXITSTATUS(status), 0, "WEXITSTATUS");
+ ASSERT_EQ(skel->bss->executed, 1, "executed");
+
+cleanup:
+ uprobe_syscall_executed__destroy(skel);
+ close(go[0]);
+ close(go[1]);
+}
#else
static void test_uretprobe_regs_equal(void)
{
@@ -312,6 +365,11 @@ static void test_uretprobe_syscall_call(void)
{
test__skip();
}
+
+static void test_uretprobe_compat(void)
+{
+ test__skip();
+}
#endif
void test_uprobe_syscall(void)
@@ -322,4 +380,6 @@ void test_uprobe_syscall(void)
test_uretprobe_regs_change();
if (test__start_subtest("uretprobe_syscall_call"))
test_uretprobe_syscall_call();
+ if (test__start_subtest("uretprobe_compat"))
+ test_uretprobe_compat();
}
--
2.44.0
^ permalink raw reply related
* [PATCHv4 7/7] man2: Add uretprobe syscall page
From: Jiri Olsa @ 2024-05-02 12:23 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-api, linux-man, x86, bpf,
Song Liu, Yonghong Song, John Fastabend, Peter Zijlstra,
Thomas Gleixner, Borislav Petkov (AMD), Ingo Molnar,
Andy Lutomirski
In-Reply-To: <20240502122313.1579719-1-jolsa@kernel.org>
Adding man page for new uretprobe syscall.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
man2/uretprobe.2 | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 man2/uretprobe.2
diff --git a/man2/uretprobe.2 b/man2/uretprobe.2
new file mode 100644
index 000000000000..08fe6a670430
--- /dev/null
+++ b/man2/uretprobe.2
@@ -0,0 +1,45 @@
+.\" 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
+Kernel is using
+.BR uretprobe()
+syscall to trigger uprobe return probe consumers instead of using
+standard breakpoint instruction.
+
+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()
+syscall is initially introduced on x86-64 architecture, because doing syscall
+is faster than doing breakpoint trap on it. It might be extended to other
+architectures.
+
+.BR uretprobe()
+syscall 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
* Re: [PATCH] fs/xattr: unify *at syscalls
From: Christian Brauner @ 2024-05-02 13:02 UTC (permalink / raw)
To: Christian Goettsche
Cc: Christian Brauner, Christian Goettsche, Jan Kara, Alexander Viro,
Jan Kara, Arnd Bergmann, Thomas Gleixner, Kees Cook,
Geert Uytterhoeven, Casey Schaufler, peterz, Sohil Mehta,
Miklos Szeredi, Mark Rutland, linux-fsdevel, linux-kernel,
linux-api
In-Reply-To: <20240430151917.30036-1-cgoettsche@seltendoof.de>
On Tue, 30 Apr 2024 17:19:14 +0200, Christian Göttsche wrote:
> Use the same parameter ordering for all four newly added *xattrat
> syscalls:
>
> dirfd, pathname, at_flags, ...
>
> Also consistently use unsigned int as the type for at_flags.
>
> [...]
Applied to the vfs.xattr branch of the vfs/vfs.git tree.
Patches in the vfs.xattr branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.xattr
[1/1] fs/xattr: unify *at syscalls
https://git.kernel.org/vfs/vfs/c/1d5e73c8c531
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox