| Message ID | 1663143142-2283-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4,1/3] syscalls/creat09: Add umask test condition | expand |
Hi All > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into 6.0-rc1 kernel[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/torvalds/linux.git/commit/?id=1639a49c > [2]https://patchwork.kernel.org/project/fstests/list/?series=662984 I Should update the [2] url because the corresponding patch is in xfstest for-next branch. [2]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?h=for-next&id=c875434286737e792d200cda2eb679e2a8441837 Also miss Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org> tag. Best Regards Yang Xu > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > testcases/kernel/syscalls/creat/creat09.c | 30 +++++++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..d583cceca 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> > @@ -47,6 +57,14 @@ > static gid_t free_gid; > static int fd = -1; > > +static struct tcase { > + const char *msg; > + int mask; > +} tcases[] = { > + {"under umask(0) situation", 0}, > + {"under umask(S_IXGRP) situation", S_IXGRP} > +}; > + > static void setup(void) > { > struct stat buf; > @@ -94,8 +112,14 @@ 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) > { > + struct tcase *tc = &tcases[n]; > + > + umask(tc->mask); > + tst_res(TINFO, "Testing setgid behaviour when creating file %s", > + tc->msg); > + > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +139,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 = ARRAY_SIZE(tcases), > .skip_filesystems = (const char*[]) { > "exfat", > "ntfs", > @@ -132,6 +157,7 @@ static struct tst_test test = { > {"linux-git", "0fa3ecd87848"}, > {"CVE", "2018-13405"}, > {"linux-git", "01ea173e103e"}, > + {"linux-git", "1639a49ccdce"}, > {} > }, > };
Hi! > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..d583cceca 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> > @@ -47,6 +57,14 @@ > static gid_t free_gid; > static int fd = -1; > > +static struct tcase { > + const char *msg; > + int mask; > +} tcases[] = { > + {"under umask(0) situation", 0}, > + {"under umask(S_IXGRP) situation", S_IXGRP} > +}; > + > static void setup(void) > { > struct stat buf; > @@ -94,8 +112,14 @@ 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) > { > + struct tcase *tc = &tcases[n]; > + > + umask(tc->mask); > + tst_res(TINFO, "Testing setgid behaviour when creating file %s", > + tc->msg); This can be shorter and more to the point, something as: tst_res(TINFO, "File created with %s", tc->msg); And the msg could be just "umask(0)" and "umask(S_IXGRP)". Otherwise it's fine and I can fix the messages before applying if you want. > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +139,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 = ARRAY_SIZE(tcases), > .skip_filesystems = (const char*[]) { > "exfat", > "ntfs", > @@ -132,6 +157,7 @@ static struct tst_test test = { > {"linux-git", "0fa3ecd87848"}, > {"CVE", "2018-13405"}, > {"linux-git", "01ea173e103e"}, > + {"linux-git", "1639a49ccdce"}, > {} > }, > }; > -- > 2.23.0 >
Hi Cyril > Hi! >> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c >> index bed7bddb0..d583cceca 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> >> @@ -47,6 +57,14 @@ >> static gid_t free_gid; >> static int fd = -1; >> >> +static struct tcase { >> + const char *msg; >> + int mask; >> +} tcases[] = { >> + {"under umask(0) situation", 0}, >> + {"under umask(S_IXGRP) situation", S_IXGRP} >> +}; >> + >> static void setup(void) >> { >> struct stat buf; >> @@ -94,8 +112,14 @@ 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) >> { >> + struct tcase *tc = &tcases[n]; >> + >> + umask(tc->mask); >> + tst_res(TINFO, "Testing setgid behaviour when creating file %s", >> + tc->msg); > > This can be shorter and more to the point, something as: > > tst_res(TINFO, "File created with %s", tc->msg); > > And the msg could be just "umask(0)" and "umask(S_IXGRP)". > > > Otherwise it's fine and I can fix the messages before applying if you > want. Yes. Please apply it with fix the messages. Thanks. Best Regards Yang Xu > >> fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); >> SAFE_CLOSE(fd); >> file_test(CREAT_FILE); >> @@ -115,13 +139,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 = ARRAY_SIZE(tcases), >> .skip_filesystems = (const char*[]) { >> "exfat", >> "ntfs", >> @@ -132,6 +157,7 @@ static struct tst_test test = { >> {"linux-git", "0fa3ecd87848"}, >> {"CVE", "2018-13405"}, >> {"linux-git", "01ea173e103e"}, >> + {"linux-git", "1639a49ccdce"}, >> {} >> }, >> }; >> -- >> 2.23.0 >> >
Hi Cyril > Hi Cyril > >> Hi! >>> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c >>> index bed7bddb0..d583cceca 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> >>> @@ -47,6 +57,14 @@ >>> static gid_t free_gid; >>> static int fd = -1; >>> >>> +static struct tcase { >>> + const char *msg; >>> + int mask; >>> +} tcases[] = { >>> + {"under umask(0) situation", 0}, >>> + {"under umask(S_IXGRP) situation", S_IXGRP} >>> +}; >>> + >>> static void setup(void) >>> { >>> struct stat buf; >>> @@ -94,8 +112,14 @@ 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) >>> { >>> + struct tcase *tc = &tcases[n]; >>> + >>> + umask(tc->mask); >>> + tst_res(TINFO, "Testing setgid behaviour when creating file %s", >>> + tc->msg); >> >> This can be shorter and more to the point, something as: >> >> tst_res(TINFO, "File created with %s", tc->msg); >> >> And the msg could be just "umask(0)" and "umask(S_IXGRP)". >> >> >> Otherwise it's fine and I can fix the messages before applying if you >> want. > > Yes. Please apply it with fix the messages. Thanks. I guess you miss this patchset. so ping again. Best Regards Yang Xu > > > Best Regards > Yang Xu >> >>> fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); >>> SAFE_CLOSE(fd); >>> file_test(CREAT_FILE); >>> @@ -115,13 +139,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 = ARRAY_SIZE(tcases), >>> .skip_filesystems = (const char*[]) { >>> "exfat", >>> "ntfs", >>> @@ -132,6 +157,7 @@ static struct tst_test test = { >>> {"linux-git", "0fa3ecd87848"}, >>> {"CVE", "2018-13405"}, >>> {"linux-git", "01ea173e103e"}, >>> + {"linux-git", "1639a49ccdce"}, >>> {} >>> }, >>> }; >>> -- >>> 2.23.0 >>> >> >
Hi! > > Yes. Please apply it with fix the messages. Thanks. > > I guess you miss this patchset. so ping again. Sorry for the delay. I just did quick test and I got failures on XFS on kernel 5.19, wasn't this supposed to be fixed in 5.19? ... creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group creat09.c:112: TPASS: mntpoint/testdir/creat.tmp: Setgid bit not set creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group creat09.c:112: TPASS: mntpoint/testdir/open.tmp: Setgid bit not set creat09.c:120: TINFO: File created with umask(S_IXGRP) creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group creat09.c:110: TFAIL: mntpoint/testdir/creat.tmp: Setgid bit is set creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group creat09.c:110: TFAIL: mntpoint/testdir/open.tmp: Setgid bit is set ... $ uname -r 5.19.0 $ git describe 1639a49ccdce v5.19-rc7-3-g1639a49ccdce Is there any in-flight patch for v6.0 that fixes this for XFS?
On Wed, Sep 21, 2022 at 03:55:56PM +0200, Cyril Hrubis wrote: > Hi! > > > Yes. Please apply it with fix the messages. Thanks. > > > > I guess you miss this patchset. so ping again. > > Sorry for the delay. > > I just did quick test and I got failures on XFS on kernel 5.19, wasn't > this supposed to be fixed in 5.19? No, this is fixed starting with v6.0-rc1 :) > > ... > creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group > creat09.c:112: TPASS: mntpoint/testdir/creat.tmp: Setgid bit not set > creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group > creat09.c:112: TPASS: mntpoint/testdir/open.tmp: Setgid bit not set > creat09.c:120: TINFO: File created with umask(S_IXGRP) > creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group > creat09.c:110: TFAIL: mntpoint/testdir/creat.tmp: Setgid bit is set > creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group > creat09.c:110: TFAIL: mntpoint/testdir/open.tmp: Setgid bit is set > ... > > $ uname -r > 5.19.0 > > $ git describe 1639a49ccdce > v5.19-rc7-3-g1639a49ccdce > > > Is there any in-flight patch for v6.0 that fixes this for XFS? You're looking for https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=426b4ca2d6a5ab51f6b6175d06e4f8ddea434cdf Hope this helps! Christian
Hi! > > Is there any in-flight patch for v6.0 that fixes this for XFS? > > You're looking for > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=426b4ca2d6a5ab51f6b6175d06e4f8ddea434cdf That's the missing piece! I've added this patch to the test tags and pushed, thanks everyone.
diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c index bed7bddb0..d583cceca 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> @@ -47,6 +57,14 @@ static gid_t free_gid; static int fd = -1; +static struct tcase { + const char *msg; + int mask; +} tcases[] = { + {"under umask(0) situation", 0}, + {"under umask(S_IXGRP) situation", S_IXGRP} +}; + static void setup(void) { struct stat buf; @@ -94,8 +112,14 @@ 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) { + struct tcase *tc = &tcases[n]; + + umask(tc->mask); + tst_res(TINFO, "Testing setgid behaviour when creating file %s", + tc->msg); + fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); SAFE_CLOSE(fd); file_test(CREAT_FILE); @@ -115,13 +139,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 = ARRAY_SIZE(tcases), .skip_filesystems = (const char*[]) { "exfat", "ntfs", @@ -132,6 +157,7 @@ static struct tst_test test = { {"linux-git", "0fa3ecd87848"}, {"CVE", "2018-13405"}, {"linux-git", "01ea173e103e"}, + {"linux-git", "1639a49ccdce"}, {} }, };