Messages in this thread Patch in this message | | | Subject | Re: [PATCH] tomoyo: Avoid potential null pointer access | From | Zheng Zengkai <> | Date | Thu, 26 Nov 2020 14:33:55 +0800 |
| |
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]; >> > . >
| |