[PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[Ren Yu]

As the possible alloc failure of the kmalloc() and vmalloc(),the
return pointer could be NULL.therefore it should be better to check it.

Signed-off-by: Ren Yu <renyu@nfschina.com>
---
 drivers/misc/lkdtm/perms.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index b93404d65650..34b43b9ea1f1 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -180,6 +180,9 @@ static void lkdtm_EXEC_STACK(void)
 static void lkdtm_EXEC_KMALLOC(void)
 {
 	u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL);
+	if (!kmalloc_area)
+		return;
+
 	execute_location(kmalloc_area, CODE_WRITE);
 	kfree(kmalloc_area);
 }
@@ -187,6 +190,9 @@ static void lkdtm_EXEC_KMALLOC(void)
 static void lkdtm_EXEC_VMALLOC(void)
 {
 	u32 *vmalloc_area = vmalloc(EXEC_SIZE);
+	if (!vmalloc_area)
+		return;
+
 	execute_location(vmalloc_area, CODE_WRITE);
 	vfree(vmalloc_area);
 }
-- 
2.11.0

Re: [PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[Greg KH]

On Mon, Jul 25, 2022 at 04:11:53PM +0800, Ren Yu wrote:
> As the possible alloc failure of the kmalloc() and vmalloc(),the
> return pointer could be NULL.therefore it should be better to check it.
> 
> Signed-off-by: Ren Yu <renyu@nfschina.com>
> ---
>  drivers/misc/lkdtm/perms.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> index b93404d65650..34b43b9ea1f1 100644
> --- a/drivers/misc/lkdtm/perms.c
> +++ b/drivers/misc/lkdtm/perms.c
> @@ -180,6 +180,9 @@ static void lkdtm_EXEC_STACK(void)
>  static void lkdtm_EXEC_KMALLOC(void)
>  {
>  	u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL);
> +	if (!kmalloc_area)
> +		return;
> +

Always run checkpatch on your patches so that grumpy maintainers do not
have to tell you to run checkpatch on your patches...

Also, shouldn't this return an error?

But most importantly, how can this ever fail?

thanks,

greg k-h

Re: [PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[tury]

When there is insufficient memory, the allocation will fail.

the return value is void,so i think it is ok .

should i have changed comment to something like this ?

As the possible alloc failure of the kmalloc() and vmalloc(),
the return pointer could be NULL.therefore it should be better to check it.


在 2022年07月25日 16:42, Greg KH 写道:
> On Mon, Jul 25, 2022 at 04:11:53PM +0800, Ren Yu wrote:
>> As the possible alloc failure of the kmalloc() and vmalloc(),the
>> return pointer could be NULL.therefore it should be better to check it.
>>
>> Signed-off-by: Ren Yu <renyu@nfschina.com>
>> ---
>>   drivers/misc/lkdtm/perms.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
>> index b93404d65650..34b43b9ea1f1 100644
>> --- a/drivers/misc/lkdtm/perms.c
>> +++ b/drivers/misc/lkdtm/perms.c
>> @@ -180,6 +180,9 @@ static void lkdtm_EXEC_STACK(void)
>>   static void lkdtm_EXEC_KMALLOC(void)
>>   {
>>   	u32 *kmalloc_area = kmalloc(EXEC_SIZE, GFP_KERNEL);
>> +	if (!kmalloc_area)
>> +		return;
>> +
> Always run checkpatch on your patches so that grumpy maintainers do not
> have to tell you to run checkpatch on your patches...
>
> Also, shouldn't this return an error?
>
> But most importantly, how can this ever fail?
>
> thanks,
>
> greg k-h
>

Re: [PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Mon, Jul 25, 2022 at 05:54:15PM +0800, tury wrote:
> When there is insufficient memory, the allocation will fail.

And have you ever seen that happen here?  The issue is for small
allocations, they never will fail.

> the return value is void,so i think it is ok .

Why?

> should i have changed comment to something like this ?
> 
> As the possible alloc failure of the kmalloc() and vmalloc(),
> the return pointer could be NULL.therefore it should be better to check it.

Please wrap your lines properly, and use '.' correctly.

thanks,

greg k-h

Re: [PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[tury]

在 2022年07月25日 20:37, Greg KH 写道:
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
>
> On Mon, Jul 25, 2022 at 05:54:15PM +0800, tury wrote:
>> When there is insufficient memory, the allocation will fail.
> And have you ever seen that happen here?  The issue is for small
> allocations, they never will fail.
>
>> the return value is void,so i think it is ok .
> Why?
Because the function lkdtm_EXEC_KMALLOC()   declaration is void,and The 
return value is not checked elsewhere.
Should I add some warning messages?
>
>> should i have changed comment to something like this ?
>>
>> As the possible alloc failure of the kmalloc() and vmalloc(),
>> the return pointer could be NULL.therefore it should be better to check it.
> Please wrap your lines properly, and use '.' correctly.
>
> thanks,
>
> greg k-h
Thank you for your help and patience,
>

Re: [PATCH 2/3] lkdtm/perms: Check possible NULL pointer returned by kmalloc(),vmalloc()

[Kees Cook]

On Tue, Jul 26, 2022 at 01:43:41PM +0800, tury wrote:
> 在 2022年07月25日 20:37, Greg KH 写道:
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> > 
> > A: No.
> > Q: Should I include quotations after my reply?
> > 
> > http://daringfireball.net/2007/07/on_top
> > 
> > On Mon, Jul 25, 2022 at 05:54:15PM +0800, tury wrote:
> > > When there is insufficient memory, the allocation will fail.
> > And have you ever seen that happen here?  The issue is for small
> > allocations, they never will fail.
> > 
> > > the return value is void,so i think it is ok .
> > Why?
> Because the function lkdtm_EXEC_KMALLOC()   declaration is void,and The
> return value is not checked elsewhere.
> Should I add some warning messages?

Memory allocation failures will already be reported by the allocator, so
there is normally no need for an additional message.

-- 
Kees Cook