| Message ID | 4e3e8b9b525c8bed39c0ee2aa68f2dff701f56a4.1604542786.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Fix bpf_probe_read_user_str() overcopying | expand |
> On Nov 4, 2020, at 6:25 PM, Daniel Xu <dxu@dxuuu.xyz> wrote: > > Previously, bpf_probe_read_user_str() could potentially overcopy the > trailing bytes after the NUL due to how do_strncpy_from_user() does the > copy in long-sized strides. The issue has been fixed in the previous > commit. > > This commit adds a selftest that ensures we don't regress > bpf_probe_read_user_str() again. > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- > .../bpf/prog_tests/probe_read_user_str.c | 60 +++++++++++++++++++ > .../bpf/progs/test_probe_read_user_str.c | 34 +++++++++++ > 2 files changed, 94 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > create mode 100644 tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > new file mode 100644 > index 000000000000..597a166e6c8d > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <test_progs.h> > +#include "test_probe_read_user_str.skel.h" > + > +static const char str[] = "mestring"; > + > +void test_probe_read_user_str(void) > +{ > + struct test_probe_read_user_str *skel; > + int fd, err, duration = 0; > + char buf[256]; > + ssize_t n; > + > + skel = test_probe_read_user_str__open_and_load(); > + if (CHECK(!skel, "test_probe_read_user_str__open_and_load", > + "skeleton open and load failed\n")) > + goto out; nit: we can just return here. > + > + err = test_probe_read_user_str__attach(skel); > + if (CHECK(err, "test_probe_read_user_str__attach", > + "skeleton attach failed: %d\n", err)) > + goto out; > + > + fd = open("/dev/null", O_WRONLY); > + if (CHECK(fd < 0, "open", "open /dev/null failed: %d\n", fd)) > + goto out; > + > + /* Give pid to bpf prog so it doesn't read from anyone else */ > + skel->bss->pid = getpid(); It is better to set pid before attaching skel. > + > + /* Ensure bytes after string are ones */ > + memset(buf, 1, sizeof(buf)); > + memcpy(buf, str, sizeof(str)); > + > + /* Trigger tracepoint */ > + n = write(fd, buf, sizeof(buf)); > + if (CHECK(n != sizeof(buf), "write", "write failed: %ld\n", n)) > + goto fd_out; > + > + /* Did helper fail? */ > + if (CHECK(skel->bss->ret < 0, "prog ret", "prog returned: %d\n", In most cases, we use underscore instead of spaces in the second argument of CHECK(). IOW, please use "prog_ret" instead of "prog ret". > + skel->bss->ret)) > + goto fd_out; > + > + /* Check that string was copied correctly */ > + err = memcmp(skel->bss->buf, str, sizeof(str)); > + if (CHECK(err, "memcmp", "prog copied wrong string")) > + goto fd_out; > + > + /* Now check that no extra trailing bytes were copied */ > + memset(buf, 0, sizeof(buf)); > + err = memcmp(skel->bss->buf + sizeof(str), buf, sizeof(buf) - sizeof(str)); > + if (CHECK(err, "memcmp", "trailing bytes were not stripped")) > + goto fd_out; > + > +fd_out: > + close(fd); > +out: > + test_probe_read_user_str__destroy(skel); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > new file mode 100644 > index 000000000000..41c3e296566e > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > @@ -0,0 +1,34 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > + > +#include <sys/types.h> > + > +struct sys_enter_write_args { > + unsigned long long pad; > + int syscall_nr; > + int pad1; /* 4 byte hole */ > + unsigned int fd; > + int pad2; /* 4 byte hole */ > + const char *buf; > + size_t count; > +}; > + > +pid_t pid = 0; > +int ret = 0; > +char buf[256] = {}; > + > +SEC("tracepoint/syscalls/sys_enter_write") > +int on_write(struct sys_enter_write_args *ctx) > +{ > + if (pid != (bpf_get_current_pid_tgid() >> 32)) > + return 0; > + > + ret = bpf_probe_read_user_str(buf, sizeof(buf), ctx->buf); bpf_probe_read_user_str() returns "long". Let's use "long ret;" > + > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.28.0 >
On Thu Nov 5, 2020 at 10:30 AM PST, Song Liu wrote: > > > > On Nov 4, 2020, at 6:25 PM, Daniel Xu <dxu@dxuuu.xyz> wrote: > > > > Previously, bpf_probe_read_user_str() could potentially overcopy the > > trailing bytes after the NUL due to how do_strncpy_from_user() does the > > copy in long-sized strides. The issue has been fixed in the previous > > commit. > > > > This commit adds a selftest that ensures we don't regress > > bpf_probe_read_user_str() again. > > > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > > --- > > .../bpf/prog_tests/probe_read_user_str.c | 60 +++++++++++++++++++ > > .../bpf/progs/test_probe_read_user_str.c | 34 +++++++++++ > > 2 files changed, 94 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > > create mode 100644 tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > > new file mode 100644 > > index 000000000000..597a166e6c8d > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > > @@ -0,0 +1,60 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include <test_progs.h> > > +#include "test_probe_read_user_str.skel.h" > > + > > +static const char str[] = "mestring"; > > + > > +void test_probe_read_user_str(void) > > +{ > > + struct test_probe_read_user_str *skel; > > + int fd, err, duration = 0; > > + char buf[256]; > > + ssize_t n; > > + > > + skel = test_probe_read_user_str__open_and_load(); > > + if (CHECK(!skel, "test_probe_read_user_str__open_and_load", > > + "skeleton open and load failed\n")) > > + goto out; > > nit: we can just return here. > > > + > > + err = test_probe_read_user_str__attach(skel); > > + if (CHECK(err, "test_probe_read_user_str__attach", > > + "skeleton attach failed: %d\n", err)) > > + goto out; > > + > > + fd = open("/dev/null", O_WRONLY); > > + if (CHECK(fd < 0, "open", "open /dev/null failed: %d\n", fd)) > > + goto out; > > + > > + /* Give pid to bpf prog so it doesn't read from anyone else */ > > + skel->bss->pid = getpid(); > > It is better to set pid before attaching skel. > > > + > > + /* Ensure bytes after string are ones */ > > + memset(buf, 1, sizeof(buf)); > > + memcpy(buf, str, sizeof(str)); > > + > > + /* Trigger tracepoint */ > > + n = write(fd, buf, sizeof(buf)); > > + if (CHECK(n != sizeof(buf), "write", "write failed: %ld\n", n)) > > + goto fd_out; > > + > > + /* Did helper fail? */ > > + if (CHECK(skel->bss->ret < 0, "prog ret", "prog returned: %d\n", > > In most cases, we use underscore instead of spaces in the second > argument > of CHECK(). IOW, please use "prog_ret" instead of "prog ret". > > > + skel->bss->ret)) > > + goto fd_out; > > + > > + /* Check that string was copied correctly */ > > + err = memcmp(skel->bss->buf, str, sizeof(str)); > > + if (CHECK(err, "memcmp", "prog copied wrong string")) > > + goto fd_out; > > + > > + /* Now check that no extra trailing bytes were copied */ > > + memset(buf, 0, sizeof(buf)); > > + err = memcmp(skel->bss->buf + sizeof(str), buf, sizeof(buf) - sizeof(str)); > > + if (CHECK(err, "memcmp", "trailing bytes were not stripped")) > > + goto fd_out; > > + > > +fd_out: > > + close(fd); > > +out: > > + test_probe_read_user_str__destroy(skel); > > +} > > diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > new file mode 100644 > > index 000000000000..41c3e296566e > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > @@ -0,0 +1,34 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/bpf.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > + > > +#include <sys/types.h> > > + > > +struct sys_enter_write_args { > > + unsigned long long pad; > > + int syscall_nr; > > + int pad1; /* 4 byte hole */ > > + unsigned int fd; > > + int pad2; /* 4 byte hole */ > > + const char *buf; > > + size_t count; > > +}; > > + > > +pid_t pid = 0; > > +int ret = 0; > > +char buf[256] = {}; > > + > > +SEC("tracepoint/syscalls/sys_enter_write") > > +int on_write(struct sys_enter_write_args *ctx) > > +{ > > + if (pid != (bpf_get_current_pid_tgid() >> 32)) > > + return 0; > > + > > + ret = bpf_probe_read_user_str(buf, sizeof(buf), ctx->buf); > > bpf_probe_read_user_str() returns "long". Let's use "long ret;" Thanks for review, will send v3 with these changes. [...]
On Wed, Nov 4, 2020 at 8:51 PM Daniel Xu <dxu@dxuuu.xyz> wrote: > > Previously, bpf_probe_read_user_str() could potentially overcopy the > trailing bytes after the NUL due to how do_strncpy_from_user() does the > copy in long-sized strides. The issue has been fixed in the previous > commit. > > This commit adds a selftest that ensures we don't regress > bpf_probe_read_user_str() again. > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- > .../bpf/prog_tests/probe_read_user_str.c | 60 +++++++++++++++++++ > .../bpf/progs/test_probe_read_user_str.c | 34 +++++++++++ > 2 files changed, 94 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > create mode 100644 tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > new file mode 100644 > index 000000000000..597a166e6c8d > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <test_progs.h> > +#include "test_probe_read_user_str.skel.h" > + > +static const char str[] = "mestring"; > + > +void test_probe_read_user_str(void) > +{ > + struct test_probe_read_user_str *skel; > + int fd, err, duration = 0; > + char buf[256]; > + ssize_t n; > + > + skel = test_probe_read_user_str__open_and_load(); > + if (CHECK(!skel, "test_probe_read_user_str__open_and_load", > + "skeleton open and load failed\n")) > + goto out; > + > + err = test_probe_read_user_str__attach(skel); > + if (CHECK(err, "test_probe_read_user_str__attach", > + "skeleton attach failed: %d\n", err)) > + goto out; > + > + fd = open("/dev/null", O_WRONLY); > + if (CHECK(fd < 0, "open", "open /dev/null failed: %d\n", fd)) > + goto out; > + > + /* Give pid to bpf prog so it doesn't read from anyone else */ > + skel->bss->pid = getpid(); > + > + /* Ensure bytes after string are ones */ > + memset(buf, 1, sizeof(buf)); > + memcpy(buf, str, sizeof(str)); > + > + /* Trigger tracepoint */ > + n = write(fd, buf, sizeof(buf)); > + if (CHECK(n != sizeof(buf), "write", "write failed: %ld\n", n)) > + goto fd_out; > + > + /* Did helper fail? */ > + if (CHECK(skel->bss->ret < 0, "prog ret", "prog returned: %d\n", > + skel->bss->ret)) > + goto fd_out; > + > + /* Check that string was copied correctly */ > + err = memcmp(skel->bss->buf, str, sizeof(str)); > + if (CHECK(err, "memcmp", "prog copied wrong string")) > + goto fd_out; > + > + /* Now check that no extra trailing bytes were copied */ > + memset(buf, 0, sizeof(buf)); > + err = memcmp(skel->bss->buf + sizeof(str), buf, sizeof(buf) - sizeof(str)); > + if (CHECK(err, "memcmp", "trailing bytes were not stripped")) > + goto fd_out; > + > +fd_out: > + close(fd); > +out: > + test_probe_read_user_str__destroy(skel); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > new file mode 100644 > index 000000000000..41c3e296566e > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > @@ -0,0 +1,34 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > + > +#include <sys/types.h> > + > +struct sys_enter_write_args { > + unsigned long long pad; > + int syscall_nr; > + int pad1; /* 4 byte hole */ I have a hunch that this explicit padding might break on big-endian architectures?.. Can you instead include "vmlinux.h" in this file and use struct trace_event_raw_sys_enter? you'll just need ctx->args[2] to get that buffer pointer. Alternatively, and it's probably simpler overall would be to just provide user-space pointer through global variable: void *user_ptr; bpf_probe_read_user_str(buf, ..., user_ptr); From user-space: skel->bss->user_ptr = &my_userspace_buf; Full control. You can trigger tracepoint with just an usleep(1), for instance. > + unsigned int fd; > + int pad2; /* 4 byte hole */ > + const char *buf; > + size_t count; > +}; > + > +pid_t pid = 0; > +int ret = 0; > +char buf[256] = {}; > + > +SEC("tracepoint/syscalls/sys_enter_write") > +int on_write(struct sys_enter_write_args *ctx) > +{ > + if (pid != (bpf_get_current_pid_tgid() >> 32)) > + return 0; > + > + ret = bpf_probe_read_user_str(buf, sizeof(buf), ctx->buf); > + > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.28.0 >
On Thu Nov 5, 2020 at 1:32 PM PST, Andrii Nakryiko wrote: > On Wed, Nov 4, 2020 at 8:51 PM Daniel Xu <dxu@dxuuu.xyz> wrote: [...] > > diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > new file mode 100644 > > index 000000000000..41c3e296566e > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > > @@ -0,0 +1,34 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/bpf.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > + > > +#include <sys/types.h> > > + > > +struct sys_enter_write_args { > > + unsigned long long pad; > > + int syscall_nr; > > + int pad1; /* 4 byte hole */ > > I have a hunch that this explicit padding might break on big-endian > architectures?.. > > Can you instead include "vmlinux.h" in this file and use struct > trace_event_raw_sys_enter? you'll just need ctx->args[2] to get that > buffer pointer. > > Alternatively, and it's probably simpler overall would be to just > provide user-space pointer through global variable: > > void *user_ptr; > > > bpf_probe_read_user_str(buf, ..., user_ptr); > > From user-space: > > skel->bss->user_ptr = &my_userspace_buf; > > Full control. You can trigger tracepoint with just an usleep(1), for > instance. Yeah, that sounds better. I'll send a v4 with passing a ptr. Thanks, Daniel [...]
> On Nov 5, 2020, at 3:22 PM, Daniel Xu <dxu@dxuuu.xyz> wrote: > > On Thu Nov 5, 2020 at 1:32 PM PST, Andrii Nakryiko wrote: >> On Wed, Nov 4, 2020 at 8:51 PM Daniel Xu <dxu@dxuuu.xyz> wrote: > [...] >>> diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c >>> new file mode 100644 >>> index 000000000000..41c3e296566e >>> --- /dev/null >>> +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c >>> @@ -0,0 +1,34 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> + >>> +#include <linux/bpf.h> >>> +#include <bpf/bpf_helpers.h> >>> +#include <bpf/bpf_tracing.h> >>> + >>> +#include <sys/types.h> >>> + >>> +struct sys_enter_write_args { >>> + unsigned long long pad; >>> + int syscall_nr; >>> + int pad1; /* 4 byte hole */ >> >> I have a hunch that this explicit padding might break on big-endian >> architectures?.. >> >> Can you instead include "vmlinux.h" in this file and use struct >> trace_event_raw_sys_enter? you'll just need ctx->args[2] to get that >> buffer pointer. >> >> Alternatively, and it's probably simpler overall would be to just >> provide user-space pointer through global variable: >> >> void *user_ptr; >> >> >> bpf_probe_read_user_str(buf, ..., user_ptr); >> >> From user-space: >> >> skel->bss->user_ptr = &my_userspace_buf; >> >> Full control. You can trigger tracepoint with just an usleep(1), for >> instance. > > Yeah, that sounds better. I'll send a v4 with passing a ptr. > > Thanks, > Daniel One more comment, how about we test multiple strings with different lengths? In this way, we can catch other alignment issues. Thanks, Song
On Thu Nov 5, 2020 at 3:31 PM PST, Song Liu wrote: > > > > On Nov 5, 2020, at 3:22 PM, Daniel Xu <dxu@dxuuu.xyz> wrote: > > > > On Thu Nov 5, 2020 at 1:32 PM PST, Andrii Nakryiko wrote: > >> On Wed, Nov 4, 2020 at 8:51 PM Daniel Xu <dxu@dxuuu.xyz> wrote: > > [...] > >>> diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > >>> new file mode 100644 > >>> index 000000000000..41c3e296566e > >>> --- /dev/null > >>> +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c > >>> @@ -0,0 +1,34 @@ > >>> +// SPDX-License-Identifier: GPL-2.0 > >>> + > >>> +#include <linux/bpf.h> > >>> +#include <bpf/bpf_helpers.h> > >>> +#include <bpf/bpf_tracing.h> > >>> + > >>> +#include <sys/types.h> > >>> + > >>> +struct sys_enter_write_args { > >>> + unsigned long long pad; > >>> + int syscall_nr; > >>> + int pad1; /* 4 byte hole */ > >> > >> I have a hunch that this explicit padding might break on big-endian > >> architectures?.. > >> > >> Can you instead include "vmlinux.h" in this file and use struct > >> trace_event_raw_sys_enter? you'll just need ctx->args[2] to get that > >> buffer pointer. > >> > >> Alternatively, and it's probably simpler overall would be to just > >> provide user-space pointer through global variable: > >> > >> void *user_ptr; > >> > >> > >> bpf_probe_read_user_str(buf, ..., user_ptr); > >> > >> From user-space: > >> > >> skel->bss->user_ptr = &my_userspace_buf; > >> > >> Full control. You can trigger tracepoint with just an usleep(1), for > >> instance. > > > > Yeah, that sounds better. I'll send a v4 with passing a ptr. > > > > Thanks, > > Daniel > > One more comment, how about we test multiple strings with different > lengths? In this way, we can catch other alignment issues. Sure, will do that in v4 also.
diff --git a/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c new file mode 100644 index 000000000000..597a166e6c8d --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include "test_probe_read_user_str.skel.h" + +static const char str[] = "mestring"; + +void test_probe_read_user_str(void) +{ + struct test_probe_read_user_str *skel; + int fd, err, duration = 0; + char buf[256]; + ssize_t n; + + skel = test_probe_read_user_str__open_and_load(); + if (CHECK(!skel, "test_probe_read_user_str__open_and_load", + "skeleton open and load failed\n")) + goto out; + + err = test_probe_read_user_str__attach(skel); + if (CHECK(err, "test_probe_read_user_str__attach", + "skeleton attach failed: %d\n", err)) + goto out; + + fd = open("/dev/null", O_WRONLY); + if (CHECK(fd < 0, "open", "open /dev/null failed: %d\n", fd)) + goto out; + + /* Give pid to bpf prog so it doesn't read from anyone else */ + skel->bss->pid = getpid(); + + /* Ensure bytes after string are ones */ + memset(buf, 1, sizeof(buf)); + memcpy(buf, str, sizeof(str)); + + /* Trigger tracepoint */ + n = write(fd, buf, sizeof(buf)); + if (CHECK(n != sizeof(buf), "write", "write failed: %ld\n", n)) + goto fd_out; + + /* Did helper fail? */ + if (CHECK(skel->bss->ret < 0, "prog ret", "prog returned: %d\n", + skel->bss->ret)) + goto fd_out; + + /* Check that string was copied correctly */ + err = memcmp(skel->bss->buf, str, sizeof(str)); + if (CHECK(err, "memcmp", "prog copied wrong string")) + goto fd_out; + + /* Now check that no extra trailing bytes were copied */ + memset(buf, 0, sizeof(buf)); + err = memcmp(skel->bss->buf + sizeof(str), buf, sizeof(buf) - sizeof(str)); + if (CHECK(err, "memcmp", "trailing bytes were not stripped")) + goto fd_out; + +fd_out: + close(fd); +out: + test_probe_read_user_str__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c new file mode 100644 index 000000000000..41c3e296566e --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +#include <sys/types.h> + +struct sys_enter_write_args { + unsigned long long pad; + int syscall_nr; + int pad1; /* 4 byte hole */ + unsigned int fd; + int pad2; /* 4 byte hole */ + const char *buf; + size_t count; +}; + +pid_t pid = 0; +int ret = 0; +char buf[256] = {}; + +SEC("tracepoint/syscalls/sys_enter_write") +int on_write(struct sys_enter_write_args *ctx) +{ + if (pid != (bpf_get_current_pid_tgid() >> 32)) + return 0; + + ret = bpf_probe_read_user_str(buf, sizeof(buf), ctx->buf); + + return 0; +} + +char _license[] SEC("license") = "GPL";
Previously, bpf_probe_read_user_str() could potentially overcopy the trailing bytes after the NUL due to how do_strncpy_from_user() does the copy in long-sized strides. The issue has been fixed in the previous commit. This commit adds a selftest that ensures we don't regress bpf_probe_read_user_str() again. Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- .../bpf/prog_tests/probe_read_user_str.c | 60 +++++++++++++++++++ .../bpf/progs/test_probe_read_user_str.c | 34 +++++++++++ 2 files changed, 94 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c create mode 100644 tools/testing/selftests/bpf/progs/test_probe_read_user_str.c