[LTP] [PATCH v3 0/3] Fix bug caused by default umask

[LTP] [PATCH v3 0/3] Fix bug caused by default umask

[Zou Wei]

v3:
The case is only successful when system default umask is 0022.
When default umask is 0077 or other values, should clear umask by umask(0) in setup().

Zou Wei (3):
  security/dirtyc0w: Fix bug caused by default umask
  syscalls/fstat: Fix bug caused by default umask
  syscalls/statx01: Fix bug caused by default umask

 testcases/kernel/security/dirtyc0w/dirtyc0w.c | 2 ++
 testcases/kernel/syscalls/fstat/fstat02.c     | 2 ++
 testcases/kernel/syscalls/statx/statx01.c     | 2 ++
 3 files changed, 6 insertions(+)

--
2.6.2

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Zou Wei]

v3:
The case is only successful when system default umask is 0022.
When default umask is 0077 or other values, should clear current umask by umask(0) in setup().

Signed-off-by: Zou Wei <zou_wei@huawei.com>
---
 testcases/kernel/security/dirtyc0w/dirtyc0w.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/security/dirtyc0w/dirtyc0w.c b/testcases/kernel/security/dirtyc0w/dirtyc0w.c
index a0a81b6..6455f9c 100644
--- a/testcases/kernel/security/dirtyc0w/dirtyc0w.c
+++ b/testcases/kernel/security/dirtyc0w/dirtyc0w.c
@@ -45,6 +45,8 @@ static void setup(void)
 {
 	struct passwd *pw;

+	umask(0);
+
 	pw = SAFE_GETPWNAM("nobody");

 	nobody_uid = pw->pw_uid;
--
2.6.2

[LTP] [PATCH v3 2/3] syscalls/fstat: Fix bug caused by default umask

[Zou Wei]

v3:
The case is only successful when system default umask is 0022.
When default umask is 0077 or other values, should clear current umask by umask(0) in setup().

Signed-off-by: Zou Wei <zou_wei@huawei.com>
---
 testcases/kernel/syscalls/fstat/fstat02.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/syscalls/fstat/fstat02.c b/testcases/kernel/syscalls/fstat/fstat02.c
index 4744560..c0229de 100644
--- a/testcases/kernel/syscalls/fstat/fstat02.c
+++ b/testcases/kernel/syscalls/fstat/fstat02.c
@@ -72,6 +72,8 @@ static void setup(void)
 	user_id  = getuid();
 	group_id = getgid();

+	umask(0);
+
 	fildes = SAFE_OPEN(TESTFILE, O_WRONLY | O_CREAT, FILE_MODE);

 	if (tst_fill_file(TESTFILE, 'a', FILE_SIZE, 1))
--
2.6.2

[LTP] [PATCH v3 3/3] syscalls/statx01: Fix bug caused by default umask

[Zou Wei]

v3:
The case is only successful when system default umask is 0022.
When default umask is 0077 or other values, should clear current umask by umask(0) in setup().

Signed-off-by: Zou Wei <zou_wei@huawei.com>
---
 testcases/kernel/syscalls/statx/statx01.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
index 4f9d582..2358dd7 100644
--- a/testcases/kernel/syscalls/statx/statx01.c
+++ b/testcases/kernel/syscalls/statx/statx01.c
@@ -148,6 +148,8 @@ static void setup(void)
 	char data_buff[SIZE];
 	int file_fd;

+	umask(0);
+
 	memset(data_buff, '@', sizeof(data_buff));

 	file_fd =  SAFE_OPEN(TESTFILE, O_RDWR|O_CREAT, MODE);
--
2.6.2

[LTP] [PATCH v3 0/3] Fix bug caused by default umask

[Xiao Yang]

On 2020/3/30 15:27, Zou Wei wrote:
> v3:
> The case is only successful when system default umask is 0022.
> When default umask is 0077 or other values, should clear umask by umask(0) in setup().
Hi Zou,

Thanks for your patch set.
Did you find any other cases related with the same issue?

Thanks,
Xiao Yang
>
> Zou Wei (3):
>    security/dirtyc0w: Fix bug caused by default umask
>    syscalls/fstat: Fix bug caused by default umask
>    syscalls/statx01: Fix bug caused by default umask
>
>   testcases/kernel/security/dirtyc0w/dirtyc0w.c | 2 ++
>   testcases/kernel/syscalls/fstat/fstat02.c     | 2 ++
>   testcases/kernel/syscalls/statx/statx01.c     | 2 ++
>   3 files changed, 6 insertions(+)
>
> --
> 2.6.2
>
>

[LTP] 答复: [PATCH v3 0/3] Fix bug caused by default umask

[Zouwei]

Hi Yang,

I downloaded the development version of ltp two weeks ago, ran the cases in the runtest directory, and analyzed all the failed cases. 
I found that these 3 cases have the umask issue. 

Due to the limited time invested by individuals, it is possible that other cases with the same issue will not be discovered.
I will analyze the other failed cases that have not been concluded further in the next few weeks.

Thanks for your suggestions and reviews.

Best Regards
Zou Wei
-----????-----
???: Xiao Yang [mailto:yangx.jy@cn.fujitsu.com] 
????: 2020?3?30? 15:34
???: Zouwei (Samuel) <zou_wei@huawei.com>
??: ltp@lists.linux.it
??: Re: [LTP] [PATCH v3 0/3] Fix bug caused by default umask

On 2020/3/30 15:27, Zou Wei wrote:
> v3:
> The case is only successful when system default umask is 0022.
> When default umask is 0077 or other values, should clear umask by umask(0) in setup().
Hi Zou,

Thanks for your patch set.
Did you find any other cases related with the same issue?

Thanks,
Xiao Yang
>
> Zou Wei (3):
>    security/dirtyc0w: Fix bug caused by default umask
>    syscalls/fstat: Fix bug caused by default umask
>    syscalls/statx01: Fix bug caused by default umask
>
>   testcases/kernel/security/dirtyc0w/dirtyc0w.c | 2 ++
>   testcases/kernel/syscalls/fstat/fstat02.c     | 2 ++
>   testcases/kernel/syscalls/statx/statx01.c     | 2 ++
>   3 files changed, 6 insertions(+)
>
> --
> 2.6.2
>
>

[LTP] 答复: [PATCH v3 0/3] Fix bug caused by default umask

[Xiao Yang]

On 2020/3/30 16:22, Zouwei (Samuel) wrote:
> Hi Yang,
> 
> I downloaded the development version of ltp two weeks ago, ran the cases in the runtest directory, and analyzed all the failed cases.
> I found that these 3 cases have the umask issue.
> 
> Due to the limited time invested by individuals, it is possible that other cases with the same issue will not be discovered.
> I will analyze the other failed cases that have not been concluded further in the next few weeks.
Hi Zou,

Thanks, I have pushed the patch set.
Perhaps, you can fix all same umask issues by a single patch in future.

Thanks,
Xiao Yang
> 
> Thanks for your suggestions and reviews.
> 
> Best Regards
> Zou Wei
> -----????-----
> ???: Xiao Yang [mailto:yangx.jy@cn.fujitsu.com]
> ????: 2020?3?30? 15:34
> ???: Zouwei (Samuel)<zou_wei@huawei.com>
> ??: ltp@lists.linux.it
> ??: Re: [LTP] [PATCH v3 0/3] Fix bug caused by default umask
> 
> On 2020/3/30 15:27, Zou Wei wrote:
>> v3:
>> The case is only successful when system default umask is 0022.
>> When default umask is 0077 or other values, should clear umask by umask(0) in setup().
> Hi Zou,
> 
> Thanks for your patch set.
> Did you find any other cases related with the same issue?
> 
> Thanks,
> Xiao Yang
>>
>> Zou Wei (3):
>>     security/dirtyc0w: Fix bug caused by default umask
>>     syscalls/fstat: Fix bug caused by default umask
>>     syscalls/statx01: Fix bug caused by default umask
>>
>>    testcases/kernel/security/dirtyc0w/dirtyc0w.c | 2 ++
>>    testcases/kernel/syscalls/fstat/fstat02.c     | 2 ++
>>    testcases/kernel/syscalls/statx/statx01.c     | 2 ++
>>    3 files changed, 6 insertions(+)
>>
>> --
>> 2.6.2
>>
>>
> 
> 
> 
> 
> 

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Cyril Hrubis]

Hi!
> The case is only successful when system default umask is 0022.
> When default umask is 0077 or other values, should clear current umask by umask(0) in setup().

What exactly fails in the dirtyc0w because of non-standard umask?

I can easily understand why fstat and statx fail, since we check the
file permissions there. Ideally the failure should have been part of the
patch description.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Cyril Hrubis]

Hi!
> > The case is only successful when system default umask is 0022.
> > When default umask is 0077 or other values, should clear current umask by umask(0) in setup().
> 
> What exactly fails in the dirtyc0w because of non-standard umask?
> 
> I can easily understand why fstat and statx fail, since we check the
> file permissions there. Ideally the failure should have been part of the
> patch description.

Looking at the code I guess that the child fails to open the file
created in parent before the setgid/setuid, something like this should
really have been part of the patch description.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Xiao Yang]

On 3/30/20 5:20 PM, Cyril Hrubis wrote:
> Hi!
>>> The case is only successful when system default umask is 0022.
>>> When default umask is 0077 or other values, should clear current umask by umask(0) in setup().
>> What exactly fails in the dirtyc0w because of non-standard umask?
>>
>> I can easily understand why fstat and statx fail, since we check the
>> file permissions there. Ideally the failure should have been part of the
>> patch description.
> Looking at the code I guess that the child fails to open the file
> created in parent before the setgid/setuid, something like this should
> really have been part of the patch description.
>
Hi Cyril,

Right, some default mask drop the read permission of g/o and

nobody user cannot do read. :-)

Thanks,

Xiao Yang

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Xiao Yang]

Hi Cyril, Petr

Just one question when reviewing this patch set.

I saw the umask set(e.g. set umask to 0 or 0022) is used by a lot of tests.

Is it necessary for metadata project to introduce .needs_umask?

Thanks,

Xiao Yang

On 3/30/20 5:20 PM, Cyril Hrubis wrote:
> Hi!
>>> The case is only successful when system default umask is 0022.
>>> When default umask is 0077 or other values, should clear current umask by umask(0) in setup().
>> What exactly fails in the dirtyc0w because of non-standard umask?
>>
>> I can easily understand why fstat and statx fail, since we check the
>> file permissions there. Ideally the failure should have been part of the
>> patch description.
> Looking at the code I guess that the child fails to open the file
> created in parent before the setgid/setuid, something like this should
> really have been part of the patch description.
>

[LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Cyril Hrubis]

Hi!
> Just one question when reviewing this patch set.
> 
> I saw the umask set(e.g. set umask to 0 or 0022) is used by a lot of tests.
> 
> Is it necessary for metadata project to introduce .needs_umask?

I do no think so.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] 答复: [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

[Zouwei]

Hi Yang and Cyril,

Thank you so much for all your suggestions.

Best Regards
Zou Wei

-----????-----
???: Xiao Yang [mailto:ice_yangxiao@163.com] 
????: 2020?3?30? 20:34
???: Cyril Hrubis <chrubis@suse.cz>; Zouwei (Samuel) <zou_wei@huawei.com>
??: ltp@lists.linux.it
??: Re: [LTP] [PATCH v3 1/3] security/dirtyc0w: Fix bug caused by default umask

On 3/30/20 5:20 PM, Cyril Hrubis wrote:
> Hi!
>>> The case is only successful when system default umask is 0022.
>>> When default umask is 0077 or other values, should clear current umask by umask(0) in setup().
>> What exactly fails in the dirtyc0w because of non-standard umask?
>>
>> I can easily understand why fstat and statx fail, since we check the 
>> file permissions there. Ideally the failure should have been part of 
>> the patch description.
> Looking at the code I guess that the child fails to open the file 
> created in parent before the setgid/setuid, something like this should 
> really have been part of the patch description.
>
Hi Cyril,

Right, some default mask drop the read permission of g/o and

nobody user cannot do read. :-)

Thanks,

Xiao Yang