Re: [PATCH bpf-next v4 8/9] bpf: Add tests for task_local_storage
On Thu, Nov 5, 2020 at 11:03 PM Martin KaFai Lau wrote: > > On Thu, Nov 05, 2020 at 03:47:54PM +0100, KP Singh wrote: > > From: KP Singh > > > > The test exercises the syscall based map operations by creating a pidfd > > for the current process. > > [...] > > +} > > + > > +unsigned int duration; > static Fixed. > > > + > > +#define TEST_STORAGE_VALUE 0xbeefdead > > + > > +struct storage { > > + void *inode; > > + unsigned int value; > > + /* Lock ensures that spin locked versions of local stoage operations > > + * also work, most operations in this tests are still single threaded > > + */ > > + struct bpf_spin_lock lock; > > +}; > > + > > +/* Copies an rm binary to a temp file. dest is a mkstemp template */ > > +int copy_rm(char *dest) > static FIxed. [...] > > + ret = chmod(dest, 0100); > > + if (ret == -1) > > + return errno; > > + > > + close(fd_in); > > + close(fd_out); > fd_in and fd_out are not closed in error cases. Fixed. > > > { > > - char fname[PATH_MAX] = "/tmp/fileXX"; > > - int fd; > > + int ret, fd_in, fd_out; > > + struct stat stat; > > [...] > > + */ > > +int run_self_unlink(int *monitored_pid, const char *rm_path) > static Fixed. > > [ ... ] > > > +bool check_syscall_operations(int map_fd, int obj_fd) > static Fixed. > > [ ... ] > > > void test_test_local_storage(void) > > { > > + char tmp_exec_path[PATH_MAX] = "/tmp/copy_of_rmXX"; > > + int err, serv_sk = -1, task_fd = -1; > > struct local_storage *skel = NULL; > > - int err, duration = 0, serv_sk = -1; > > [...] > > + err = unlink(tmp_exec_path); > Will tmp_exec_path file be removed if there is error earlier? No. Since I cannot move this unlink as inode_unlink LSM hook sets the inode_storage result, I added another label close_prog_unlink which cleans it up for the errors before this succeeds. > > > + if (CHECK(err != 0, "unlink", "unable to unlink %s: %d", > > tmp_exec_path, > > + errno)) > > goto close_prog; > > > > CHECK(skel->data->inode_storage_result != 0, "inode_storage_result", > > @@ -56,5 +200,6 @@ void test_test_local_storage(void) > > close(serv_sk); > > > > close_prog: > > + close(task_fd); > > local_storage__destroy(skel); > > }
Re: [PATCH bpf-next v4 8/9] bpf: Add tests for task_local_storage
On Thu, Nov 05, 2020 at 03:47:54PM +0100, KP Singh wrote: > From: KP Singh > > The test exercises the syscall based map operations by creating a pidfd > for the current process. > > For verifying kernel / LSM functionality, the test implements a simple > MAC policy which denies an executable from unlinking itself. The LSM > program bprm_committed_creds sets a task_local_storage with a pointer to > the inode. This is then used to detect if the task is trying to unlink > itself in the inode_unlink LSM hook. > > The test copies /bin/rm to /tmp and executes it in a child thread with > the intention of deleting itself. A successful test should prevent the > the running executable from deleting itself. > > The bpf programs are also updated to call bpf_spin_{lock, unlock} to > trigger the verfier checks for spin locks. > > The temporary file is cleaned up later in the test. > > Signed-off-by: KP Singh > --- > .../bpf/prog_tests/test_local_storage.c | 167 -- > .../selftests/bpf/progs/local_storage.c | 61 ++- > 2 files changed, 210 insertions(+), 18 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 91cd6f357246..feba23f8848b 100644 > --- a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c > +++ b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c > @@ -4,30 +4,149 @@ > * Copyright (C) 2020 Google LLC. > */ > > +#define _GNU_SOURCE > + > +#include > +#include > +#include > #include > #include > > #include "local_storage.skel.h" > #include "network_helpers.h" > > -int create_and_unlink_file(void) > +static inline int sys_pidfd_open(pid_t pid, unsigned int flags) > +{ > + return syscall(__NR_pidfd_open, pid, flags); > +} > + > +unsigned int duration; static > + > +#define TEST_STORAGE_VALUE 0xbeefdead > + > +struct storage { > + void *inode; > + unsigned int value; > + /* Lock ensures that spin locked versions of local stoage operations > + * also work, most operations in this tests are still single threaded > + */ > + struct bpf_spin_lock lock; > +}; > + > +/* Copies an rm binary to a temp file. dest is a mkstemp template */ > +int copy_rm(char *dest) static > { > - char fname[PATH_MAX] = "/tmp/fileXX"; > - int fd; > + int ret, fd_in, fd_out; > + struct stat stat; > > - fd = mkstemp(fname); > - if (fd < 0) > - return fd; > + fd_in = open("/bin/rm", O_RDONLY); > + if (fd_in < 0) > + return fd_in; > > - close(fd); > - unlink(fname); > + fd_out = mkstemp(dest); > + if (fd_out < 0) > + return fd_out; > + > + ret = fstat(fd_in, ); > + if (ret == -1) > + return errno; > + > + ret = copy_file_range(fd_in, NULL, fd_out, NULL, stat.st_size, 0); > + if (ret == -1) > + return errno; > + > + /* Set executable permission on the copied file */ > + ret = chmod(dest, 0100); > + if (ret == -1) > + return errno; > + > + close(fd_in); > + close(fd_out); fd_in and fd_out are not closed in error cases. > return 0; > } > > +/* Fork and exec the provided rm binary and return the exit code of the > + * forked process and its pid. > + */ > +int run_self_unlink(int *monitored_pid, const char *rm_path) static [ ... ] > +bool check_syscall_operations(int map_fd, int obj_fd) static [ ... ] > void test_test_local_storage(void) > { > + char tmp_exec_path[PATH_MAX] = "/tmp/copy_of_rmXX"; > + int err, serv_sk = -1, task_fd = -1; > struct local_storage *skel = NULL; > - int err, duration = 0, serv_sk = -1; > > skel = local_storage__open_and_load(); > if (CHECK(!skel, "skel_load", "lsm skeleton failed\n")) > @@ -37,10 +156,35 @@ void test_test_local_storage(void) > if (CHECK(err, "attach", "lsm attach failed: %d\n", err)) > goto close_prog; > > + task_fd = sys_pidfd_open(getpid(), 0); > + if (CHECK(task_fd < 0, "pidfd_open", > + "failed to get pidfd err:%d, errno:%d", task_fd, errno)) > + goto close_prog; > + > + if (!check_syscall_operations(bpf_map__fd(skel->maps.task_storage_map), > + task_fd)) > + goto close_prog; > + > + err = copy_rm(tmp_exec_path); > + if (CHECK(err < 0, "copy_rm", "err %d errno %d\n", err, errno)) > + goto close_prog; > + > + /* Sets skel->bss->monitored_pid to the pid of the forked child > + * forks a child process that executes tmp_exec_path and tries to > + * unlink its executable. This operation should be denied by the loaded > + * LSM program. > + */ > + err = run_self_unlink(>bss->monitored_pid, tmp_exec_path); > + if (CHECK(err != EPERM, "run_self_unlink", "err %d want EPERM\n", err)) > + goto close_prog; > + > +
[PATCH bpf-next v4 8/9] bpf: Add tests for task_local_storage
From: KP Singh The test exercises the syscall based map operations by creating a pidfd for the current process. For verifying kernel / LSM functionality, the test implements a simple MAC policy which denies an executable from unlinking itself. The LSM program bprm_committed_creds sets a task_local_storage with a pointer to the inode. This is then used to detect if the task is trying to unlink itself in the inode_unlink LSM hook. The test copies /bin/rm to /tmp and executes it in a child thread with the intention of deleting itself. A successful test should prevent the the running executable from deleting itself. The bpf programs are also updated to call bpf_spin_{lock, unlock} to trigger the verfier checks for spin locks. The temporary file is cleaned up later in the test. Signed-off-by: KP Singh --- .../bpf/prog_tests/test_local_storage.c | 167 -- .../selftests/bpf/progs/local_storage.c | 61 ++- 2 files changed, 210 insertions(+), 18 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 91cd6f357246..feba23f8848b 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c +++ b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c @@ -4,30 +4,149 @@ * Copyright (C) 2020 Google LLC. */ +#define _GNU_SOURCE + +#include +#include +#include #include #include #include "local_storage.skel.h" #include "network_helpers.h" -int create_and_unlink_file(void) +static inline int sys_pidfd_open(pid_t pid, unsigned int flags) +{ + return syscall(__NR_pidfd_open, pid, flags); +} + +unsigned int duration; + +#define TEST_STORAGE_VALUE 0xbeefdead + +struct storage { + void *inode; + unsigned int value; + /* Lock ensures that spin locked versions of local stoage operations +* also work, most operations in this tests are still single threaded +*/ + struct bpf_spin_lock lock; +}; + +/* Copies an rm binary to a temp file. dest is a mkstemp template */ +int copy_rm(char *dest) { - char fname[PATH_MAX] = "/tmp/fileXX"; - int fd; + int ret, fd_in, fd_out; + struct stat stat; - fd = mkstemp(fname); - if (fd < 0) - return fd; + fd_in = open("/bin/rm", O_RDONLY); + if (fd_in < 0) + return fd_in; - close(fd); - unlink(fname); + fd_out = mkstemp(dest); + if (fd_out < 0) + return fd_out; + + ret = fstat(fd_in, ); + if (ret == -1) + return errno; + + ret = copy_file_range(fd_in, NULL, fd_out, NULL, stat.st_size, 0); + if (ret == -1) + return errno; + + /* Set executable permission on the copied file */ + ret = chmod(dest, 0100); + if (ret == -1) + return errno; + + close(fd_in); + close(fd_out); return 0; } +/* Fork and exec the provided rm binary and return the exit code of the + * forked process and its pid. + */ +int run_self_unlink(int *monitored_pid, const char *rm_path) +{ + int child_pid, child_status, ret; + int null_fd; + + child_pid = fork(); + if (child_pid == 0) { + null_fd = open("/dev/null", O_WRONLY); + dup2(null_fd, STDOUT_FILENO); + dup2(null_fd, STDERR_FILENO); + close(null_fd); + + *monitored_pid = getpid(); + /* Use the copied /usr/bin/rm to delete itself +* /tmp/copy_of_rm /tmp/copy_of_rm. +*/ + ret = execlp(rm_path, rm_path, rm_path, NULL); + if (ret) + exit(errno); + } else if (child_pid > 0) { + waitpid(child_pid, _status, 0); + return WEXITSTATUS(child_status); + } + + return -EINVAL; +} + +bool check_syscall_operations(int map_fd, int obj_fd) +{ + struct storage val = { .value = TEST_STORAGE_VALUE, .lock = { 0 } }, + lookup_val = { .value = 0, .lock = { 0 } }; + int err; + + /* Looking up an existing element should fail initially */ + err = bpf_map_lookup_elem_flags(map_fd, _fd, _val, + BPF_F_LOCK); + if (CHECK(!err || errno != ENOENT, "bpf_map_lookup_elem", + "err:%d errno:%d\n", err, errno)) + return false; + + /* Create a new element */ + err = bpf_map_update_elem(map_fd, _fd, , + BPF_NOEXIST | BPF_F_LOCK); + if (CHECK(err < 0, "bpf_map_update_elem", "err:%d errno:%d\n", err, + errno)) + return false; + + /* Lookup the newly created element */ + err = bpf_map_lookup_elem_flags(map_fd, _fd, _val, + BPF_F_LOCK); + if (CHECK(err < 0, "bpf_map_lookup_elem", "err:%d errno:%d",