* [PATCH] staging: gasket Create a memory allocation path
@ 2018-10-29 14:07 Ioannis Valasakis
2018-10-29 14:09 ` [Outreachy kernel] " Vaishali Thakkar
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ioannis Valasakis @ 2018-10-29 14:07 UTC (permalink / raw)
To: outreachy-kernel; +Cc: gregkh, rspringer
toddpoynor@google.com, benchan@chromium.org
Bcc:
Subject: [PATCH] staging: gasket: Create a memory allocation error handling
path
Reply-To:
Create an error handling path for memory allocation. It avoid repeating
the same assignments and returns the respective ENOMEM.
Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
---
drivers/staging/gasket/gasket_interrupt.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index 1cfbc120f228..cdfb72af1c52 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -348,27 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
sizeof(struct msix_entry),
GFP_KERNEL);
if (!interrupt_data->msix_entries) {
- kfree(interrupt_data);
- return -ENOMEM;
+ goto err_alloc;
}
interrupt_data->eventfd_ctxs = kcalloc(num_interrupts,
sizeof(struct eventfd_ctx *),
GFP_KERNEL);
if (!interrupt_data->eventfd_ctxs) {
- kfree(interrupt_data->msix_entries);
- kfree(interrupt_data);
- return -ENOMEM;
+ goto err_alloc;
}
interrupt_data->interrupt_counts = kcalloc(num_interrupts,
sizeof(ulong),
GFP_KERNEL);
if (!interrupt_data->interrupt_counts) {
- kfree(interrupt_data->eventfd_ctxs);
- kfree(interrupt_data->msix_entries);
- kfree(interrupt_data);
- return -ENOMEM;
+ goto err_alloc;
}
switch (interrupt_data->type) {
@@ -402,6 +396,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
interrupt_sysfs_attrs);
return 0;
+
+err_alloc:
+ kfree(interrupt_data);
+ kfree(interrupt_data->msix_entries);
+ kfree(interrupt_data->eventfd_ctxs);
+ return -ENOMEM;
}
static void
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: gasket Create a memory allocation path
2018-10-29 14:07 [PATCH] staging: gasket Create a memory allocation path Ioannis Valasakis
@ 2018-10-29 14:09 ` Vaishali Thakkar
2018-10-29 14:14 ` Julia Lawall
2018-10-29 14:31 ` Greg KH
2 siblings, 0 replies; 6+ messages in thread
From: Vaishali Thakkar @ 2018-10-29 14:09 UTC (permalink / raw)
To: code; +Cc: outreachy-kernel, Greg KH, Rob Springer
On Mon, Oct 29, 2018 at 7:37 PM Ioannis Valasakis <code@wizofe.uk> wrote:
>
> toddpoynor@google.com, benchan@chromium.org
> Bcc:
> Subject: [PATCH] staging: gasket: Create a memory allocation error handling
> path
> Reply-To:
Hi Ioannis,
This didn't work properly. Please send again and always check by sending
it to yourself first. :)
> Create an error handling path for memory allocation. It avoid repeating
> the same assignments and returns the respective ENOMEM.
>
> Signed-off-by: Ioannis Valasakis <code@wizofe.uk>
> ---
> drivers/staging/gasket/gasket_interrupt.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
> index 1cfbc120f228..cdfb72af1c52 100644
> --- a/drivers/staging/gasket/gasket_interrupt.c
> +++ b/drivers/staging/gasket/gasket_interrupt.c
> @@ -348,27 +348,21 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
> sizeof(struct msix_entry),
> GFP_KERNEL);
> if (!interrupt_data->msix_entries) {
> - kfree(interrupt_data);
> - return -ENOMEM;
> + goto err_alloc;
> }
>
> interrupt_data->eventfd_ctxs = kcalloc(num_interrupts,
> sizeof(struct eventfd_ctx *),
> GFP_KERNEL);
> if (!interrupt_data->eventfd_ctxs) {
> - kfree(interrupt_data->msix_entries);
> - kfree(interrupt_data);
> - return -ENOMEM;
> + goto err_alloc;
> }
>
> interrupt_data->interrupt_counts = kcalloc(num_interrupts,
> sizeof(ulong),
> GFP_KERNEL);
> if (!interrupt_data->interrupt_counts) {
> - kfree(interrupt_data->eventfd_ctxs);
> - kfree(interrupt_data->msix_entries);
> - kfree(interrupt_data);
> - return -ENOMEM;
> + goto err_alloc;
> }
>
> switch (interrupt_data->type) {
> @@ -402,6 +396,12 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev, const char *name,
> interrupt_sysfs_attrs);
>
> return 0;
> +
> +err_alloc:
> + kfree(interrupt_data);
> + kfree(interrupt_data->msix_entries);
> + kfree(interrupt_data->eventfd_ctxs);
> + return -ENOMEM;
> }
>
> static void
> --
> 2.19.1
>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181029140731.GA18826%40kvasir.local.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: gasket Create a memory allocation path
2018-10-29 14:07 [PATCH] staging: gasket Create a memory allocation path Ioannis Valasakis
2018-10-29 14:09 ` [Outreachy kernel] " Vaishali Thakkar
@ 2018-10-29 14:14 ` Julia Lawall
2018-10-29 21:31 ` Sasha Levin
2018-10-29 14:31 ` Greg KH
2 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2018-10-29 14:14 UTC (permalink / raw)
To: Ioannis Valasakis; +Cc: outreachy-kernel, gregkh, rspringer
> +err_alloc:
> + kfree(interrupt_data);
> + kfree(interrupt_data->msix_entries);
> + kfree(interrupt_data->eventfd_ctxs);
> + return -ENOMEM;
Besides Vaishali's suggestion, could you also make specific tags for
specific cases? Please first read this message from Dan Carpenter:
https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
julia
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: gasket Create a memory allocation path
2018-10-29 14:14 ` Julia Lawall
@ 2018-10-29 21:31 ` Sasha Levin
0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2018-10-29 21:31 UTC (permalink / raw)
To: Julia Lawall; +Cc: Ioannis Valasakis, outreachy-kernel, gregkh, rspringer
On Mon, Oct 29, 2018 at 03:14:56PM +0100, Julia Lawall wrote:
>> +err_alloc:
>> + kfree(interrupt_data);
>> + kfree(interrupt_data->msix_entries);
>> + kfree(interrupt_data->eventfd_ctxs);
>> + return -ENOMEM;
>
>Besides Vaishali's suggestion, could you also make specific tags for
>specific cases? Please first read this message from Dan Carpenter:
>
>https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
You'll also notice that you made the very same mistake Dan describes in
his post :)
You first freed interrupt_data, and proceeded to try and free
interrupt_data->msix_entries, which will attempt to dereference
interrupt_data which you freed the line before.
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: gasket Create a memory allocation path
2018-10-29 14:07 [PATCH] staging: gasket Create a memory allocation path Ioannis Valasakis
2018-10-29 14:09 ` [Outreachy kernel] " Vaishali Thakkar
2018-10-29 14:14 ` Julia Lawall
@ 2018-10-29 14:31 ` Greg KH
2018-10-30 13:41 ` Ioannis Valasakis
2 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2018-10-29 14:31 UTC (permalink / raw)
To: Ioannis Valasakis; +Cc: outreachy-kernel, rspringer
On Mon, Oct 29, 2018 at 02:07:31PM +0000, Ioannis Valasakis wrote:
> +err_alloc:
> + kfree(interrupt_data);
> + kfree(interrupt_data->msix_entries);
> + kfree(interrupt_data->eventfd_ctxs);
Think about the three lines here, and try to find the bug :)
This is not going to work at all, sorry, please look up what kfree()
does.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: gasket Create a memory allocation path
2018-10-29 14:31 ` Greg KH
@ 2018-10-30 13:41 ` Ioannis Valasakis
0 siblings, 0 replies; 6+ messages in thread
From: Ioannis Valasakis @ 2018-10-30 13:41 UTC (permalink / raw)
To: Greg KH; +Cc: outreachy-kernel
On Mon, Oct 29, 2018 at 03:31:16PM +0100, Greg KH wrote:
> On Mon, Oct 29, 2018 at 02:07:31PM +0000, Ioannis Valasakis wrote:
> > +err_alloc:
> > + kfree(interrupt_data);
> > + kfree(interrupt_data->msix_entries);
> > + kfree(interrupt_data->eventfd_ctxs);
>
> Think about the three lines here, and try to find the bug :)
Right, I'll check this. The article from Dan was very useful. Thanks
Julia, Greg and Sasha!
> This is not going to work at all, sorry, please look up what kfree()
> does.
>
> thanks,
>
> greg k-h
I am going to resubmit this when corrected.
ta
ioannis
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-30 13:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-29 14:07 [PATCH] staging: gasket Create a memory allocation path Ioannis Valasakis
2018-10-29 14:09 ` [Outreachy kernel] " Vaishali Thakkar
2018-10-29 14:14 ` Julia Lawall
2018-10-29 21:31 ` Sasha Levin
2018-10-29 14:31 ` Greg KH
2018-10-30 13:41 ` Ioannis Valasakis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.