Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752059AbdHHBYU (ORCPT ); Mon, 7 Aug 2017 21:24:20 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:35184 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751677AbdHHBYS (ORCPT ); Mon, 7 Aug 2017 21:24:18 -0400 Subject: Re: [PATCH 3/4] selftests/seccomp: Refactor RET_ERRNO tests To: Kees Cook , linux-kernel@vger.kernel.org Cc: Fabricio Voznika , Andy Lutomirski , Will Drewry , Shuah Khan , linux-kselftest@vger.kernel.org, linux-security-module@vger.kernel.org References: <1501730353-46840-1-git-send-email-keescook@chromium.org> <1501730353-46840-4-git-send-email-keescook@chromium.org> From: Tyler Hicks Message-ID: <87fb1e1a-a391-478b-15dd-3cec730c0853@canonical.com> Date: Mon, 7 Aug 2017 20:24:11 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1501730353-46840-4-git-send-email-keescook@chromium.org> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="kCX6o6kL7qlmK7sfXTBMqGeCWqbs3rvk8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7777 Lines: 234 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --kCX6o6kL7qlmK7sfXTBMqGeCWqbs3rvk8 Content-Type: multipart/mixed; boundary="bRMrrBXxErmBwOIA90cbwtsKVrFPIoNVI"; protected-headers="v1" From: Tyler Hicks To: Kees Cook , linux-kernel@vger.kernel.org Cc: Fabricio Voznika , Andy Lutomirski , Will Drewry , Shuah Khan , linux-kselftest@vger.kernel.org, linux-security-module@vger.kernel.org Message-ID: <87fb1e1a-a391-478b-15dd-3cec730c0853@canonical.com> Subject: Re: [PATCH 3/4] selftests/seccomp: Refactor RET_ERRNO tests References: <1501730353-46840-1-git-send-email-keescook@chromium.org> <1501730353-46840-4-git-send-email-keescook@chromium.org> In-Reply-To: <1501730353-46840-4-git-send-email-keescook@chromium.org> --bRMrrBXxErmBwOIA90cbwtsKVrFPIoNVI Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Kees Cook This all looks good and is a great idea. Reviewed-by: Tyler Hicks Tyler > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 95 ++++++++++++++++---= -------- > 1 file changed, 58 insertions(+), 37 deletions(-) >=20 > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/test= ing/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) > } > } > =20 > -/* 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 NU= LL. */ > TEST(mode_filter_support) > { > long ret; > @@ -541,26 +541,30 @@ TEST(arg_out_of_range) > EXPECT_EQ(EINVAL, errno); > } > =20 > +#define ERRNO_FILTER(name, errno) \ > + struct sock_filter _read_filter_##name[] =3D { \ > + 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 =3D { \ > + .len =3D (unsigned short)ARRAY_SIZE(_read_filter_##name), \ > + .filter =3D _read_filter_##name, \ > + } > + > +/* Make sure basic errno values are correctly passed through a filter.= */ > TEST(ERRNO_valid) > { > - struct sock_filter filter[] =3D { > - 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 =3D { > - .len =3D (unsigned short)ARRAY_SIZE(filter), > - .filter =3D filter, > - }; > + ERRNO_FILTER(valid, E2BIG); > long ret; > pid_t parent =3D getppid(); > =20 > ret =3D prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > =20 > - ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog); > + ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_valid); > ASSERT_EQ(0, ret); > =20 > EXPECT_EQ(parent, syscall(__NR_getppid)); > @@ -568,26 +572,17 @@ TEST(ERRNO_valid) > EXPECT_EQ(E2BIG, errno); > } > =20 > +/* Make sure an errno of zero is correctly handled by the arch code. *= / > TEST(ERRNO_zero) > { > - struct sock_filter filter[] =3D { > - 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 =3D { > - .len =3D (unsigned short)ARRAY_SIZE(filter), > - .filter =3D filter, > - }; > + ERRNO_FILTER(zero, 0); > long ret; > pid_t parent =3D getppid(); > =20 > ret =3D prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > =20 > - ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog); > + ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_zero); > ASSERT_EQ(0, ret); > =20 > EXPECT_EQ(parent, syscall(__NR_getppid)); > @@ -595,26 +590,21 @@ TEST(ERRNO_zero) > EXPECT_EQ(0, read(0, NULL, 0)); > } > =20 > +/* > + * 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[] =3D { > - 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 =3D { > - .len =3D (unsigned short)ARRAY_SIZE(filter), > - .filter =3D filter, > - }; > + ERRNO_FILTER(capped, 4096); > long ret; > pid_t parent =3D getppid(); > =20 > ret =3D prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > =20 > - ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog); > + ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_capped); > ASSERT_EQ(0, ret); > =20 > EXPECT_EQ(parent, syscall(__NR_getppid)); > @@ -622,6 +612,37 @@ TEST(ERRNO_capped) > EXPECT_EQ(4095, errno); > } > =20 > +/* > + * Filters are processed in reverse order: last applied is executed fi= rst. > + * 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 =3D getppid(); > + > + ret =3D prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > + ASSERT_EQ(0, ret); > + > + ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_first); > + ASSERT_EQ(0, ret); > + > + ret =3D prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_second); > + ASSERT_EQ(0, ret); > + > + ret =3D 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; > }; >=20 --bRMrrBXxErmBwOIA90cbwtsKVrFPIoNVI-- --kCX6o6kL7qlmK7sfXTBMqGeCWqbs3rvk8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCgAGBQJZiRK7AAoJENaSAD2qAscK+OsQAL0ZicUN3HGxXvN5/2J4hJU+ ohmOX5yIEs2ktJ7wETd6ciFOxpe07AXwMKrc2hVaNXSKu+qKhyAda16hXBTmY9pJ 2T7dDzy+kF3OgFQlp4Kp5qzanMQ40Juk+64QLVFlNcQbCEoQRadR9ZCvBx1GChG4 s2SW/lBxNEc8iqxIFUQqk9oXFKzBahrxfT0NWUPbd88JnN3pZN0hQS8rSdmuob66 eri0IkJUkBVO9eWiQCqANTlY5bHb+vcK1Q6zePKQ3HDrFjnfhtjfk1kBholMOf7Y NshYidcvRtERIgTgnmVib/nTeNWPzB+eBaEivS7mThxHsi8unceeGcAALl7VDKOA Rpd4LqGpezdcGu9sj6xzcDSbJLTPaMWMfHd5+3t5FyNIJ/Vg08yenMGMkFWr03GZ 5TaHWAPQWB+B99o1z7xS3mufyL0A0tkxSUfKMwt/vKrZ+oY0ItMHYnq0U4FMSiEl o1rsxj1noBGHrWCuOy2+MrzZwGZzarOBN5DjcJDLoUFOPAbMpOyAbWnqYH/esxm+ joQtExY6+2VW+lLgBb/4l/wmzcYlGp9AJQ2f6R4rEOTEB2fuHpS1KRb6lfW52/gs VV5I79UII9IehiJT93mEuXdHbNkbeHkF5OgFcwD0ImCl5DNzIoVAZHOatt1RxDqJ 2X1wzId4ijzQWSZpA6M2 =VE/1 -----END PGP SIGNATURE----- --kCX6o6kL7qlmK7sfXTBMqGeCWqbs3rvk8--