[PATCH v2] unshare_test: set nr_open using soft limit

[PATCH v2] unshare_test: set nr_open using soft limit

[lufei]

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

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

diff --git a/tools/testing/selftests/core/unshare_test.c b/tools/testing/selftests/core/unshare_test.c
index 7fec9dfb1b0e..66b651168887 100644
--- a/tools/testing/selftests/core/unshare_test.c
+++ b/tools/testing/selftests/core/unshare_test.c
@@ -30,6 +30,7 @@ TEST(unshare_EMFILE)
 	static char buf[512], buf2[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 */
+		n2 = sprintf(buf2, "%d\n", rlimit_max);
 		lseek(fd, 0, SEEK_SET);
-		write(fd, buf, n);
+		write(fd, buf2, n2);
+
 		/* ... 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);
 	}
 
-- 
2.39.3

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

[Shuah Khan]

On 3/31/25 19:55, lufei wrote:
> Set maximum file descriptor number limit by rlimit.rlim_max than
> nr_open(hard limit). Hard limit may cause dup2 fail.
> 
> Signed-off-by: lufei <lufei@uniontech.com>

Petr, Al,

Okay to take this patch?

thanks,
-- Shuah

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

[Petr Vorel]

> On 3/31/25 19:55, lufei wrote:
> > Set maximum file descriptor number limit by rlimit.rlim_max than
> > nr_open(hard limit). Hard limit may cause dup2 fail.

> > Signed-off-by: lufei <lufei@uniontech.com>

> Petr, Al,

> Okay to take this patch?

LGTM, hopefully I haven't overlook anything.

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

FYI Cyril Hrubis suggested a different approach (for LTP tests which is based on
this Al's test):

https://lore.kernel.org/ltp/Z-u7yYJpTBG8Hi6A@yuki.lan/
https://lore.kernel.org/ltp/Z-vwYZxLms8juTjX@yuki.lan/

	filedescriptor that is >= 64 and set the nr_open to 64.

That would help to avoid using /proc/sys/fs/nr_open.

Kind regards,
Petr

> thanks,
> -- Shuah

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

[Shuah Khan]

On 4/8/25 23:56, Petr Vorel wrote:
>> On 3/31/25 19:55, lufei wrote:
>>> Set maximum file descriptor number limit by rlimit.rlim_max than
>>> nr_open(hard limit). Hard limit may cause dup2 fail.
> 
>>> Signed-off-by: lufei <lufei@uniontech.com>
> 
>> Petr, Al,
> 
>> Okay to take this patch?
> 
> LGTM, hopefully I haven't overlook anything.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> FYI Cyril Hrubis suggested a different approach (for LTP tests which is based on
> this Al's test):
> 
> https://lore.kernel.org/ltp/Z-u7yYJpTBG8Hi6A@yuki.lan/
> https://lore.kernel.org/ltp/Z-vwYZxLms8juTjX@yuki.lan/
> 
> 	filedescriptor that is >= 64 and set the nr_open to 64.
> 
> That would help to avoid using /proc/sys/fs/nr_open.
> 

Thank you for this suggestion.

lufei, Can you look into this as a solution? Also the short log
should include the subsystem: e.:

selftests:core: unshare_test: -----

thanks,
-- Shuah

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

[Lu Fei]

On Wed, Apr 09, 2025 at 03:30:32PM -0600, Shuah Khan wrote:
> On 4/8/25 23:56, Petr Vorel wrote:
> > > On 3/31/25 19:55, lufei wrote:
> > > > Set maximum file descriptor number limit by rlimit.rlim_max than
> > > > nr_open(hard limit). Hard limit may cause dup2 fail.
> > 
> > > > Signed-off-by: lufei <lufei@uniontech.com>
> > 
> > > Petr, Al,
> > 
> > > Okay to take this patch?
> > 
> > LGTM, hopefully I haven't overlook anything.
> > 
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > 
> > FYI Cyril Hrubis suggested a different approach (for LTP tests which is based on
> > this Al's test):
> > 
> > https://lore.kernel.org/ltp/Z-u7yYJpTBG8Hi6A@yuki.lan/
> > https://lore.kernel.org/ltp/Z-vwYZxLms8juTjX@yuki.lan/
> > 
> > 	filedescriptor that is >= 64 and set the nr_open to 64.
> > 
> > That would help to avoid using /proc/sys/fs/nr_open.
> > 
> 
> Thank you for this suggestion.
> 
> lufei, Can you look into this as a solution? Also the short log
> should include the subsystem: e.:
> 
> selftests:core: unshare_test: -----
> 
> thanks,
> -- Shuah
> 

Hi Shuah,

Thanks very much for reply.

I'll submit a v3 patch with dup a filedescriptor >= sizeof(long)*8 and
set nr_open to sizeof(long)*8 to trigger EMFILE.

Hi Petr, Shuah, I don't have a i686 environment, only on x86_64 has been
tested. Is this all right?

I'll re-write short log as asked in v3 patch.

[PATCH v3] selftests:core: unshare_test: using bits_per_long instead of nr_open+64.

[lufei]

dup2(2, nr_open+64) may cause ENOMEM error, change to dup2(2,
bits_per_long +1) and set nr_open to bits_per_long to avoid ENOMEM.

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

diff --git a/tools/testing/selftests/core/unshare_test.c b/tools/testing/selftests/core/unshare_test.c
index 7fec9dfb1b0e..ea4caa7ec28d 100644
--- a/tools/testing/selftests/core/unshare_test.c
+++ b/tools/testing/selftests/core/unshare_test.c
@@ -28,8 +28,8 @@ TEST(unshare_EMFILE)
 	int fd;
 	ssize_t n, n2;
 	static char buf[512], buf2[512];
-	struct rlimit rlimit;
 	int nr_open;
+	int bits_per_long = sizeof(long) * 8;
 
 	fd = open("/proc/sys/fs/nr_open", O_RDWR);
 	ASSERT_GE(fd, 0);
@@ -40,24 +40,8 @@ TEST(unshare_EMFILE)
 
 	ASSERT_EQ(sscanf(buf, "%d", &nr_open), 1);
 
-	ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlimit));
-
-	/* bump fs.nr_open */
-	n2 = sprintf(buf2, "%d\n", nr_open + 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;
-	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 >= bits_per_long */
+	EXPECT_GE(dup2(2, bits_per_long+1), 0) {
 		lseek(fd, 0, SEEK_SET);
 		write(fd, buf, n);
 		exit(EXIT_FAILURE);
@@ -74,15 +58,21 @@ TEST(unshare_EMFILE)
 	if (pid == 0) {
 		int err;
 
-		/* restore fs.nr_open */
+		/* set nr_open == bits_per_long */
+		n2 = sprintf(buf2, "%d\n", bits_per_long);
 		lseek(fd, 0, SEEK_SET);
-		write(fd, buf, n);
+		write(fd, buf2, n2);
+
 		/* ... 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);
 	}
 
-- 
2.39.3