[PATCH] selftests/seccomp: Accept any valid fd in user_notification_addfd

[PATCH] selftests/seccomp: Accept any valid fd in user_notification_addfd

[Seth Forshee]

This test expects fds to have specific values, which works fine
when the test is run standalone. However, the kselftest runner
consumes a couple of extra fds for redirection when running
tests, so the test fails when run via kselftest.

Change the test to pass on any valid fd number.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 26c72f2b61b1..9338df6f4ca8 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -4019,18 +4019,14 @@ TEST(user_notification_addfd)
 
 	/* Verify we can set an arbitrary remote fd */
 	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd);
-	/*
-	 * The child has fds 0(stdin), 1(stdout), 2(stderr), 3(memfd),
-	 * 4(listener), so the newly allocated fd should be 5.
-	 */
-	EXPECT_EQ(fd, 5);
+	EXPECT_GE(fd, 0);
 	EXPECT_EQ(filecmp(getpid(), pid, memfd, fd), 0);
 
 	/* Verify we can set an arbitrary remote fd with large size */
 	memset(&big, 0x0, sizeof(big));
 	big.addfd = addfd;
 	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_BIG, &big);
-	EXPECT_EQ(fd, 6);
+	EXPECT_GE(fd, 0);
 
 	/* Verify we can set a specific remote fd */
 	addfd.newfd = 42;
-- 
2.29.2

Re: [PATCH] selftests/seccomp: Accept any valid fd in user_notification_addfd

[Shuah Khan]

On 1/28/21 9:17 AM, Seth Forshee wrote:
> This test expects fds to have specific values, which works fine
> when the test is run standalone. However, the kselftest runner
> consumes a couple of extra fds for redirection when running
> tests, so the test fails when run via kselftest.
> 
> Change the test to pass on any valid fd number.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>   tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 26c72f2b61b1..9338df6f4ca8 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -4019,18 +4019,14 @@ TEST(user_notification_addfd)
>   
>   	/* Verify we can set an arbitrary remote fd */
>   	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd);
> -	/*
> -	 * The child has fds 0(stdin), 1(stdout), 2(stderr), 3(memfd),
> -	 * 4(listener), so the newly allocated fd should be 5.
> -	 */
> -	EXPECT_EQ(fd, 5);
> +	EXPECT_GE(fd, 0);
>   	EXPECT_EQ(filecmp(getpid(), pid, memfd, fd), 0);
>   
>   	/* Verify we can set an arbitrary remote fd with large size */
>   	memset(&big, 0x0, sizeof(big));
>   	big.addfd = addfd;
>   	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_BIG, &big);
> -	EXPECT_EQ(fd, 6);
> +	EXPECT_GE(fd, 0);
>   
>   	/* Verify we can set a specific remote fd */
>   	addfd.newfd = 42;
> 

Here is my Ack if Kees wants to take it through seccomp.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH] selftests/seccomp: Accept any valid fd in user_notification_addfd

[Kees Cook]

On Thu, Jan 28, 2021 at 10:17:21AM -0600, Seth Forshee wrote:
> This test expects fds to have specific values, which works fine
> when the test is run standalone. However, the kselftest runner
> consumes a couple of extra fds for redirection when running
> tests, so the test fails when run via kselftest.
> 
> Change the test to pass on any valid fd number.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Thanks!

Acked-by: Kees Cook <keescook@chromium.org>

I'll snag this if Shuah doesn't first. :)

-Kees

> ---
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 26c72f2b61b1..9338df6f4ca8 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -4019,18 +4019,14 @@ TEST(user_notification_addfd)
>  
>  	/* Verify we can set an arbitrary remote fd */
>  	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd);
> -	/*
> -	 * The child has fds 0(stdin), 1(stdout), 2(stderr), 3(memfd),
> -	 * 4(listener), so the newly allocated fd should be 5.
> -	 */
> -	EXPECT_EQ(fd, 5);
> +	EXPECT_GE(fd, 0);
>  	EXPECT_EQ(filecmp(getpid(), pid, memfd, fd), 0);
>  
>  	/* Verify we can set an arbitrary remote fd with large size */
>  	memset(&big, 0x0, sizeof(big));
>  	big.addfd = addfd;
>  	fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_BIG, &big);
> -	EXPECT_EQ(fd, 6);
> +	EXPECT_GE(fd, 0);
>  
>  	/* Verify we can set a specific remote fd */
>  	addfd.newfd = 42;
> -- 
> 2.29.2
> 

-- 
Kees Cook

Re: [PATCH] selftests/seccomp: Accept any valid fd in user_notification_addfd

[Shuah Khan]

On 2/9/21 5:33 PM, Kees Cook wrote:
> On Thu, Jan 28, 2021 at 10:17:21AM -0600, Seth Forshee wrote:
>> This test expects fds to have specific values, which works fine
>> when the test is run standalone. However, the kselftest runner
>> consumes a couple of extra fds for redirection when running
>> tests, so the test fails when run via kselftest.
>>
>> Change the test to pass on any valid fd number.
>>
>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> Thanks!
> 
> Acked-by: Kees Cook <keescook@chromium.org>
> 
> I'll snag this if Shuah doesn't first. :)
> 

I will apply this. I have several queued for 5.12-rc1 anyway.

thanks,
-- Shuah