diff mbox series

[v2,1/2] syscalls/creat09: Add umask test condition

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

Commit Message

xuyang2018.jy@fujitsu.com Aug. 3, 2022, 3:24 a.m. UTC
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(-)

Comments

Christian Brauner Aug. 3, 2022, 7:49 a.m. UTC | #1
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
xuyang2018.jy@fujitsu.com Aug. 3, 2022, 8:06 a.m. UTC | #2
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
Martin Doucha Aug. 4, 2022, 3:47 p.m. UTC | #3
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"},
>  		{}
>  	},
>  };
xuyang2018.jy@fujitsu.com Aug. 5, 2022, 11:13 a.m. UTC | #4
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"},
>>   		{}
>>   	},
>>   };
> 
>
Christian Brauner Aug. 15, 2022, 9:32 a.m. UTC | #5
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 mbox series

Patch

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"},
 		{}
 	},
 };