Re: [PATCH] unshare_test: set nr_open using soft limit

[Petr Vorel <pvorel@suse.cz>]

Hi all,

> Set maximum file descriptor number limit by rlimit.rlim_max than
> nr_open(hard limit). Hard limit may cause dup2 fail.

It actually changes from failure:

# ./unshare_test 
TAP version 13
1..1
# Starting 1 tests from 1 test cases.
#  RUN           global.unshare_EMFILE ...
# unshare_test.c:60:unshare_EMFILE:Expected dup2(2, nr_open + 64) (-1) >= 0 (0)
# unshare_EMFILE: Test failed
#          FAIL  global.unshare_EMFILE
not ok 1 global.unshare_EMFILE
# FAILED: 0 / 1 tests passed.
# Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0

to pass:

TAP version 13
1..1
# Starting 1 tests from 1 test cases.
#  RUN           global.unshare_EMFILE ...
#            OK  global.unshare_EMFILE
ok 1 global.unshare_EMFILE
# PASSED: 1 / 1 tests passed.
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

> Signed-off-by: lufei <lufei@uniontech.com>
> ---
>  tools/testing/selftests/core/unshare_test.c | 28 +++++++++++++--------
>  1 file changed, 18 insertions(+), 10 deletions(-)

> diff --git a/tools/testing/selftests/core/unshare_test.c b/tools/testing/selftests/core/unshare_test.c
> index 7fec9dfb1b0e..2c4e7104b0d9 100644
> --- a/tools/testing/selftests/core/unshare_test.c
> +++ b/tools/testing/selftests/core/unshare_test.c
> @@ -26,10 +26,11 @@ TEST(unshare_EMFILE)
>  		.exit_signal = SIGCHLD,
>  	};
>  	int fd;
> -	ssize_t n, n2;
> -	static char buf[512], buf2[512];
> +	ssize_t n, n2, n3;
> +	static char buf[512], buf2[512], buf3[512];
>  	struct rlimit rlimit;
>  	int nr_open;
> +	int rlimit_max;

>  	fd = open("/proc/sys/fs/nr_open", O_RDWR);
>  	ASSERT_GE(fd, 0);
> @@ -42,22 +43,24 @@ TEST(unshare_EMFILE)

>  	ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlimit));

> -	/* bump fs.nr_open */
> -	n2 = sprintf(buf2, "%d\n", nr_open + 1024);
> +	rlimit_max = rlimit.rlim_max;
> +
> +	/* bump rlimit.rlim_max */
> +	n2 = sprintf(buf2, "%d\n", rlimit_max + 1024);
>  	lseek(fd, 0, SEEK_SET);
>  	write(fd, buf2, n2);

>  	/* bump ulimit -n */
> -	rlimit.rlim_cur = nr_open + 1024;
> -	rlimit.rlim_max = nr_open + 1024;
> +	rlimit.rlim_cur = rlimit_max + 1024;
> +	rlimit.rlim_max = rlimit_max + 1024;
>  	EXPECT_EQ(0, setrlimit(RLIMIT_NOFILE, &rlimit)) {
>  		lseek(fd, 0, SEEK_SET);
>  		write(fd, buf, n);
>  		exit(EXIT_FAILURE);
>  	}

> -	/* get a descriptor past the old fs.nr_open */
> -	EXPECT_GE(dup2(2, nr_open + 64), 0) {
> +	/* get a descriptor past the old rlimit.rlim_max */
> +	EXPECT_GE(dup2(2, rlimit_max + 64), 0) {
>  		lseek(fd, 0, SEEK_SET);
>  		write(fd, buf, n);
>  		exit(EXIT_FAILURE);
> @@ -74,15 +77,20 @@ TEST(unshare_EMFILE)
>  	if (pid == 0) {
>  		int err;

> -		/* restore fs.nr_open */
> +		n3 = sprintf(buf3, "%d\n", rlimit_max);
>  		lseek(fd, 0, SEEK_SET);
> -		write(fd, buf, n);
> +		write(fd, buf3, n3);
nit: do we really need buf3? Can't we reuse buf2?
Or do I miss something obvious?

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +
>  		/* ... and now unshare(CLONE_FILES) must fail with EMFILE */
>  		err = unshare(CLONE_FILES);
>  		EXPECT_EQ(err, -1)
>  			exit(EXIT_FAILURE);
>  		EXPECT_EQ(errno, EMFILE)
>  			exit(EXIT_FAILURE);
> +
> +		/* restore fs.nr_open */
> +		lseek(fd, 0, SEEK_SET);
> +		write(fd, buf, n);
>  		exit(EXIT_SUCCESS);
>  	}