* [PATCH] firmware: Google VPD: Fix memory allocation error handling
@ 2017-05-05 19:08 Christophe JAILLET
2017-05-05 19:56 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Christophe JAILLET @ 2017-05-05 19:08 UTC (permalink / raw)
To: gregkh, thierry.escande, wnhuang
Cc: linux-kernel, kernel-janitors, Christophe JAILLET
This patch fixes several issues:
- if the 1st 'kzalloc' fails, we dereference a NULL pointer
- if the 2nd 'kzalloc' fails, there is a memory leak
- if 'sysfs_create_bin_file' fails there is also a memory leak
Fix it by adding a test after the first memory allocation and some error
handling paths to correctly free memory if needed.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/firmware/google/vpd.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 619f4bae474f..2a1b0040a220 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -116,9 +116,13 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
return VPD_OK;
info = kzalloc(sizeof(*info), GFP_KERNEL);
- info->key = kzalloc(key_len + 1, GFP_KERNEL);
- if (!info->key)
+ if (!info)
return -ENOMEM;
+ info->key = kzalloc(key_len + 1, GFP_KERNEL);
+ if (!info->key) {
+ ret = -ENOMEM;
+ goto free_info;
+ }
memcpy(info->key, key, key_len);
@@ -135,12 +139,17 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
list_add_tail(&info->list, &sec->attribs);
ret = sysfs_create_bin_file(sec->kobj, &info->bin_attr);
- if (ret) {
- kfree(info->key);
- return ret;
- }
+ if (ret)
+ goto free_info_key;
return 0;
+
+free_info_key:
+ kfree(info->key);
+free_info:
+ kfree(info);
+
+ return ret;
}
static void vpd_section_attrib_destroy(struct vpd_section *sec)
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: Google VPD: Fix memory allocation error handling
2017-05-05 19:08 [PATCH] firmware: Google VPD: Fix memory allocation error handling Christophe JAILLET
@ 2017-05-05 19:56 ` Greg KH
2017-05-06 5:02 ` Christophe JAILLET
2017-05-06 8:40 ` Dan Carpenter
0 siblings, 2 replies; 4+ messages in thread
From: Greg KH @ 2017-05-05 19:56 UTC (permalink / raw)
To: Christophe JAILLET
Cc: thierry.escande, wnhuang, linux-kernel, kernel-janitors
On Fri, May 05, 2017 at 09:08:44PM +0200, Christophe JAILLET wrote:
> This patch fixes several issues:
> - if the 1st 'kzalloc' fails, we dereference a NULL pointer
> - if the 2nd 'kzalloc' fails, there is a memory leak
> - if 'sysfs_create_bin_file' fails there is also a memory leak
Then it should be multiple patches, not fixing 3 things in one patch,
right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: Google VPD: Fix memory allocation error handling
2017-05-05 19:56 ` Greg KH
@ 2017-05-06 5:02 ` Christophe JAILLET
2017-05-06 8:40 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2017-05-06 5:02 UTC (permalink / raw)
To: Greg KH; +Cc: thierry.escande, wnhuang, linux-kernel, kernel-janitors
Le 05/05/2017 à 21:56, Greg KH a écrit :
> On Fri, May 05, 2017 at 09:08:44PM +0200, Christophe JAILLET wrote:
>> This patch fixes several issues:
>> - if the 1st 'kzalloc' fails, we dereference a NULL pointer
>> - if the 2nd 'kzalloc' fails, there is a memory leak
>> - if 'sysfs_create_bin_file' fails there is also a memory leak
> Then it should be multiple patches, not fixing 3 things in one patch,
> right?
>
> thanks,
>
> greg k-h
>
I can split it if you want, but the 3 points are more or less related
and all belong to the same few lines of code. I didn't think having 3
patches was needed in this case. I just wanted to give a detailed changelog.
If the commit message was only "This patch fixes memory allocation error
handling in fct xxx' (as in the topic), I guess it would has been
accepted (if it is correct of course) as-is.
Just tell me if you really prefer 3 patches, and I will resubmit.
CJ
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: Google VPD: Fix memory allocation error handling
2017-05-05 19:56 ` Greg KH
2017-05-06 5:02 ` Christophe JAILLET
@ 2017-05-06 8:40 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-05-06 8:40 UTC (permalink / raw)
To: Greg KH
Cc: Christophe JAILLET, thierry.escande, wnhuang, linux-kernel,
kernel-janitors
On Fri, May 05, 2017 at 12:56:47PM -0700, Greg KH wrote:
> On Fri, May 05, 2017 at 09:08:44PM +0200, Christophe JAILLET wrote:
> > This patch fixes several issues:
> > - if the 1st 'kzalloc' fails, we dereference a NULL pointer
> > - if the 2nd 'kzalloc' fails, there is a memory leak
> > - if 'sysfs_create_bin_file' fails there is also a memory leak
>
> Then it should be multiple patches, not fixing 3 things in one patch,
> right?
>
I agree with Christophe that this is basically one thing. Otherwise you
end up breaking it up how Elfring does it into tiny tiny snippets that I
can't read. Doing it this way is easier to review for me.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-06 8:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-05 19:08 [PATCH] firmware: Google VPD: Fix memory allocation error handling Christophe JAILLET
2017-05-05 19:56 ` Greg KH
2017-05-06 5:02 ` Christophe JAILLET
2017-05-06 8:40 ` Dan Carpenter
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).