From d8db84d71c0e539f7ce902e2fe297e535ba4d46c Mon Sep 17 00:00:00 2001 From: Martin KaFai Lau Date: Wed, 22 Mar 2023 14:52:45 -0700 Subject: [PATCH] selftests/bpf: Test task storage when local_storage->smap is NULL The current sk storage test ensures the memory free works when the local_storage->smap is NULL. This patch adds a task storage test to ensure the memory free code path works when local_storage->smap is NULL. Signed-off-by: Martin KaFai Lau Link: https://lore.kernel.org/r/20230322215246.1675516-5-martin.lau@linux.dev Signed-off-by: Alexei Starovoitov --- .../selftests/bpf/prog_tests/test_local_storage.c | 7 +-- tools/testing/selftests/bpf/progs/local_storage.c | 56 ++++++++++++++++------ 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c index 563a9c746b7b..bcf2e1905ed7 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c +++ b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c @@ -23,7 +23,7 @@ struct storage { /* Fork and exec the provided rm binary and return the exit code of the * forked process and its pid. */ -static int run_self_unlink(int *monitored_pid, const char *rm_path) +static int run_self_unlink(struct local_storage *skel, const char *rm_path) { int child_pid, child_status, ret; int null_fd; @@ -35,7 +35,7 @@ static int run_self_unlink(int *monitored_pid, const char *rm_path) dup2(null_fd, STDERR_FILENO); close(null_fd); - *monitored_pid = getpid(); + skel->bss->monitored_pid = getpid(); /* Use the copied /usr/bin/rm to delete itself * /tmp/copy_of_rm /tmp/copy_of_rm. */ @@ -44,6 +44,7 @@ static int run_self_unlink(int *monitored_pid, const char *rm_path) exit(errno); } else if (child_pid > 0) { waitpid(child_pid, &child_status, 0); + ASSERT_EQ(skel->data->task_storage_result, 0, "task_storage_result"); return WEXITSTATUS(child_status); } @@ -133,7 +134,7 @@ void test_test_local_storage(void) * unlink its executable. This operation should be denied by the loaded * LSM program. */ - err = run_self_unlink(&skel->bss->monitored_pid, tmp_exec_path); + err = run_self_unlink(skel, tmp_exec_path); if (!ASSERT_EQ(err, EPERM, "run_self_unlink")) goto close_prog_rmdir; diff --git a/tools/testing/selftests/bpf/progs/local_storage.c b/tools/testing/selftests/bpf/progs/local_storage.c index c8ba7207f5a5..bc8ea56671a1 100644 --- a/tools/testing/selftests/bpf/progs/local_storage.c +++ b/tools/testing/selftests/bpf/progs/local_storage.c @@ -16,6 +16,7 @@ char _license[] SEC("license") = "GPL"; int monitored_pid = 0; int inode_storage_result = -1; int sk_storage_result = -1; +int task_storage_result = -1; struct local_storage { struct inode *exec_inode; @@ -50,26 +51,57 @@ struct { __type(value, struct local_storage); } task_storage_map SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_TASK_STORAGE); + __uint(map_flags, BPF_F_NO_PREALLOC); + __type(key, int); + __type(value, struct local_storage); +} task_storage_map2 SEC(".maps"); + SEC("lsm/inode_unlink") int BPF_PROG(unlink_hook, struct inode *dir, struct dentry *victim) { __u32 pid = bpf_get_current_pid_tgid() >> 32; + struct bpf_local_storage *local_storage; struct local_storage *storage; + struct task_struct *task; bool is_self_unlink; if (pid != monitored_pid) return 0; - storage = bpf_task_storage_get(&task_storage_map, - bpf_get_current_task_btf(), 0, 0); - if (storage) { - /* Don't let an executable delete itself */ - is_self_unlink = storage->exec_inode == victim->d_inode; - if (is_self_unlink) - return -EPERM; - } + task = bpf_get_current_task_btf(); + if (!task) + return 0; - return 0; + task_storage_result = -1; + + storage = bpf_task_storage_get(&task_storage_map, task, 0, 0); + if (!storage) + return 0; + + /* Don't let an executable delete itself */ + is_self_unlink = storage->exec_inode == victim->d_inode; + + storage = bpf_task_storage_get(&task_storage_map2, task, 0, + BPF_LOCAL_STORAGE_GET_F_CREATE); + if (!storage || storage->value) + return 0; + + if (bpf_task_storage_delete(&task_storage_map, task)) + return 0; + + /* Ensure that the task_storage_map is disconnected from the storage. + * The storage memory should not be freed back to the + * bpf_mem_alloc. + */ + local_storage = task->bpf_storage; + if (!local_storage || local_storage->smap) + return 0; + + task_storage_result = 0; + + return is_self_unlink ? -EPERM : 0; } SEC("lsm.s/inode_rename") @@ -139,11 +171,7 @@ int BPF_PROG(socket_bind, struct socket *sock, struct sockaddr *address, if (bpf_sk_storage_delete(&sk_storage_map, sock->sk)) return 0; - /* Ensure that the sk_storage_map is disconnected from the storage. - * The storage memory should not be freed back to the - * bpf_mem_alloc of the sk_bpf_storage_map because - * sk_bpf_storage_map may have been gone. - */ + /* Ensure that the sk_storage_map is disconnected from the storage. */ if (!sock->sk->sk_bpf_storage || sock->sk->sk_bpf_storage->smap) return 0; -- 2.11.0