Commit 1d528e79 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf-fix-bpf_d_path-helper-prototype' 

Shuran Liu says:

====================
bpf: fix bpf_d_path() helper prototype

Hi,

This series fixes a verifier issue with bpf_d_path() and adds a
regression test to cover its use within a hook function.

Patch 1 updates the bpf_d_path() helper prototype so that the second
argument is marked as MEM_WRITE. This makes it explicit to the verifier
that the helper writes into the provided buffer.

Patch 2 extends the existing d_path selftest to cover incorrect verifier
assumptions caused by an incorrect function prototype. The test program calls
bpf_d_path() and checks if the first character of the path can be read.
It ensures the verifier does not assume the buffer remains unwritten.

Changelog
=========

v5:
  - Moved the temporary file for the fallocate test from /tmp to /dev/shm
    Since bpf CI's 9P filesystem under /tmp does not support fallocate.

v4:
  - Use the fallocate hook instead of an LSM hook to simplify the selftest,
    as suggested by Matt and Alexei.
  - Add a utility function in test_d_path.c to load the BPF program,
    improving code reuse.

v3:
  - Switch the pathname prefix loop to use bpf_for() instead of
    #pragma unroll, as suggested by Matt.
  - Remove /tmp/bpf_d_path_test in the test cleanup path.
  - Add the missing Reviewed-by tags.

v2:
  - Merge the new test into the existing d_path selftest rather than
  creating new files.
  - Add PID filtering in the LSM program to avoid nondeterministic failures
  due to unrelated processes triggering bprm_check_security.
  - Synchronize child execution using a pipe to ensure deterministic
  updates to the PID.

Thanks for your time and reviews.
====================

Link: https://patch.msgid.link/20251206141210.3148-1-electronlsr@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 9489d457 79e247d6

kernel/trace/bpf_trace.c

+1 −1
Original line number Diff line number Diff line
@@ -965,7 +965,7 @@ static const struct bpf_func_proto bpf_d_path_proto = { 
	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_PTR_TO_BTF_ID,

	.arg1_btf_id	= &bpf_d_path_btf_ids[0],

	.arg2_type	= ARG_PTR_TO_MEM,

	.arg2_type	= ARG_PTR_TO_MEM | MEM_WRITE,

	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,

	.allowed	= bpf_d_path_allowed,

};

tools/testing/selftests/bpf/prog_tests/d_path.c

+72 −17
Original line number Diff line number Diff line
@@ -38,6 +38,14 @@ static int set_pathname(int fd, pid_t pid) 
	return readlink(buf, src.paths[src.cnt++], MAX_PATH_LEN);

}



static inline long syscall_close(int fd)

{

	return syscall(__NR_close_range,

			(unsigned int)fd,

			(unsigned int)fd,

			0u);

}



static int trigger_fstat_events(pid_t pid)

{

	int sockfd = -1, procfd = -1, devfd = -1;
@@ -104,18 +112,34 @@ static int trigger_fstat_events(pid_t pid) 
	/* sys_close no longer triggers filp_close, but we can

	 * call sys_close_range instead which still does

	 */

#define close(fd) syscall(__NR_close_range, fd, fd, 0)

	syscall_close(pipefd[0]);

	syscall_close(pipefd[1]);

	syscall_close(sockfd);

	syscall_close(procfd);

	syscall_close(devfd);

	syscall_close(localfd);

	syscall_close(indicatorfd);

	return ret;

}



	close(pipefd[0]);

	close(pipefd[1]);

	close(sockfd);

	close(procfd);

	close(devfd);

	close(localfd);

	close(indicatorfd);

static void attach_and_load(struct test_d_path **skel)

{

	int err;



#undef close

	return ret;

	*skel = test_d_path__open_and_load();

	if (CHECK(!*skel, "setup", "d_path skeleton failed\n"))

		goto cleanup;



	err = test_d_path__attach(*skel);

	if (CHECK(err, "setup", "attach failed: %d\n", err))

		goto cleanup;



	(*skel)->bss->my_pid = getpid();

	return;



cleanup:

	test_d_path__destroy(*skel);

	*skel = NULL;

}



static void test_d_path_basic(void)
@@ -124,16 +148,11 @@ static void test_d_path_basic(void) 
	struct test_d_path *skel;

	int err;



	skel = test_d_path__open_and_load();

	if (CHECK(!skel, "setup", "d_path skeleton failed\n"))

		goto cleanup;



	err = test_d_path__attach(skel);

	if (CHECK(err, "setup", "attach failed: %d\n", err))

	attach_and_load(&skel);

	if (!skel)

		goto cleanup;



	bss = skel->bss;

	bss->my_pid = getpid();



	err = trigger_fstat_events(bss->my_pid);

	if (err < 0)
@@ -195,6 +214,39 @@ static void test_d_path_check_types(void) 
	test_d_path_check_types__destroy(skel);

}



/* Check if the verifier correctly generates code for

 * accessing the memory modified by d_path helper.

 */

static void test_d_path_mem_access(void)

{

	int localfd = -1;

	char path_template[] = "/dev/shm/d_path_loadgen.XXXXXX";

	struct test_d_path__bss *bss;

	struct test_d_path *skel;



	attach_and_load(&skel);

	if (!skel)

		goto cleanup;



	bss = skel->bss;



	localfd = mkstemp(path_template);

	if (CHECK(localfd < 0, "trigger", "mkstemp failed\n"))

		goto cleanup;



	if (CHECK(fallocate(localfd, 0, 0, 1024) < 0, "trigger", "fallocate failed\n"))

		goto cleanup;

	remove(path_template);



	if (CHECK(!bss->path_match_fallocate, "check",

		  "failed to read fallocate path"))

		goto cleanup;



cleanup:

	syscall_close(localfd);

	test_d_path__destroy(skel);

}



void test_d_path(void)

{

	if (test__start_subtest("basic"))
@@ -205,4 +257,7 @@ void test_d_path(void) 


	if (test__start_subtest("check_alloc_mem"))

		test_d_path_check_types();



	if (test__start_subtest("check_mem_access"))

		test_d_path_mem_access();

}

tools/testing/selftests/bpf/progs/test_d_path.c

+23 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ int rets_close[MAX_FILES] = {}; 


int called_stat = 0;

int called_close = 0;

int path_match_fallocate = 0;



SEC("fentry/security_inode_getattr")

int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
@@ -62,4 +63,26 @@ int BPF_PROG(prog_close, struct file *file, void *id) 
	return 0;

}



SEC("fentry/vfs_fallocate")

int BPF_PROG(prog_fallocate, struct file *file, int mode, loff_t offset, loff_t len)

{

	pid_t pid = bpf_get_current_pid_tgid() >> 32;

	int ret = 0;

	char path_fallocate[MAX_PATH_LEN] = {};



	if (pid != my_pid)

		return 0;



	ret = bpf_d_path(&file->f_path,

			 path_fallocate, MAX_PATH_LEN);

	if (ret < 0)

		return 0;



	if (!path_fallocate[0])

		return 0;



	path_match_fallocate = 1;

	return 0;

}



char _license[] SEC("license") = "GPL";