selftests/bpf: Test task storage when local_storage->smap is NULL
authorMartin KaFai Lau <martin.lau@kernel.org>
Wed, 22 Mar 2023 21:52:45 +0000 (14:52 -0700)
committerAlexei Starovoitov <ast@kernel.org>
Sun, 26 Mar 2023 02:52:52 +0000 (19:52 -0700)
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 <martin.lau@kernel.org>
Link: https://lore.kernel.org/r/20230322215246.1675516-5-martin.lau@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
tools/testing/selftests/bpf/prog_tests/test_local_storage.c
tools/testing/selftests/bpf/progs/local_storage.c

index 563a9c7..bcf2e19 100644 (file)
@@ -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;
 
index c8ba720..bc8ea56 100644 (file)
@@ -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;