| Message ID | 1659497063-2212-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] syscalls/creat09: Add umask test condition | expand |
On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[1]. > > I will add acl and umask test[2] in xfstests because there is more suitable > to do this. > > Here I just only add umask test condition simply. > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> Fyi, I have this Thursday and Friday off which is why I haven't sent the pull request for setgid changes to Linus yet. I only sent the ones that I thought were less likely to cause regressions. I don't want to send a PR and then not be around to respond to issues. So I will send the setgid PR on Tuesday or Wednesday next week. Just a heads up! Christian
on 2022/08/03 15:49, Christian Brauner wrote: > On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: >> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by >> this case has been merged into linux-next branch[1]. >> >> I will add acl and umask test[2] in xfstests because there is more suitable >> to do this. >> >> Here I just only add umask test condition simply. >> >> [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce >> [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 >> >> Reviewed-by: Petr Vorel <pvorel@suse.cz> >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > > Fyi, I have this Thursday and Friday off which is why I haven't sent the > pull request for setgid changes to Linus yet. I only sent the ones that > I thought were less likely to cause regressions. I don't want to send a > PR and then not be around to respond to issues. So I will send the > setgid PR on Tuesday or Wednesday next week. Just a heads up! Glad to know this. Thanks! Best Regards Yang Xu > > Christian
Hi, On 03. 08. 22 5:24, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[1]. > > I will add acl and umask test[2] in xfstests because there is more suitable > to do this. > > Here I just only add umask test condition simply. > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > v1-v2: > 1.update linux-next and xfstests url > 2.use Ternary Operator instead of switch or if > testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..7077cbcff 100644 > --- a/testcases/kernel/syscalls/creat/creat09.c > +++ b/testcases/kernel/syscalls/creat/creat09.c > @@ -28,6 +28,16 @@ > * Date: Fri Jan 22 16:48:18 2021 -0800 > * > * xfs: fix up non-directory creation in SGID directories > + * > + * When use acl or umask, it still has bug. > + * > + * Fixed in: > + * > + * commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > + * Date: Thu July 14 14:11:27 2022 +0800 > + * > + * fs: move S_ISGID stripping into the vfs_*() helpers > */ > > #include <stdlib.h> > @@ -94,8 +104,11 @@ static void file_test(const char *name) > tst_res(TPASS, "%s: Setgid bit not set", name); > } > > -static void run(void) > +static void run(unsigned int n) > { > + umask(n ? S_IXGRP : 0); > + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); It'd be much cleaner to use an array of testcase structures and then call: tst_res(TINFO, testcase_list[n].name); umask(testcase_list[n].mask); ... .tcnt = ARRAY_SIZE(testcase_list), See also creat04 for example. > + > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +128,14 @@ static void cleanup(void) > } > > static struct tst_test test = { > - .test_all = run, > + .test = run, > .setup = setup, > .cleanup = cleanup, > .needs_root = 1, > .all_filesystems = 1, > .mount_device = 1, > .mntpoint = MNTPOINT, > + .tcnt = 2, > .skip_filesystems = (const char*[]) { > "exfat", > "ntfs", > @@ -132,6 +146,7 @@ static struct tst_test test = { > {"linux-git", "0fa3ecd87848"}, > {"CVE", "2018-13405"}, > {"linux-git", "01ea173e103e"}, > + {"linux-git", "1639a49ccdce"}, > {} > }, > };
Hi Martin Thanks for your review. I have a holiday next week, so will send v2 when I come back. Best Regards Yang Xu > Hi, > > On 03. 08. 22 5:24, Yang Xu wrote: >> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by >> this case has been merged into linux-next branch[1]. >> >> I will add acl and umask test[2] in xfstests because there is more suitable >> to do this. >> >> Here I just only add umask test condition simply. >> >> [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce >> [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 >> >> Reviewed-by: Petr Vorel <pvorel@suse.cz> >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> >> --- >> v1-v2: >> 1.update linux-next and xfstests url >> 2.use Ternary Operator instead of switch or if >> testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c >> index bed7bddb0..7077cbcff 100644 >> --- a/testcases/kernel/syscalls/creat/creat09.c >> +++ b/testcases/kernel/syscalls/creat/creat09.c >> @@ -28,6 +28,16 @@ >> * Date: Fri Jan 22 16:48:18 2021 -0800 >> * >> * xfs: fix up non-directory creation in SGID directories >> + * >> + * When use acl or umask, it still has bug. >> + * >> + * Fixed in: >> + * >> + * commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b >> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> >> + * Date: Thu July 14 14:11:27 2022 +0800 >> + * >> + * fs: move S_ISGID stripping into the vfs_*() helpers >> */ >> >> #include <stdlib.h> >> @@ -94,8 +104,11 @@ static void file_test(const char *name) >> tst_res(TPASS, "%s: Setgid bit not set", name); >> } >> >> -static void run(void) >> +static void run(unsigned int n) >> { >> + umask(n ? S_IXGRP : 0); >> + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); > > It'd be much cleaner to use an array of testcase structures and then call: > tst_res(TINFO, testcase_list[n].name); > umask(testcase_list[n].mask); > > ... > > .tcnt = ARRAY_SIZE(testcase_list), > > See also creat04 for example. > >> + >> fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); >> SAFE_CLOSE(fd); >> file_test(CREAT_FILE); >> @@ -115,13 +128,14 @@ static void cleanup(void) >> } >> >> static struct tst_test test = { >> - .test_all = run, >> + .test = run, >> .setup = setup, >> .cleanup = cleanup, >> .needs_root = 1, >> .all_filesystems = 1, >> .mount_device = 1, >> .mntpoint = MNTPOINT, >> + .tcnt = 2, >> .skip_filesystems = (const char*[]) { >> "exfat", >> "ntfs", >> @@ -132,6 +146,7 @@ static struct tst_test test = { >> {"linux-git", "0fa3ecd87848"}, >> {"CVE", "2018-13405"}, >> {"linux-git", "01ea173e103e"}, >> + {"linux-git", "1639a49ccdce"}, >> {} >> }, >> }; > >
On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[1]. > > I will add acl and umask test[2] in xfstests because there is more suitable > to do this. > > Here I just only add umask test condition simply. > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- Looks good to me, Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org> > v1-v2: > 1.update linux-next and xfstests url > 2.use Ternary Operator instead of switch or if > testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..7077cbcff 100644 > --- a/testcases/kernel/syscalls/creat/creat09.c > +++ b/testcases/kernel/syscalls/creat/creat09.c > @@ -28,6 +28,16 @@ > * Date: Fri Jan 22 16:48:18 2021 -0800 > * > * xfs: fix up non-directory creation in SGID directories > + * > + * When use acl or umask, it still has bug. > + * > + * Fixed in: > + * > + * commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > + * Date: Thu July 14 14:11:27 2022 +0800 > + * > + * fs: move S_ISGID stripping into the vfs_*() helpers > */ > > #include <stdlib.h> > @@ -94,8 +104,11 @@ static void file_test(const char *name) > tst_res(TPASS, "%s: Setgid bit not set", name); > } > > -static void run(void) > +static void run(unsigned int n) > { > + umask(n ? S_IXGRP : 0); > + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); > + > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +128,14 @@ static void cleanup(void) > } > > static struct tst_test test = { > - .test_all = run, > + .test = run, > .setup = setup, > .cleanup = cleanup, > .needs_root = 1, > .all_filesystems = 1, > .mount_device = 1, > .mntpoint = MNTPOINT, > + .tcnt = 2, > .skip_filesystems = (const char*[]) { > "exfat", > "ntfs", > @@ -132,6 +146,7 @@ static struct tst_test test = { > {"linux-git", "0fa3ecd87848"}, > {"CVE", "2018-13405"}, > {"linux-git", "01ea173e103e"}, > + {"linux-git", "1639a49ccdce"}, > {} > }, > }; > -- > 2.23.0 >
diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c index bed7bddb0..7077cbcff 100644 --- a/testcases/kernel/syscalls/creat/creat09.c +++ b/testcases/kernel/syscalls/creat/creat09.c @@ -28,6 +28,16 @@ * Date: Fri Jan 22 16:48:18 2021 -0800 * * xfs: fix up non-directory creation in SGID directories + * + * When use acl or umask, it still has bug. + * + * Fixed in: + * + * commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> + * Date: Thu July 14 14:11:27 2022 +0800 + * + * fs: move S_ISGID stripping into the vfs_*() helpers */ #include <stdlib.h> @@ -94,8 +104,11 @@ static void file_test(const char *name) tst_res(TPASS, "%s: Setgid bit not set", name); } -static void run(void) +static void run(unsigned int n) { + umask(n ? S_IXGRP : 0); + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); + fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); SAFE_CLOSE(fd); file_test(CREAT_FILE); @@ -115,13 +128,14 @@ static void cleanup(void) } static struct tst_test test = { - .test_all = run, + .test = run, .setup = setup, .cleanup = cleanup, .needs_root = 1, .all_filesystems = 1, .mount_device = 1, .mntpoint = MNTPOINT, + .tcnt = 2, .skip_filesystems = (const char*[]) { "exfat", "ntfs", @@ -132,6 +146,7 @@ static struct tst_test test = { {"linux-git", "0fa3ecd87848"}, {"CVE", "2018-13405"}, {"linux-git", "01ea173e103e"}, + {"linux-git", "1639a49ccdce"}, {} }, };