From: Matt Bobrowski <mattbobrowski@google.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, kpsingh@google.com,
jannh@google.com, jolsa@kernel.org, daniel@iogearbox.net,
brauner@kernel.org, linux-fsdevel@vger.kernel.org
Subject: [PATCH bpf-next 02/11] bpf/selftests: adjust selftests for BPF helper bpf_d_path()
Date: Tue, 20 Feb 2024 09:27:30 +0000 [thread overview]
Message-ID: <18c7b587d43bbc7e80593bf51ea9d3eb99e47bc1.1708377880.git.mattbobrowski@google.com> (raw)
In-Reply-To: <cover.1708377880.git.mattbobrowski@google.com>
The BPF helper bpf_d_path() has been modified such that it makes use
of probe-read semantics. In turn, the behaviour of the BPF helper
bpf_d_path() has slightly changed under certain circumstances,
therefore needing to also adjust the backing test suite to account for
this.
The probe-read based d_path() implementation cannot handle dentries
that have their name backed by d_op->d_dname(). For paths containing
such dentries, the probe-read based implementation returns
-EOPNOTSUPP, so the test suite has been updated to assert this
behaviour.
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
.../testing/selftests/bpf/prog_tests/d_path.c | 84 +++++++++++++++----
.../testing/selftests/bpf/progs/test_d_path.c | 2 +-
2 files changed, 68 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index ccc768592e66..d77ae1b1e6ba 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -6,7 +6,7 @@
#include <sys/syscall.h>
#define MAX_PATH_LEN 128
-#define MAX_FILES 7
+#define MAX_FILES 8
#include "test_d_path.skel.h"
#include "test_d_path_check_rdonly_mem.skel.h"
@@ -25,9 +25,15 @@
static int duration;
+struct want {
+ bool err;
+ long err_code;
+ char path[MAX_PATH_LEN];
+};
+
static struct {
__u32 cnt;
- char paths[MAX_FILES][MAX_PATH_LEN];
+ struct want want[MAX_FILES];
} src;
static int set_pathname(int fd, pid_t pid)
@@ -35,12 +41,12 @@ static int set_pathname(int fd, pid_t pid)
char buf[MAX_PATH_LEN];
snprintf(buf, MAX_PATH_LEN, "/proc/%d/fd/%d", pid, fd);
- return readlink(buf, src.paths[src.cnt++], MAX_PATH_LEN);
+ return readlink(buf, src.want[src.cnt++].path, MAX_PATH_LEN);
}
static int trigger_fstat_events(pid_t pid)
{
- int sockfd = -1, procfd = -1, devfd = -1;
+ int sockfd = -1, procfd = -1, devfd = -1, mntnsfd = -1;
int localfd = -1, indicatorfd = -1;
int pipefd[2] = { -1, -1 };
struct stat fileStat;
@@ -49,10 +55,15 @@ static int trigger_fstat_events(pid_t pid)
/* unmountable pseudo-filesystems */
if (CHECK(pipe(pipefd) < 0, "trigger", "pipe failed\n"))
return ret;
- /* unmountable pseudo-filesystems */
+
sockfd = socket(AF_INET, SOCK_STREAM, 0);
if (CHECK(sockfd < 0, "trigger", "socket failed\n"))
goto out_close;
+
+ mntnsfd = open("/proc/self/ns/mnt", O_RDONLY);
+ if (CHECK(mntnsfd < 0, "trigger", "mntnsfd failed"))
+ goto out_close;
+
/* mountable pseudo-filesystems */
procfd = open("/proc/self/comm", O_RDONLY);
if (CHECK(procfd < 0, "trigger", "open /proc/self/comm failed\n"))
@@ -69,15 +80,35 @@ static int trigger_fstat_events(pid_t pid)
if (CHECK(indicatorfd < 0, "trigger", "open /tmp/ failed\n"))
goto out_close;
+ /*
+ * With bpf_d_path() being backed by probe-read semantics, we cannot
+ * safely resolve paths that are comprised of dentries that make use of
+ * dynamic names. We expect to return -EOPNOTSUPP for such paths.
+ */
+ src.want[src.cnt].err = true;
+ src.want[src.cnt].err_code = -EOPNOTSUPP;
ret = set_pathname(pipefd[0], pid);
if (CHECK(ret < 0, "trigger", "set_pathname failed for pipe[0]\n"))
goto out_close;
+
+ src.want[src.cnt].err = true;
+ src.want[src.cnt].err_code = -EOPNOTSUPP;
ret = set_pathname(pipefd[1], pid);
if (CHECK(ret < 0, "trigger", "set_pathname failed for pipe[1]\n"))
goto out_close;
+
+ src.want[src.cnt].err = true;
+ src.want[src.cnt].err_code = -EOPNOTSUPP;
ret = set_pathname(sockfd, pid);
if (CHECK(ret < 0, "trigger", "set_pathname failed for socket\n"))
goto out_close;
+
+ src.want[src.cnt].err = true;
+ src.want[src.cnt].err_code = -EOPNOTSUPP;
+ ret = set_pathname(mntnsfd, pid);
+ if (CHECK(ret < 0, "trigger", "set_pathname failed for mntnsfd\n"))
+ goto out_close;
+
ret = set_pathname(procfd, pid);
if (CHECK(ret < 0, "trigger", "set_pathname failed for proc\n"))
goto out_close;
@@ -95,6 +126,7 @@ static int trigger_fstat_events(pid_t pid)
fstat(pipefd[0], &fileStat);
fstat(pipefd[1], &fileStat);
fstat(sockfd, &fileStat);
+ fstat(mntnsfd, &fileStat);
fstat(procfd, &fileStat);
fstat(devfd, &fileStat);
fstat(localfd, &fileStat);
@@ -109,6 +141,7 @@ static int trigger_fstat_events(pid_t pid)
close(pipefd[0]);
close(pipefd[1]);
close(sockfd);
+ close(mntnsfd);
close(procfd);
close(devfd);
close(localfd);
@@ -150,24 +183,41 @@ static void test_d_path_basic(void)
goto cleanup;
for (int i = 0; i < MAX_FILES; i++) {
- CHECK(strncmp(src.paths[i], bss->paths_stat[i], MAX_PATH_LEN),
- "check",
- "failed to get stat path[%d]: %s vs %s\n",
- i, src.paths[i], bss->paths_stat[i]);
- CHECK(strncmp(src.paths[i], bss->paths_close[i], MAX_PATH_LEN),
- "check",
- "failed to get close path[%d]: %s vs %s\n",
- i, src.paths[i], bss->paths_close[i]);
+ struct want want = src.want[i];
+
+ /*
+ * Assert that we get the correct error code from bpf_d_path()
+ * when the underlying path contains a dentry that is backed by
+ * a dynamic name.
+ */
+ if (want.err) {
+ CHECK(want.err_code != bss->rets_stat[i], "check",
+ "failed to match stat return[%d]: got=%d, want=%ld [%s]\n",
+ i, bss->rets_stat[i], want.err_code,
+ bss->paths_stat[i]);
+ CHECK(want.err_code != bss->rets_close[i], "check",
+ "failed to match close return[%d]: got=%d, want=%ld [%s]\n",
+ i, bss->rets_close[i], want.err_code,
+ bss->paths_close[i]);
+ continue;
+ }
+
+ CHECK(strncmp(want.path, bss->paths_stat[i], MAX_PATH_LEN),
+ "check", "failed to get stat path[%d]: %s vs %s\n", i,
+ want.path, bss->paths_stat[i]);
+ CHECK(strncmp(want.path, bss->paths_close[i], MAX_PATH_LEN),
+ "check", "failed to get close path[%d]: %s vs %s\n", i,
+ want.path, bss->paths_close[i]);
/* The d_path helper returns size plus NUL char, hence + 1 */
CHECK(bss->rets_stat[i] != strlen(bss->paths_stat[i]) + 1,
"check",
- "failed to match stat return [%d]: %d vs %zd [%s]\n",
- i, bss->rets_stat[i], strlen(bss->paths_stat[i]) + 1,
+ "failed to match stat return [%d]: %d vs %zd [%s]\n", i,
+ bss->rets_stat[i], strlen(bss->paths_stat[i]) + 1,
bss->paths_stat[i]);
CHECK(bss->rets_close[i] != strlen(bss->paths_stat[i]) + 1,
"check",
- "failed to match stat return [%d]: %d vs %zd [%s]\n",
- i, bss->rets_close[i], strlen(bss->paths_close[i]) + 1,
+ "failed to match stat return [%d]: %d vs %zd [%s]\n", i,
+ bss->rets_close[i], strlen(bss->paths_close[i]) + 1,
bss->paths_stat[i]);
}
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
index 84e1f883f97b..fc2754f166ec 100644
--- a/tools/testing/selftests/bpf/progs/test_d_path.c
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -5,7 +5,7 @@
#include <bpf/bpf_tracing.h>
#define MAX_PATH_LEN 128
-#define MAX_FILES 7
+#define MAX_FILES 8
pid_t my_pid = 0;
__u32 cnt_stat = 0;
--
2.44.0.rc0.258.g7320e95886-goog
/M
next prev parent reply other threads:[~2024-02-20 9:27 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-20 9:27 [PATCH bpf-next 00/11] bpf: probe-read bpf_d_path() and add new acquire/release BPF kfuncs Matt Bobrowski
2024-02-20 9:27 ` [PATCH bpf-next 01/11] bpf: make bpf_d_path() helper use probe-read semantics Matt Bobrowski
2024-02-20 9:48 ` Christian Brauner
2024-02-20 13:22 ` Matt Bobrowski
2024-02-21 7:55 ` Christian Brauner
2024-02-21 13:38 ` Matt Bobrowski
2024-02-20 9:27 ` Matt Bobrowski [this message]
2024-02-20 9:27 ` [PATCH bpf-next 03/11] bpf: rename fs_kfunc_set_ids to lsm_kfunc_set_ids Matt Bobrowski
2024-02-20 9:27 ` [PATCH bpf-next 04/11] bpf: add new acquire/release BPF kfuncs for mm_struct Matt Bobrowski
2024-02-20 9:27 ` [PATCH bpf-next 05/11] bpf/selftests: add selftests for mm_struct acquire/release BPF kfuncs Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 06/11] bpf: add new acquire/release based BPF kfuncs for exe_file Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 07/11] bpf/selftests: add selftests for exe_file acquire/release BPF kfuncs Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 08/11] bpf: add acquire/release based BPF kfuncs for fs_struct's paths Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 09/11] bpf/selftests: add selftests for root/pwd path based BPF kfuncs Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 10/11] bpf: add trusted d_path() based BPF kfunc bpf_path_d_path() Matt Bobrowski
2024-02-20 9:28 ` [PATCH bpf-next 11/11] bpf/selftests: adapt selftests test_d_path for " Matt Bobrowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=18c7b587d43bbc7e80593bf51ea9d3eb99e47bc1.1708377880.git.mattbobrowski@google.com \
--to=mattbobrowski@google.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=daniel@iogearbox.net \
--cc=jannh@google.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@google.com \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).