| Message ID | 20211007141331.723149-3-hengqi.chen@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Add bpf_skc_to_unix_sock() helper | expand |
On Thu, Oct 7, 2021 at 7:14 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > + > + sockaddr.sun_family = AF_UNIX; > + strcpy(sockaddr.sun_path, sock_path); please use strncpy. > + len = sizeof(sockaddr); > + unlink(sock_path); please use abstract socket to avoid unlink and potential race.
Thanks for the review. On 10/19/21 9:46 AM, Alexei Starovoitov wrote: > On Thu, Oct 7, 2021 at 7:14 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >> + >> + sockaddr.sun_family = AF_UNIX; >> + strcpy(sockaddr.sun_path, sock_path); > > please use strncpy. Will do. > >> + len = sizeof(sockaddr); >> + unlink(sock_path); > > please use abstract socket to avoid unlink and potential race. > Will switch to abstract socket and update the BPF program.
On Tue, Oct 19, 2021 at 8:23 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > > Thanks for the review. > > On 10/19/21 9:46 AM, Alexei Starovoitov wrote: > > On Thu, Oct 7, 2021 at 7:14 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > >> + > >> + sockaddr.sun_family = AF_UNIX; > >> + strcpy(sockaddr.sun_path, sock_path); > > > > please use strncpy. > > Will do. please also run checkpatch.pl and confirm you haven't introduced new styling issues. As one example (and please fix this up in the next revision), you are using C++-style comments. > > > > >> + len = sizeof(sockaddr); > >> + unlink(sock_path); > > > > please use abstract socket to avoid unlink and potential race. > > > > Will switch to abstract socket and update the BPF program.
On 2021/10/20 1:02 AM, Andrii Nakryiko wrote: > On Tue, Oct 19, 2021 at 8:23 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >> >> Thanks for the review. >> >> On 10/19/21 9:46 AM, Alexei Starovoitov wrote: >>> On Thu, Oct 7, 2021 at 7:14 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >>>> + >>>> + sockaddr.sun_family = AF_UNIX; >>>> + strcpy(sockaddr.sun_path, sock_path); >>> >>> please use strncpy. >> >> Will do. > > please also run checkpatch.pl and confirm you haven't introduced new > styling issues. As one example (and please fix this up in the next > revision), you are using C++-style comments. > Thanks, Andrii. Forgot to run checkpatch scripts before sending the email. Will do that in future patches. >> >>> >>>> + len = sizeof(sockaddr); >>>> + unlink(sock_path); >>> >>> please use abstract socket to avoid unlink and potential race. >>> >> >> Will switch to abstract socket and update the BPF program.
diff --git a/tools/testing/selftests/bpf/prog_tests/skc_to_unix_sock.c b/tools/testing/selftests/bpf/prog_tests/skc_to_unix_sock.c new file mode 100644 index 000000000000..971ef5b948bd --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/skc_to_unix_sock.c @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2021 Hengqi Chen */ + +#include <test_progs.h> +#include <sys/un.h> +#include "test_skc_to_unix_sock.skel.h" + +static const char *sock_path = "/tmp/test.sock"; + +void test_skc_to_unix_sock(void) +{ + struct test_skc_to_unix_sock *skel; + struct sockaddr_un sockaddr; + int err, len, sockfd = 0; + + skel = test_skc_to_unix_sock__open(); + if (!ASSERT_OK_PTR(skel, "could not open BPF object")) + return; + + skel->rodata->my_pid = getpid(); + + err = test_skc_to_unix_sock__load(skel); + if (!ASSERT_OK(err, "could not load BPF object")) + goto cleanup; + + err = test_skc_to_unix_sock__attach(skel); + if (!ASSERT_OK(err, "could not attach BPF object")) + goto cleanup; + + // trigger unix_listen + sockfd = socket(AF_UNIX, SOCK_STREAM, 0); + if (!ASSERT_GT(sockfd, 0, "socket failed")) + goto cleanup; + + sockaddr.sun_family = AF_UNIX; + strcpy(sockaddr.sun_path, sock_path); + len = sizeof(sockaddr); + unlink(sock_path); + + err = bind(sockfd, (struct sockaddr *)&sockaddr, len); + if (!ASSERT_OK(err, "bind failed")) + goto cleanup; + + err = listen(sockfd, 1); + if (!ASSERT_OK(err, "listen failed")) + goto cleanup; + + ASSERT_EQ(strcmp(skel->bss->path, sock_path), 0, "bpf_skc_to_unix_sock failed"); + +cleanup: + if (sockfd) + close(sockfd); + test_skc_to_unix_sock__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c b/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c new file mode 100644 index 000000000000..098c49c2edce --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2021 Hengqi Chen */ + +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <bpf/bpf_core_read.h> + +const volatile pid_t my_pid = 0; +char path[256] = {}; + +SEC("fentry/unix_listen") +int BPF_PROG(unix_listen, struct socket *sock, int backlog) +{ + pid_t pid = bpf_get_current_pid_tgid() >> 32; + struct unix_sock *unix_sk; + + if (pid != my_pid) + return 0; + + unix_sk = (struct unix_sock *)bpf_skc_to_unix_sock(sock->sk); + if (!unix_sk) + return 0; + + bpf_core_read_str(path, sizeof(path), &unix_sk->addr->name->sun_path); + return 0; +} + +char _license[] SEC("license") = "GPL";
Add a new test which triggers unix_listen kernel function to test bpf_skc_to_unix_sock helper. Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> --- .../bpf/prog_tests/skc_to_unix_sock.c | 54 +++++++++++++++++++ .../bpf/progs/test_skc_to_unix_sock.c | 29 ++++++++++ 2 files changed, 83 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/skc_to_unix_sock.c create mode 100644 tools/testing/selftests/bpf/progs/test_skc_to_unix_sock.c -- 2.25.1