| Message ID | 20200410120006.6261-2-cascardo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Disco/Eoan] Revert "selftests/seccomp: Catch garbage on SECCOMP_IOCTL_NOTIF_RECV" | expand |
On Fri, Apr 10, 2020 at 09:00:06AM -0300, Thadeu Lima de Souza Cascardo wrote: > BugLink: https://bugs.launchpad.net/bugs/1862588 > > This reverts upstream commit e4ab5ccc357b978999328fadae164e098c26fa40. > > Running linux/tools/testing/selftests/seccomp/seccomp_bpf will fail > with: > > seccomp_bpf.c:3149:global.user_notification_basic:Expected -1 > (18446744073709551615) == ret (0) > seccomp_bpf.c:3150:global.user_notification_basic:Expected EINVAL (22) > == errno (0) > global.user_notification_basic: Test failed at step #3 > [ FAIL ] global.user_notification_basic > > The test is checking that the given structure which the kernel will write > to is all zeroes and would return EINVAL otherwise. It's doing it because > it wants userspace to have the possibility in the future to give data there > indicating support for an extension that might be developed in the future. > > As the test is there right now, not applying the breaking uABI fix might > cause us to miss applications that would break in future kernels. As the > backport for that is prone for more regression potential, we are deciding > to revert the new test. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Makes sense to me. Acked-by: Andrea Righi <andrea.righi@canonical.com>
On 10/04/2020 13:00, Thadeu Lima de Souza Cascardo wrote: > BugLink: https://bugs.launchpad.net/bugs/1862588 > > This reverts upstream commit e4ab5ccc357b978999328fadae164e098c26fa40. > > Running linux/tools/testing/selftests/seccomp/seccomp_bpf will fail > with: > > seccomp_bpf.c:3149:global.user_notification_basic:Expected -1 > (18446744073709551615) == ret (0) > seccomp_bpf.c:3150:global.user_notification_basic:Expected EINVAL (22) > == errno (0) > global.user_notification_basic: Test failed at step #3 > [ FAIL ] global.user_notification_basic > > The test is checking that the given structure which the kernel will write > to is all zeroes and would return EINVAL otherwise. It's doing it because > it wants userspace to have the possibility in the future to give data there > indicating support for an extension that might be developed in the future. > > As the test is there right now, not applying the breaking uABI fix might > cause us to miss applications that would break in future kernels. As the > backport for that is prone for more regression potential, we are deciding > to revert the new test. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index a819ab57fafe..f80b248eb389 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -3154,18 +3154,7 @@ TEST(user_notification_basic) > EXPECT_GT(poll(&pollfd, 1, -1), 0); > EXPECT_EQ(pollfd.revents, POLLIN); > > - /* Test that we can't pass garbage to the kernel. */ > - memset(&req, 0, sizeof(req)); > - req.pid = -1; > - errno = 0; > - ret = ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req); > - EXPECT_EQ(-1, ret); > - EXPECT_EQ(EINVAL, errno); > - > - if (ret) { > - req.pid = 0; > - EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); > - } > + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); > > pollfd.fd = listener; > pollfd.events = POLLIN | POLLOUT; > Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index a819ab57fafe..f80b248eb389 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3154,18 +3154,7 @@ TEST(user_notification_basic) EXPECT_GT(poll(&pollfd, 1, -1), 0); EXPECT_EQ(pollfd.revents, POLLIN); - /* Test that we can't pass garbage to the kernel. */ - memset(&req, 0, sizeof(req)); - req.pid = -1; - errno = 0; - ret = ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req); - EXPECT_EQ(-1, ret); - EXPECT_EQ(EINVAL, errno); - - if (ret) { - req.pid = 0; - EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); - } + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); pollfd.fd = listener; pollfd.events = POLLIN | POLLOUT;
BugLink: https://bugs.launchpad.net/bugs/1862588 This reverts upstream commit e4ab5ccc357b978999328fadae164e098c26fa40. Running linux/tools/testing/selftests/seccomp/seccomp_bpf will fail with: seccomp_bpf.c:3149:global.user_notification_basic:Expected -1 (18446744073709551615) == ret (0) seccomp_bpf.c:3150:global.user_notification_basic:Expected EINVAL (22) == errno (0) global.user_notification_basic: Test failed at step #3 [ FAIL ] global.user_notification_basic The test is checking that the given structure which the kernel will write to is all zeroes and would return EINVAL otherwise. It's doing it because it wants userspace to have the possibility in the future to give data there indicating support for an extension that might be developed in the future. As the test is there right now, not applying the breaking uABI fix might cause us to miss applications that would break in future kernels. As the backport for that is prone for more regression potential, we are deciding to revert the new test. Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> --- tools/testing/selftests/seccomp/seccomp_bpf.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)