| Message ID | 20240219134845.22171-1-wegao@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] tst_test.c: Set system default umaks to 0022 | expand |
Hi Wei, > Some system's default umask is 0077, this will trigger issues for LTP case. > Example 1: > cgroup_core01.c will report following error msg: > cgroup_core01.c:50: TBROK: openat(21</sys/fs/cgroup/memory/ltp/test-3519/child_b>, 'tasks', 2, 0): EACCES (13) > Detail reason: > Code: > cg_child_a = tst_cg_group_mk(tst_cg, "child_a"); > cg_child_b = tst_cg_group_mk(tst_cg, "child_b"); > Result: > wegao:/sys/fs/cgroup # ll ltp/test-25304/ > drwx------ 2 root root 0 Feb 10 10:37 child_a <<<<<< group and other has no any permission > drwx------ 2 root root 0 Feb 10 10:38 child_b > Code: > SAFE_CG_FCHOWN(cg_child_a, "cgroup.procs", nobody_uid, -1); > SAFE_CG_FCHOWN(cg_child_b, "cgroup.procs", nobody_uid, -1); > Result: > wegao:/sys/fs/cgroup # ll ltp/test-26388/child_a/ > total 0 > -rw-r--r-- 1 nobody root 0 Feb 10 10:44 cgroup.procs <<<<<< > Code: > SAFE_SETEUID(nobody_uid); <<<< after switch to nobody then can not access child_{a,b} or any file > under child_{a,b} > Result: > nobody@wegao:/sys/fs/cgroup/ltp/test-26730> ll child_a/ <<< nobody has no permission on child_a > ls: cannot open directory 'child_a/': Permission denied > Code: > loops = SAFE_CG_OPEN(cg_child_b, "cgroup.procs", O_RDWR, fds); > Result: > cgroup_core01.c:76: TBROK: openat(9</sys/fs/cgroup/ltp/test-26388/child_b>, 'cgroup.procs', 2, 0): EACCES (13) > Example 2: > chdir01 > chdir01.c:107: TFAIL: nobody: chdir("testfile") returned unexpected error: EACCES (13) > Reason: > susetest:/tmp/LTP_chd5JtblL/mntpoint # mount | grep loop0 > /dev/loop0 on /tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) > /dev/loop0 on /var/tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) > susetest:/tmp/LTP_chd5JtblL # ll > total 307216 > drwx------ 4 root root 16384 Dec 31 1969 mntpoint <<<< nobody has no permission on mount directory > -rw------- 1 root root 314572800 Feb 19 04:51 test_dev.img > susetest:/tmp/LTP_chd5JtblL/mntpoint # ll > total 16 > -rwx------ 1 root root 0 Feb 19 04:53 aa > drwx------ 2 root root 8192 Feb 19 04:50 keep_out > drwx------ 2 root root 8192 Feb 19 04:50 subdir > -rwx------ 1 root root 0 Feb 19 04:50 testfile <<<< nobody has no permission on testfile > Signed-off-by: Wei Gao <wegao@suse.com> > --- > lib/tst_test.c | 2 ++ > 1 file changed, 2 insertions(+) > diff --git a/lib/tst_test.c b/lib/tst_test.c > index edb42f7f4..e03fd93e7 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -1174,6 +1174,8 @@ static void do_setup(int argc, char *argv[]) > { > char *tdebug_env = getenv("LTP_ENABLE_DEBUG"); > + umask(0022); I don't think this is a good idea. Changing umask() in the library can influent tests. Could you please instead add umask(0) (to reset umask) on the tests which need it? The way it's done in open07.c. Kind regards, Petr
On Tue, Feb 20, 2024 at 10:18:37AM +0100, Petr Vorel wrote: > Hi Wei, > > > char *tdebug_env = getenv("LTP_ENABLE_DEBUG"); > > > + umask(0022); > > I don't think this is a good idea. Changing umask() in the library can influent > tests. Could you please instead add umask(0) (to reset umask) on the tests which > need it? The way it's done in open07.c. cgroup_core01.c and statx07.c can use umask(0) in tests and i have updated the patch. But chdir01 will still failed on vfat and exfat. Like following error msg: chdir01.c:100: TFAIL: nobody: chdir("subdir") returned unexpected value -1: EACCES (13) The reason is umask(0) should set before mount /dev/loop0 to /tmp/xxx/mntpoint, otherwise you have no chance modify permission of file. What's your suggestion? susetest:/tmp/LTP_chd5JtblL/mntpoint # mount | grep loop0 /dev/loop0 on /tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) /dev/loop0 on /var/tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) susetest:/tmp/LTP_chd5JtblL # chmod 777 mntpoint <<<< susetest:/tmp/LTP_chd5JtblL # ll total 307216 drwx------ 4 root root 16384 Dec 31 1969 mntpoint <<<<<<<< group and other has no any permission even try chmod -rw------- 1 root root 314572800 Feb 19 04:51 test_dev.img susetest:/tmp/LTP_chd5JtblL/mntpoint # touch aa susetest:/tmp/LTP_chd5JtblL/mntpoint # ll total 16 -rwx------ 1 root root 0a drwx------ 2 root root 8192 Feb 19 04:50 keep_out drwx------ 2 root root 8192 Feb 19 04:50 subdir -rwx------ 1 root root 0 Feb 19 04:50 testfile <<<<<< Thanks. > > Kind regards, > Petr
Hi all, > On Tue, Feb 20, 2024 at 10:18:37AM +0100, Petr Vorel wrote: > > Hi Wei, > > > char *tdebug_env = getenv("LTP_ENABLE_DEBUG"); > > > + umask(0022); > > I don't think this is a good idea. Changing umask() in the library can influent > > tests. Could you please instead add umask(0) (to reset umask) on the tests which > > need it? The way it's done in open07.c. First I thought that modifying umask (either with umask(0022) or just reset with umask(0)) is not a good idea, but now I'm not sure: 1) We have quite a few tests, which will needs it: NEW API tests: $ git grep '^\s*umask(0' $(git grep -l tst_test.h) | wc -l 15 All tests: $ git grep '^\s*umask(0' testcases/ | wc -l 34 And for sure some other, which will fail when too restrictive umask is set. Wouldn't be better to have it in the library? Hopefully fewer tests would be influenced this setup (and would need to set different umask()). > cgroup_core01.c and statx07.c can use umask(0) in tests and i have updated the patch. > But chdir01 will still failed on vfat and exfat. Like following error msg: > chdir01.c:100: TFAIL: nobody: chdir("subdir") returned unexpected value -1: EACCES (13) FYI test fails on vfat and ntfs when umask is 0077 (because override file and directory permissions) (other filesystems are ok). > The reason is umask(0) should set before mount /dev/loop0 to /tmp/xxx/mntpoint, otherwise you > have no chance modify permission of file. What's your suggestion? > susetest:/tmp/LTP_chd5JtblL/mntpoint # mount | grep loop0 > /dev/loop0 on /tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) > /dev/loop0 on /var/tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) 2) tests, which set .mount_device = 1 and have more restrictive umask will not work. Workaround would be to not use it and mount manually in the setup(). Or, reset umask with umask(0). WDYT? Kind regards, Petr > susetest:/tmp/LTP_chd5JtblL # chmod 777 mntpoint <<<< > susetest:/tmp/LTP_chd5JtblL # ll > total 307216 > drwx------ 4 root root 16384 Dec 31 1969 mntpoint <<<<<<<< group and other has no any permission even try chmod > -rw------- 1 root root 314572800 Feb 19 04:51 test_dev.img > susetest:/tmp/LTP_chd5JtblL/mntpoint # touch aa > susetest:/tmp/LTP_chd5JtblL/mntpoint # ll > total 16 > -rwx------ 1 root root 0a > drwx------ 2 root root 8192 Feb 19 04:50 keep_out > drwx------ 2 root root 8192 Feb 19 04:50 subdir > -rwx------ 1 root root 0 Feb 19 04:50 testfile <<<<<< > Thanks. > > Kind regards, > > Petr
On Wed, Feb 21, 2024 at 05:19:07PM +0100, Petr Vorel wrote: > Hi all, > > > On Tue, Feb 20, 2024 at 10:18:37AM +0100, Petr Vorel wrote: > > > Hi Wei, > > > > > char *tdebug_env = getenv("LTP_ENABLE_DEBUG"); > > > > > + umask(0022); > > > > I don't think this is a good idea. Changing umask() in the library can influent > > > tests. Could you please instead add umask(0) (to reset umask) on the tests which > > > need it? The way it's done in open07.c. > > First I thought that modifying umask (either with umask(0022) or just reset with > umask(0)) is not a good idea, but now I'm not sure: > > 1) We have quite a few tests, which will needs it: > NEW API tests: > $ git grep '^\s*umask(0' $(git grep -l tst_test.h) | wc -l > 15 > > All tests: > $ git grep '^\s*umask(0' testcases/ | wc -l > 34 > > And for sure some other, which will fail when too restrictive umask is set. > > Wouldn't be better to have it in the library? Hopefully fewer tests would be > influenced this setup (and would need to set different umask()). > > > cgroup_core01.c and statx07.c can use umask(0) in tests and i have updated the patch. > > > But chdir01 will still failed on vfat and exfat. Like following error msg: > > chdir01.c:100: TFAIL: nobody: chdir("subdir") returned unexpected value -1: EACCES (13) > > FYI test fails on vfat and ntfs when umask is 0077 (because override file and > directory permissions) (other filesystems are ok). > > > The reason is umask(0) should set before mount /dev/loop0 to /tmp/xxx/mntpoint, otherwise you > > have no chance modify permission of file. What's your suggestion? > > > susetest:/tmp/LTP_chd5JtblL/mntpoint # mount | grep loop0 > > /dev/loop0 on /tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) > > /dev/loop0 on /var/tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) > > 2) tests, which set .mount_device = 1 and have more restrictive umask will not > work. Workaround would be to not use it and mount manually in the setup(). > Or, reset umask with umask(0). > > WDYT? Currently i only need fix one case chdir01 so i will try to do workaround(mount mount manually in setup). If new fail happen in future then i suppose we need consider do umask in lib. Thanks for your comments. > > Kind regards, > Petr > > > susetest:/tmp/LTP_chd5JtblL # chmod 777 mntpoint <<<< > > susetest:/tmp/LTP_chd5JtblL # ll > > total 307216 > > drwx------ 4 root root 16384 Dec 31 1969 mntpoint <<<<<<<< group and other has no any permission even try chmod > > -rw------- 1 root root 314572800 Feb 19 04:51 test_dev.img > > > susetest:/tmp/LTP_chd5JtblL/mntpoint # touch aa > > susetest:/tmp/LTP_chd5JtblL/mntpoint # ll > > total 16 > > -rwx------ 1 root root 0a > > drwx------ 2 root root 8192 Feb 19 04:50 keep_out > > drwx------ 2 root root 8192 Feb 19 04:50 subdir > > -rwx------ 1 root root 0 Feb 19 04:50 testfile <<<<<< > > > Thanks. > > > > > Kind regards, > > > Petr
diff --git a/lib/tst_test.c b/lib/tst_test.c index edb42f7f4..e03fd93e7 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -1174,6 +1174,8 @@ static void do_setup(int argc, char *argv[]) { char *tdebug_env = getenv("LTP_ENABLE_DEBUG"); + umask(0022); + if (!tst_test) tst_brk(TBROK, "No tests to run");
Some system's default umask is 0077, this will trigger issues for LTP case. Example 1: cgroup_core01.c will report following error msg: cgroup_core01.c:50: TBROK: openat(21</sys/fs/cgroup/memory/ltp/test-3519/child_b>, 'tasks', 2, 0): EACCES (13) Detail reason: Code: cg_child_a = tst_cg_group_mk(tst_cg, "child_a"); cg_child_b = tst_cg_group_mk(tst_cg, "child_b"); Result: wegao:/sys/fs/cgroup # ll ltp/test-25304/ drwx------ 2 root root 0 Feb 10 10:37 child_a <<<<<< group and other has no any permission drwx------ 2 root root 0 Feb 10 10:38 child_b Code: SAFE_CG_FCHOWN(cg_child_a, "cgroup.procs", nobody_uid, -1); SAFE_CG_FCHOWN(cg_child_b, "cgroup.procs", nobody_uid, -1); Result: wegao:/sys/fs/cgroup # ll ltp/test-26388/child_a/ total 0 -rw-r--r-- 1 nobody root 0 Feb 10 10:44 cgroup.procs <<<<<< Code: SAFE_SETEUID(nobody_uid); <<<< after switch to nobody then can not access child_{a,b} or any file under child_{a,b} Result: nobody@wegao:/sys/fs/cgroup/ltp/test-26730> ll child_a/ <<< nobody has no permission on child_a ls: cannot open directory 'child_a/': Permission denied Code: loops = SAFE_CG_OPEN(cg_child_b, "cgroup.procs", O_RDWR, fds); Result: cgroup_core01.c:76: TBROK: openat(9</sys/fs/cgroup/ltp/test-26388/child_b>, 'cgroup.procs', 2, 0): EACCES (13) Example 2: chdir01 chdir01.c:107: TFAIL: nobody: chdir("testfile") returned unexpected error: EACCES (13) Reason: susetest:/tmp/LTP_chd5JtblL/mntpoint # mount | grep loop0 /dev/loop0 on /tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) /dev/loop0 on /var/tmp/LTP_chd5JtblL/mntpoint type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro) susetest:/tmp/LTP_chd5JtblL # ll total 307216 drwx------ 4 root root 16384 Dec 31 1969 mntpoint <<<< nobody has no permission on mount directory -rw------- 1 root root 314572800 Feb 19 04:51 test_dev.img susetest:/tmp/LTP_chd5JtblL/mntpoint # ll total 16 -rwx------ 1 root root 0 Feb 19 04:53 aa drwx------ 2 root root 8192 Feb 19 04:50 keep_out drwx------ 2 root root 8192 Feb 19 04:50 subdir -rwx------ 1 root root 0 Feb 19 04:50 testfile <<<< nobody has no permission on testfile Signed-off-by: Wei Gao <wegao@suse.com> --- lib/tst_test.c | 2 ++ 1 file changed, 2 insertions(+)