On 08/02/2017 10:19 PM, Kees Cook wrote:
> This refactors the errno tests (since they all use the same pattern for
> their filter) and adds a RET_DATA field ordering test.
>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
This all looks good and is a great idea.
Reviewed-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
Tyler
> ---
> tools/testing/selftests/seccomp/seccomp_bpf.c | 95 ++++++++++++++++-----------
> 1 file changed, 58 insertions(+), 37 deletions(-)
>
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 73f5ea6778ce..ee78a53da5d1 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -136,7 +136,7 @@ TEST(no_new_privs_support)
> }
> }
>
> -/* Tests kernel support by checking for a copy_from_user() fault on * NULL. */
> +/* Tests kernel support by checking for a copy_from_user() fault on NULL. */
> TEST(mode_filter_support)
> {
> long ret;
> @@ -541,26 +541,30 @@ TEST(arg_out_of_range)
> EXPECT_EQ(EINVAL, errno);
> }
>
> +#define ERRNO_FILTER(name, errno) \
> + struct sock_filter _read_filter_##name[] = { \
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS, \
> + offsetof(struct seccomp_data, nr)), \
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_read, 0, 1), \
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ERRNO | errno), \
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW), \
> + }; \
> + struct sock_fprog prog_##name = { \
> + .len = (unsigned short)ARRAY_SIZE(_read_filter_##name), \
> + .filter = _read_filter_##name, \
> + }
> +
> +/* Make sure basic errno values are correctly passed through a filter. */
> TEST(ERRNO_valid)
> {
> - struct sock_filter filter[] = {
> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> - offsetof(struct seccomp_data, nr)),
> - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_read, 0, 1),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ERRNO | E2BIG),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> - };
> - struct sock_fprog prog = {
> - .len = (unsigned short)ARRAY_SIZE(filter),
> - .filter = filter,
> - };
> + ERRNO_FILTER(valid, E2BIG);
> long ret;
> pid_t parent = getppid();
>
> ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> ASSERT_EQ(0, ret);
>
> - ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_valid);
> ASSERT_EQ(0, ret);
>
> EXPECT_EQ(parent, syscall(__NR_getppid));
> @@ -568,26 +572,17 @@ TEST(ERRNO_valid)
> EXPECT_EQ(E2BIG, errno);
> }
>
> +/* Make sure an errno of zero is correctly handled by the arch code. */
> TEST(ERRNO_zero)
> {
> - struct sock_filter filter[] = {
> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> - offsetof(struct seccomp_data, nr)),
> - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_read, 0, 1),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ERRNO | 0),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> - };
> - struct sock_fprog prog = {
> - .len = (unsigned short)ARRAY_SIZE(filter),
> - .filter = filter,
> - };
> + ERRNO_FILTER(zero, 0);
> long ret;
> pid_t parent = getppid();
>
> ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> ASSERT_EQ(0, ret);
>
> - ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_zero);
> ASSERT_EQ(0, ret);
>
> EXPECT_EQ(parent, syscall(__NR_getppid));
> @@ -595,26 +590,21 @@ TEST(ERRNO_zero)
> EXPECT_EQ(0, read(0, NULL, 0));
> }
>
> +/*
> + * The SECCOMP_RET_DATA mask is 16 bits wide, but errno is smaller.
> + * This tests that the errno value gets capped correctly, fixed by
> + * 580c57f10768 ("seccomp: cap SECCOMP_RET_ERRNO data to MAX_ERRNO").
> + */
> TEST(ERRNO_capped)
> {
> - struct sock_filter filter[] = {
> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> - offsetof(struct seccomp_data, nr)),
> - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_read, 0, 1),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ERRNO | 4096),
> - BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> - };
> - struct sock_fprog prog = {
> - .len = (unsigned short)ARRAY_SIZE(filter),
> - .filter = filter,
> - };
> + ERRNO_FILTER(capped, 4096);
> long ret;
> pid_t parent = getppid();
>
> ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> ASSERT_EQ(0, ret);
>
> - ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_capped);
> ASSERT_EQ(0, ret);
>
> EXPECT_EQ(parent, syscall(__NR_getppid));
> @@ -622,6 +612,37 @@ TEST(ERRNO_capped)
> EXPECT_EQ(4095, errno);
> }
>
> +/*
> + * Filters are processed in reverse order: last applied is executed first.
> + * Since only the SECCOMP_RET_ACTION mask is tested for return values, the
> + * SECCOMP_RET_DATA mask results will follow the most recently applied
> + * matching filter return (and not the lowest or highest value).
> + */
> +TEST(ERRNO_order)
> +{
> + ERRNO_FILTER(first, 11);
> + ERRNO_FILTER(second, 13);
> + ERRNO_FILTER(third, 12);
> + long ret;
> + pid_t parent = getppid();
> +
> + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_first);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_second);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_third);
> + ASSERT_EQ(0, ret);
> +
> + EXPECT_EQ(parent, syscall(__NR_getppid));
> + EXPECT_EQ(-1, read(0, NULL, 0));
> + EXPECT_EQ(12, errno);
> +}
> +
> FIXTURE_DATA(TRAP) {
> struct sock_fprog prog;
> };
>
Attachment:
signature.asc
Description: OpenPGP digital signature
