* [PATCH] tomoyo: Avoid potential null pointer access
@ 2020-11-25 12:10 Zheng Zengkai
2020-11-25 12:25 ` Tetsuo Handa
0 siblings, 1 reply; 6+ messages in thread
From: Zheng Zengkai @ 2020-11-25 12:10 UTC (permalink / raw)
To: takedakn, penguin-kernel, jmorris, serge
Cc: linux-security-module, linux-kernel, zhengzengkai
Calls to kzalloc() should be null-checked in order to avoid
any potential failures or unnecessary code execution.
Fix this by adding null checks for _entry_ right after allocation.
Fixes: 57c2590fb7fd ("TOMOYO: Update profile structure")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com>
---
security/tomoyo/common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 4bee32bfe16d..99b4fafcb100 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -499,6 +499,8 @@ static struct tomoyo_profile *tomoyo_assign_profile
if (ptr)
return ptr;
entry = kzalloc(sizeof(*entry), GFP_NOFS);
+ if (!entry)
+ return NULL;
if (mutex_lock_interruptible(&tomoyo_policy_lock))
goto out;
ptr = ns->profile_ptr[profile];
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tomoyo: Avoid potential null pointer access 2020-11-25 12:10 [PATCH] tomoyo: Avoid potential null pointer access Zheng Zengkai @ 2020-11-25 12:25 ` Tetsuo Handa 2020-11-26 6:33 ` Zheng Zengkai 0 siblings, 1 reply; 6+ messages in thread From: Tetsuo Handa @ 2020-11-25 12:25 UTC (permalink / raw) To: Zheng Zengkai; +Cc: linux-security-module, linux-kernel, jmorris, serge Hello, Zheng. Thank you for a patch, but I won't apply this patch. Expected behavior is that tomoyo_warn_oom() is called if tomoyo_memory_ok() is called with entry == NULL. Adding __GFP_NOWARN might be OK, but returning without tomoyo_warn_oom() is NG. On 2020/11/25 21:10, Zheng Zengkai wrote: > Calls to kzalloc() should be null-checked in order to avoid > any potential failures or unnecessary code execution. > Fix this by adding null checks for _entry_ right after allocation. > > Fixes: 57c2590fb7fd ("TOMOYO: Update profile structure") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com> Nacked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > security/tomoyo/common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c > index 4bee32bfe16d..99b4fafcb100 100644 > --- a/security/tomoyo/common.c > +++ b/security/tomoyo/common.c > @@ -499,6 +499,8 @@ static struct tomoyo_profile *tomoyo_assign_profile > if (ptr) > return ptr; > entry = kzalloc(sizeof(*entry), GFP_NOFS); > + if (!entry) > + return NULL; > if (mutex_lock_interruptible(&tomoyo_policy_lock)) > goto out; > ptr = ns->profile_ptr[profile]; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tomoyo: Avoid potential null pointer access 2020-11-25 12:25 ` Tetsuo Handa @ 2020-11-26 6:33 ` Zheng Zengkai 2020-11-26 6:57 ` Tetsuo Handa 0 siblings, 1 reply; 6+ messages in thread From: Zheng Zengkai @ 2020-11-26 6:33 UTC (permalink / raw) To: Tetsuo Handa Cc: linux-security-module, linux-kernel, jmorris, serge, weiyongjun1 Hello, Tetsuo Got it , Thank you for your explanation. > Hello, Zheng. > > Thank you for a patch, but I won't apply this patch. > Expected behavior is that tomoyo_warn_oom() is called > if tomoyo_memory_ok() is called with entry == NULL. > > Adding __GFP_NOWARN might be OK, but returning without tomoyo_warn_oom() is NG. > > On 2020/11/25 21:10, Zheng Zengkai wrote: >> Calls to kzalloc() should be null-checked in order to avoid >> any potential failures or unnecessary code execution. >> Fix this by adding null checks for _entry_ right after allocation. >> >> Fixes: 57c2590fb7fd ("TOMOYO: Update profile structure") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com> > Nacked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> As your say, I found the function tomoyo_assign_namespace( ) in security/tomoyo/domain.c has the similar situation, Can I add __GFP_NOWARN for both and remove the null check for _entry_ in tomoyo_assign_namespace( )? diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 4bee32bfe16d..bc54d3c8c70a 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -498,7 +498,7 @@ static struct tomoyo_profile *tomoyo_assign_profile ptr = ns->profile_ptr[profile]; if (ptr) return ptr; - entry = kzalloc(sizeof(*entry), GFP_NOFS); + entry = kzalloc(sizeof(*entry), GFP_NOFS | __GFP_NOWARN); if (mutex_lock_interruptible(&tomoyo_policy_lock)) goto out; ptr = ns->profile_ptr[profile]; diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c index dc4ecc0b2038..c6e5cc5cc7cd 100644 --- a/security/tomoyo/domain.c +++ b/security/tomoyo/domain.c @@ -473,9 +473,7 @@ struct tomoyo_policy_namespace *tomoyo_assign_namespace(const char *domainname) return ptr; if (len >= TOMOYO_EXEC_TMPSIZE - 10 || !tomoyo_domain_def(domainname)) return NULL; - entry = kzalloc(sizeof(*entry) + len + 1, GFP_NOFS); - if (!entry) - return NULL; + entry = kzalloc(sizeof(*entry) + len + 1, GFP_NOFS | __GFP_NOWARN); if (mutex_lock_interruptible(&tomoyo_policy_lock)) goto out; ptr = tomoyo_find_namespace(domainname, len); >> --- >> security/tomoyo/common.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c >> index 4bee32bfe16d..99b4fafcb100 100644 >> --- a/security/tomoyo/common.c >> +++ b/security/tomoyo/common.c >> @@ -499,6 +499,8 @@ static struct tomoyo_profile *tomoyo_assign_profile >> if (ptr) >> return ptr; >> entry = kzalloc(sizeof(*entry), GFP_NOFS); >> + if (!entry) >> + return NULL; >> if (mutex_lock_interruptible(&tomoyo_policy_lock)) >> goto out; >> ptr = ns->profile_ptr[profile]; >> > . > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tomoyo: Avoid potential null pointer access 2020-11-26 6:33 ` Zheng Zengkai @ 2020-11-26 6:57 ` Tetsuo Handa 2020-11-27 7:17 ` Zheng Zengkai 0 siblings, 1 reply; 6+ messages in thread From: Tetsuo Handa @ 2020-11-26 6:57 UTC (permalink / raw) To: Zheng Zengkai Cc: linux-security-module, linux-kernel, jmorris, serge, weiyongjun1 On 2020/11/26 15:33, Zheng Zengkai wrote: > As your say, I found the function tomoyo_assign_namespace( ) > > in security/tomoyo/domain.c has the similar situation, > > Can I add __GFP_NOWARN for both and remove the null check for _entry_ in tomoyo_assign_namespace( )? > Good catch. Yes, please send as a patch. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tomoyo: Avoid potential null pointer access 2020-11-26 6:57 ` Tetsuo Handa @ 2020-11-27 7:17 ` Zheng Zengkai 2020-11-27 10:52 ` Tetsuo Handa 0 siblings, 1 reply; 6+ messages in thread From: Zheng Zengkai @ 2020-11-27 7:17 UTC (permalink / raw) To: Tetsuo Handa Cc: linux-security-module, linux-kernel, jmorris, serge, weiyongjun1 Hello Tetsuo, > On 2020/11/26 15:33, Zheng Zengkai wrote: >> As your say, I found the function tomoyo_assign_namespace( ) >> >> in security/tomoyo/domain.c has the similar situation, >> >> Can I add __GFP_NOWARN for both and remove the null check for _entry_ in tomoyo_assign_namespace( )? >> > Good catch. Yes, please send as a patch. > . I have resent a patch, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tomoyo: Avoid potential null pointer access 2020-11-27 7:17 ` Zheng Zengkai @ 2020-11-27 10:52 ` Tetsuo Handa 0 siblings, 0 replies; 6+ messages in thread From: Tetsuo Handa @ 2020-11-27 10:52 UTC (permalink / raw) To: Zheng Zengkai Cc: linux-security-module, linux-kernel, jmorris, serge, weiyongjun1 On 2020/11/27 16:17, Zheng Zengkai wrote: > Hello Tetsuo, >> On 2020/11/26 15:33, Zheng Zengkai wrote: >>> As your say, I found the function tomoyo_assign_namespace( ) >>> >>> in security/tomoyo/domain.c has the similar situation, >>> >>> Can I add __GFP_NOWARN for both and remove the null check for _entry_ in tomoyo_assign_namespace( )? >>> >> Good catch. Yes, please send as a patch. >> . > > I have resent a patch, thanks! > Applied to tomoyo-test1.git tree. Thank you. By the way, since some people automatically backport patches with Fixes: tag, I think we don't need to add Fixes: tag for patches like this one. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-27 10:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-25 12:10 [PATCH] tomoyo: Avoid potential null pointer access Zheng Zengkai 2020-11-25 12:25 ` Tetsuo Handa 2020-11-26 6:33 ` Zheng Zengkai 2020-11-26 6:57 ` Tetsuo Handa 2020-11-27 7:17 ` Zheng Zengkai 2020-11-27 10:52 ` Tetsuo Handa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).