| Message ID | 20240809025825.4055-1-ice_yangxiao@163.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | syscalls/{fanotify17, getxattr05}: simplify code by using save_restore | expand |
Hi! > - /* > - * The default value of max_user_namespaces is set to 0 on some distros, > - * We need to change the default value to call unshare(). > - */ > - if (access(SELF_USERNS, F_OK) != 0) { > + if (access(SELF_USERNS, F_OK) != 0) > user_ns_supported = 0; > - } else if (!access(MAX_USERNS, F_OK)) { > - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); > - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); Here the original code writes 10 to the MAX_USERNS. > - } > > /* > * In older kernels those limits were fixed in kernel and fanotify is > @@ -244,21 +234,18 @@ static void setup(void) > setup_rlimit(max_groups * 2); > } > > -static void cleanup(void) > -{ > - if (orig_max_userns != -1) > - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); > -} > - > static struct tst_test test = { > .test = test_fanotify, > .tcnt = ARRAY_SIZE(tcases), > .setup = setup, > - .cleanup = cleanup, > .needs_root = 1, > .forks_child = 1, > .mount_device = 1, > .mntpoint = MOUNT_PATH, > + .save_restore = (const struct tst_path_val[]) { > + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, > + {} > + }, And here you are initializing it to 1024? Shouldn't the value here be 10 too? > }; > #else > TST_TEST_TCONF("system doesn't have required fanotify support"); > diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c > index d9717a695..3dff8e27f 100644 > --- a/testcases/kernel/syscalls/getxattr/getxattr05.c > +++ b/testcases/kernel/syscalls/getxattr/getxattr05.c > @@ -40,11 +40,9 @@ > > #define TEST_FILE "testfile" > #define SELF_USERNS "/proc/self/ns/user" > -#define MAX_USERNS "/proc/sys/user/max_user_namespaces" > #define UID_MAP "/proc/self/uid_map" > > static acl_t acl; > -static int orig_max_userns = -1; > static int user_ns_supported = 1; > > static struct tcase { > @@ -149,23 +147,13 @@ static void setup(void) > tst_brk(TBROK | TERRNO, "acl_set_file(%s) failed", TEST_FILE); > } > > - /* The default value of max_user_namespaces is set to 0 on some distros, > - * We need to change the default value to call unshare(). > - */ > - if (access(SELF_USERNS, F_OK) != 0) { > + if (access(SELF_USERNS, F_OK) != 0) > user_ns_supported = 0; > - } else if (!access(MAX_USERNS, F_OK)) { > - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); > - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); > - } > > } > > static void cleanup(void) > { > - if (orig_max_userns != -1) > - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); > - > if (acl) > acl_free(acl); > } > @@ -181,7 +169,11 @@ static struct tst_test test = { > .tags = (const struct tst_tag[]) { > {"linux-git", "82c9a927bc5d"}, > {} > -}, > + }, > + .save_restore = (const struct tst_path_val[]) { > + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, And same here. > + {} > + }, > }; > > #else /* HAVE_SYS_XATTR_H && HAVE_LIBACL*/ > -- > 2.45.2 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
At 2024-09-17 19:10:39, "Cyril Hrubis" <chrubis@suse.cz> wrote: >Hi! >> - /* >> - * The default value of max_user_namespaces is set to 0 on some distros, >> - * We need to change the default value to call unshare(). >> - */ >> - if (access(SELF_USERNS, F_OK) != 0) { >> + if (access(SELF_USERNS, F_OK) != 0) >> user_ns_supported = 0; >> - } else if (!access(MAX_USERNS, F_OK)) { >> - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); >> - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); > >Here the original code writes 10 to the MAX_USERNS. > >> - } >> >> /* >> * In older kernels those limits were fixed in kernel and fanotify is >> @@ -244,21 +234,18 @@ static void setup(void) >> setup_rlimit(max_groups * 2); >> } >> >> -static void cleanup(void) >> -{ >> - if (orig_max_userns != -1) >> - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); >> -} >> - >> static struct tst_test test = { >> .test = test_fanotify, >> .tcnt = ARRAY_SIZE(tcases), >> .setup = setup, >> - .cleanup = cleanup, >> .needs_root = 1, >> .forks_child = 1, >> .mount_device = 1, >> .mntpoint = MOUNT_PATH, >> + .save_restore = (const struct tst_path_val[]) { >> + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, >> + {} >> + }, > >And here you are initializing it to 1024? Shouldn't the value here be 10 >too? Hi Cyril, Sorry for the late reply. If more than 10 user namspaces is currently used on system, it is still successfully to set max_user_namespaces to 10. However, these tests fail with ENOSPC. for example: # lsns -t user -l -n | wc -l 17 root@fedora40:~/ltp/testcases/kernel/syscalls/fanotify# ./fanotify17 ... fanotify17.c:174: TINFO: Test #0: Global groups limit in init user ns fanotify17.c:130: TPASS: Created 128 groups - below groups limit (128) fanotify17.c:174: TINFO: Test #1: Global groups limit in privileged user ns fanotify17.c:154: TFAIL: unshare(CLONE_NEWUSER) failed: ENOSPC (28) tst_test.c:452: TBROK: Invalid child (6958) exit value 1 Just try to fix the issue by increasing the setting of max_user_namespaces. BTW, it may be better to get the number of user namespaces in use and then add 10. Best Regards, Xiao Yang > >> }; >> #else >> TST_TEST_TCONF("system doesn't have required fanotify support"); >> diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c >> index d9717a695..3dff8e27f 100644 >> --- a/testcases/kernel/syscalls/getxattr/getxattr05.c >> +++ b/testcases/kernel/syscalls/getxattr/getxattr05.c >> @@ -40,11 +40,9 @@ >> >> #define TEST_FILE "testfile" >> #define SELF_USERNS "/proc/self/ns/user" >> -#define MAX_USERNS "/proc/sys/user/max_user_namespaces" >> #define UID_MAP "/proc/self/uid_map" >> >> static acl_t acl; >> -static int orig_max_userns = -1; >> static int user_ns_supported = 1; >> >> static struct tcase { >> @@ -149,23 +147,13 @@ static void setup(void) >> tst_brk(TBROK | TERRNO, "acl_set_file(%s) failed", TEST_FILE); >> } >> >> - /* The default value of max_user_namespaces is set to 0 on some distros, >> - * We need to change the default value to call unshare(). >> - */ >> - if (access(SELF_USERNS, F_OK) != 0) { >> + if (access(SELF_USERNS, F_OK) != 0) >> user_ns_supported = 0; >> - } else if (!access(MAX_USERNS, F_OK)) { >> - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); >> - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); >> - } >> >> } >> >> static void cleanup(void) >> { >> - if (orig_max_userns != -1) >> - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); >> - >> if (acl) >> acl_free(acl); >> } >> @@ -181,7 +169,11 @@ static struct tst_test test = { >> .tags = (const struct tst_tag[]) { >> {"linux-git", "82c9a927bc5d"}, >> {} >> -}, >> + }, >> + .save_restore = (const struct tst_path_val[]) { >> + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, > >And same here. > >> + {} >> + }, >> }; >> >> #else /* HAVE_SYS_XATTR_H && HAVE_LIBACL*/ >> -- >> 2.45.2 >> >> >> -- >> Mailing list info: https://lists.linux.it/listinfo/ltp > >-- >Cyril Hrubis >chrubis@suse.cz
Hi! > If more than 10 user namspaces is currently used on system, it is still successfully to set > max_user_namespaces to 10. However, these tests fail with ENOSPC. for example: > # lsns -t user -l -n | wc -l > 17 > > root@fedora40:~/ltp/testcases/kernel/syscalls/fanotify# ./fanotify17 > ... > fanotify17.c:174: TINFO: Test #0: Global groups limit in init user ns > fanotify17.c:130: TPASS: Created 128 groups - below groups limit (128) > fanotify17.c:174: TINFO: Test #1: Global groups limit in privileged user ns > fanotify17.c:154: TFAIL: unshare(CLONE_NEWUSER) failed: ENOSPC (28) > tst_test.c:452: TBROK: Invalid child (6958) exit value 1 > > > > Just try to fix the issue by increasing the setting of max_user_namespaces. > BTW, it may be better to get the number of user namespaces in use and then add 10. Such change should be ideally in a separate patch with this explanation and not hidden in a patch that is supposedly just moving the code that restores the original value.
diff --git a/testcases/kernel/syscalls/fanotify/fanotify17.c b/testcases/kernel/syscalls/fanotify/fanotify17.c index 3ecb31b6e..f432dff36 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify17.c +++ b/testcases/kernel/syscalls/fanotify/fanotify17.c @@ -31,7 +31,6 @@ #define MOUNT_PATH "fs_mnt" #define TEST_FILE MOUNT_PATH "/testfile" #define SELF_USERNS "/proc/self/ns/user" -#define MAX_USERNS "/proc/sys/user/max_user_namespaces" #define UID_MAP "/proc/self/uid_map" #define GLOBAL_MAX_GROUPS "/proc/sys/fs/fanotify/max_user_groups" @@ -47,7 +46,6 @@ #define DEFAULT_MAX_GROUPS 129 #define DEFAULT_MAX_MARKS 8192 -static int orig_max_userns = -1; static int user_ns_supported = 1; static int max_groups = DEFAULT_MAX_GROUPS; static int max_marks = DEFAULT_MAX_MARKS; @@ -216,16 +214,8 @@ static void setup(void) /* Check for kernel fanotify support */ REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, TEST_FILE); - /* - * The default value of max_user_namespaces is set to 0 on some distros, - * We need to change the default value to call unshare(). - */ - if (access(SELF_USERNS, F_OK) != 0) { + if (access(SELF_USERNS, F_OK) != 0) user_ns_supported = 0; - } else if (!access(MAX_USERNS, F_OK)) { - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); - } /* * In older kernels those limits were fixed in kernel and fanotify is @@ -244,21 +234,18 @@ static void setup(void) setup_rlimit(max_groups * 2); } -static void cleanup(void) -{ - if (orig_max_userns != -1) - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); -} - static struct tst_test test = { .test = test_fanotify, .tcnt = ARRAY_SIZE(tcases), .setup = setup, - .cleanup = cleanup, .needs_root = 1, .forks_child = 1, .mount_device = 1, .mntpoint = MOUNT_PATH, + .save_restore = (const struct tst_path_val[]) { + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, + {} + }, }; #else TST_TEST_TCONF("system doesn't have required fanotify support"); diff --git a/testcases/kernel/syscalls/getxattr/getxattr05.c b/testcases/kernel/syscalls/getxattr/getxattr05.c index d9717a695..3dff8e27f 100644 --- a/testcases/kernel/syscalls/getxattr/getxattr05.c +++ b/testcases/kernel/syscalls/getxattr/getxattr05.c @@ -40,11 +40,9 @@ #define TEST_FILE "testfile" #define SELF_USERNS "/proc/self/ns/user" -#define MAX_USERNS "/proc/sys/user/max_user_namespaces" #define UID_MAP "/proc/self/uid_map" static acl_t acl; -static int orig_max_userns = -1; static int user_ns_supported = 1; static struct tcase { @@ -149,23 +147,13 @@ static void setup(void) tst_brk(TBROK | TERRNO, "acl_set_file(%s) failed", TEST_FILE); } - /* The default value of max_user_namespaces is set to 0 on some distros, - * We need to change the default value to call unshare(). - */ - if (access(SELF_USERNS, F_OK) != 0) { + if (access(SELF_USERNS, F_OK) != 0) user_ns_supported = 0; - } else if (!access(MAX_USERNS, F_OK)) { - SAFE_FILE_SCANF(MAX_USERNS, "%d", &orig_max_userns); - SAFE_FILE_PRINTF(MAX_USERNS, "%d", 10); - } } static void cleanup(void) { - if (orig_max_userns != -1) - SAFE_FILE_PRINTF(MAX_USERNS, "%d", orig_max_userns); - if (acl) acl_free(acl); } @@ -181,7 +169,11 @@ static struct tst_test test = { .tags = (const struct tst_tag[]) { {"linux-git", "82c9a927bc5d"}, {} -}, + }, + .save_restore = (const struct tst_path_val[]) { + {"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP}, + {} + }, }; #else /* HAVE_SYS_XATTR_H && HAVE_LIBACL*/
Signed-off-by: Xiao Yang <ice_yangxiao@163.com> --- .../kernel/syscalls/fanotify/fanotify17.c | 23 ++++--------------- .../kernel/syscalls/getxattr/getxattr05.c | 20 +++++----------- 2 files changed, 11 insertions(+), 32 deletions(-)