diff mbox series

[v2] unshare_test: set nr_open using soft limit

Message ID 20250401015557.113872-1-lufei@uniontech.com
State New
Headers show
Series [v2] unshare_test: set nr_open using soft limit | expand

Commit Message

Lu Fei April 1, 2025, 1:55 a.m. UTC
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(-)

Comments

Shuah Khan April 8, 2025, 11:42 p.m. UTC | #1
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
Petr Vorel April 9, 2025, 5:56 a.m. UTC | #2
> 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
Shuah Khan April 9, 2025, 9:30 p.m. UTC | #3
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
Lu Fei April 10, 2025, 2:05 a.m. UTC | #4
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.
diff mbox series

Patch

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);
 	}