* [PATCH] staging: netlogic: Return zero pointer after failed kmalloc @ 2016-02-18 17:38 Laura Garcia Liebana 2016-02-23 20:42 ` [Outreachy kernel] " Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Laura Garcia Liebana @ 2016-02-18 17:38 UTC (permalink / raw) To: outreachy-kernel Return a ZERO_SIZE_PTR in the xlr_config_spill function if the kmalloc returns an invalid value. This change prevents a possible segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> --- drivers/staging/netlogic/xlr_net.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c index 98e74d7..0015847 100644 --- a/drivers/staging/netlogic/xlr_net.c +++ b/drivers/staging/netlogic/xlr_net.c @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, base = priv->base_addr; spill_size = size; spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); - if (!spill) + if (!spill) { pr_err("Unable to allocate memory for spill area!\n"); + return ZERO_SIZE_PTR; + } spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); phys_addr = virt_to_phys(spill); -- 2.7.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: netlogic: Return zero pointer after failed kmalloc 2016-02-18 17:38 [PATCH] staging: netlogic: Return zero pointer after failed kmalloc Laura Garcia Liebana @ 2016-02-23 20:42 ` Julia Lawall 2016-02-23 22:36 ` Laura Garcia 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2016-02-23 20:42 UTC (permalink / raw) To: Laura Garcia Liebana; +Cc: outreachy-kernel On Thu, 18 Feb 2016, Laura Garcia Liebana wrote: > Return a ZERO_SIZE_PTR in the xlr_config_spill function if the > kmalloc returns an invalid value. This change prevents a possible > segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > --- > drivers/staging/netlogic/xlr_net.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c > index 98e74d7..0015847 100644 > --- a/drivers/staging/netlogic/xlr_net.c > +++ b/drivers/staging/netlogic/xlr_net.c > @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, > base = priv->base_addr; > spill_size = size; > spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); > - if (!spill) > + if (!spill) { > pr_err("Unable to allocate memory for spill area!\n"); > + return ZERO_SIZE_PTR; Why did you choose this as the return value, rather than NULL? Is there a way that the first argument to kmalloc can be 0? Also, the containing function, xlr_config_spill, is called in a number of places where the results is stored in a structure field with no error checking. Have you traced through the code to see if it is appropriate to store and invalid pointer in these fields? At the moment, the crash will at least happen near the location of the faulty assignment. It could get harder to debug, if the bad value will propagate through structure fields to other parts of the driver. julia > + } > > spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); > phys_addr = virt_to_phys(spill); > -- > 2.7.0 > > -- > 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/20160218173839.GA9157%40sonyv. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: netlogic: Return zero pointer after failed kmalloc 2016-02-23 20:42 ` [Outreachy kernel] " Julia Lawall @ 2016-02-23 22:36 ` Laura Garcia 2016-02-24 9:23 ` Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Laura Garcia @ 2016-02-23 22:36 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel On Tue, Feb 23, 2016 at 09:42:04PM +0100, Julia Lawall wrote: > > > On Thu, 18 Feb 2016, Laura Garcia Liebana wrote: > > > Return a ZERO_SIZE_PTR in the xlr_config_spill function if the > > kmalloc returns an invalid value. This change prevents a possible > > segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. > > > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > > --- > > drivers/staging/netlogic/xlr_net.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c > > index 98e74d7..0015847 100644 > > --- a/drivers/staging/netlogic/xlr_net.c > > +++ b/drivers/staging/netlogic/xlr_net.c > > @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, > > base = priv->base_addr; > > spill_size = size; > > spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); > > - if (!spill) > > + if (!spill) { > > pr_err("Unable to allocate memory for spill area!\n"); > > + return ZERO_SIZE_PTR; > > Why did you choose this as the return value, rather than NULL? Is there a > way that the first argument to kmalloc can be 0? Also, the containing > function, xlr_config_spill, is called in a number of places where the > results is stored in a structure field with no error checking. Have you > traced through the code to see if it is appropriate to store and invalid > pointer in these fields? At the moment, the crash will at least happen > near the location of the faulty assignment. It could get harder to debug, > if the bad value will propagate through structure fields to other parts of > the driver. > > julia > > > + } > > > > spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); > > phys_addr = virt_to_phys(spill); > > -- > > 2.7.0 > > > > -- > > 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/20160218173839.GA9157%40sonyv. > > For more options, visit https://groups.google.com/d/optout. > > I guessed that it was the best way to return an invalid pointer after a kmalloc. Doesn't seem that the structure where the function return is stored is called anymore. So I believe that returning a NULL will be safe in this case. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: netlogic: Return zero pointer after failed kmalloc 2016-02-23 22:36 ` Laura Garcia @ 2016-02-24 9:23 ` Julia Lawall 2016-02-24 16:47 ` Laura Garcia 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2016-02-24 9:23 UTC (permalink / raw) To: Laura Garcia; +Cc: outreachy-kernel On Tue, 23 Feb 2016, Laura Garcia wrote: > On Tue, Feb 23, 2016 at 09:42:04PM +0100, Julia Lawall wrote: > > > > > > On Thu, 18 Feb 2016, Laura Garcia Liebana wrote: > > > > > Return a ZERO_SIZE_PTR in the xlr_config_spill function if the > > > kmalloc returns an invalid value. This change prevents a possible > > > segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. > > > > > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > > > --- > > > drivers/staging/netlogic/xlr_net.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c > > > index 98e74d7..0015847 100644 > > > --- a/drivers/staging/netlogic/xlr_net.c > > > +++ b/drivers/staging/netlogic/xlr_net.c > > > @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, > > > base = priv->base_addr; > > > spill_size = size; > > > spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); > > > - if (!spill) > > > + if (!spill) { > > > pr_err("Unable to allocate memory for spill area!\n"); > > > + return ZERO_SIZE_PTR; > > > > Why did you choose this as the return value, rather than NULL? Is there a > > way that the first argument to kmalloc can be 0? Also, the containing > > function, xlr_config_spill, is called in a number of places where the > > results is stored in a structure field with no error checking. Have you > > traced through the code to see if it is appropriate to store and invalid > > pointer in these fields? At the moment, the crash will at least happen > > near the location of the faulty assignment. It could get harder to debug, > > if the bad value will propagate through structure fields to other parts of > > the driver. > > > > julia > > > > > + } > > > > > > spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); > > > phys_addr = virt_to_phys(spill); > > > -- > > > 2.7.0 > > > > > > -- > > > 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/20160218173839.GA9157%40sonyv. > > > For more options, visit https://groups.google.com/d/optout. > > > > I guessed that it was the best way to return an invalid pointer after a > kmalloc. Doesn't seem that the structure where the function return is > stored is called anymore. So I believe that returning a NULL will be > safe in this case. I'm not sure to understand the second sentence. Is the kmalloc useless? To my recollection, the result of the function is stored in several structure fields. Are these structure fields never used? Also, when you reply to a comment, could you put your reply just under the comment, with blank lines around it? Otherwise, the reply is hard to find. thanks, julia > > -- > 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/20160223223647.GA4772%40sonyv. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: netlogic: Return zero pointer after failed kmalloc 2016-02-24 9:23 ` Julia Lawall @ 2016-02-24 16:47 ` Laura Garcia 2016-02-24 16:55 ` Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Laura Garcia @ 2016-02-24 16:47 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel On Wed, Feb 24, 2016 at 10:23:09AM +0100, Julia Lawall wrote: > > > On Tue, 23 Feb 2016, Laura Garcia wrote: > > > On Tue, Feb 23, 2016 at 09:42:04PM +0100, Julia Lawall wrote: > > > > > > > > > On Thu, 18 Feb 2016, Laura Garcia Liebana wrote: > > > > > > > Return a ZERO_SIZE_PTR in the xlr_config_spill function if the > > > > kmalloc returns an invalid value. This change prevents a possible > > > > segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. > > > > > > > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > > > > --- > > > > drivers/staging/netlogic/xlr_net.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c > > > > index 98e74d7..0015847 100644 > > > > --- a/drivers/staging/netlogic/xlr_net.c > > > > +++ b/drivers/staging/netlogic/xlr_net.c > > > > @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, > > > > base = priv->base_addr; > > > > spill_size = size; > > > > spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); > > > > - if (!spill) > > > > + if (!spill) { > > > > pr_err("Unable to allocate memory for spill area!\n"); > > > > + return ZERO_SIZE_PTR; > > > > > > Why did you choose this as the return value, rather than NULL? Is there a > > > way that the first argument to kmalloc can be 0? Also, the containing > > > function, xlr_config_spill, is called in a number of places where the > > > results is stored in a structure field with no error checking. Have you > > > traced through the code to see if it is appropriate to store and invalid > > > pointer in these fields? At the moment, the crash will at least happen > > > near the location of the faulty assignment. It could get harder to debug, > > > if the bad value will propagate through structure fields to other parts of > > > the driver. > > > > > > julia > > > > > > > + } > > > > > > > > spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); > > > > phys_addr = virt_to_phys(spill); > > > > -- > > > > 2.7.0 > > > > > > > > -- > > > > 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/20160218173839.GA9157%40sonyv. > > > > For more options, visit https://groups.google.com/d/optout. > > > > > > > I guessed that it was the best way to return an invalid pointer after a > > kmalloc. Doesn't seem that the structure where the function return is > > stored is called anymore. So I believe that returning a NULL will be > > safe in this case. > > I'm not sure to understand the second sentence. Is the kmalloc useless? > To my recollection, the result of the function is stored in several > structure fields. Are these structure fields never used? > Exactly, these structure fields seem that are never used. > Also, when you reply to a comment, could you put your reply just under the > comment, with blank lines around it? Otherwise, the reply is hard to > find. > Ok, got it. thanks. > thanks, > julia > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: netlogic: Return zero pointer after failed kmalloc 2016-02-24 16:47 ` Laura Garcia @ 2016-02-24 16:55 ` Julia Lawall 0 siblings, 0 replies; 6+ messages in thread From: Julia Lawall @ 2016-02-24 16:55 UTC (permalink / raw) To: Laura Garcia; +Cc: outreachy-kernel On Wed, 24 Feb 2016, Laura Garcia wrote: > On Wed, Feb 24, 2016 at 10:23:09AM +0100, Julia Lawall wrote: > > > > > > On Tue, 23 Feb 2016, Laura Garcia wrote: > > > > > On Tue, Feb 23, 2016 at 09:42:04PM +0100, Julia Lawall wrote: > > > > > > > > > > > > On Thu, 18 Feb 2016, Laura Garcia Liebana wrote: > > > > > > > > > Return a ZERO_SIZE_PTR in the xlr_config_spill function if the > > > > > kmalloc returns an invalid value. This change prevents a possible > > > > > segmentation fault as the invalid pointer is fed into PTR_ALIGN macro. > > > > > > > > > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com> > > > > > --- > > > > > drivers/staging/netlogic/xlr_net.c | 4 +++- > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c > > > > > index 98e74d7..0015847 100644 > > > > > --- a/drivers/staging/netlogic/xlr_net.c > > > > > +++ b/drivers/staging/netlogic/xlr_net.c > > > > > @@ -437,8 +437,10 @@ static void *xlr_config_spill(struct xlr_net_priv *priv, int reg_start_0, > > > > > base = priv->base_addr; > > > > > spill_size = size; > > > > > spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC); > > > > > - if (!spill) > > > > > + if (!spill) { > > > > > pr_err("Unable to allocate memory for spill area!\n"); > > > > > + return ZERO_SIZE_PTR; > > > > > > > > Why did you choose this as the return value, rather than NULL? Is there a > > > > way that the first argument to kmalloc can be 0? Also, the containing > > > > function, xlr_config_spill, is called in a number of places where the > > > > results is stored in a structure field with no error checking. Have you > > > > traced through the code to see if it is appropriate to store and invalid > > > > pointer in these fields? At the moment, the crash will at least happen > > > > near the location of the faulty assignment. It could get harder to debug, > > > > if the bad value will propagate through structure fields to other parts of > > > > the driver. > > > > > > > > julia > > > > > > > > > + } > > > > > > > > > > spill = PTR_ALIGN(spill, SMP_CACHE_BYTES); > > > > > phys_addr = virt_to_phys(spill); > > > > > -- > > > > > 2.7.0 > > > > > > > > > > -- > > > > > 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/20160218173839.GA9157%40sonyv. > > > > > For more options, visit https://groups.google.com/d/optout. > > > > > > > > > > I guessed that it was the best way to return an invalid pointer after a > > > kmalloc. Doesn't seem that the structure where the function return is > > > stored is called anymore. So I believe that returning a NULL will be > > > safe in this case. > > > > I'm not sure to understand the second sentence. Is the kmalloc useless? > > To my recollection, the result of the function is stored in several > > structure fields. Are these structure fields never used? > > > > Exactly, these structure fields seem that are never used. Then maybe the whole function is useless? It would seem to be something to study carefully, and then perhaps ask the maintainers for confirmation. julia > > Also, when you reply to a comment, could you put your reply just under the > > comment, with blank lines around it? Otherwise, the reply is hard to > > find. > > > > Ok, got it. > > thanks. > > > thanks, > > julia > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-24 16:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-18 17:38 [PATCH] staging: netlogic: Return zero pointer after failed kmalloc Laura Garcia Liebana 2016-02-23 20:42 ` [Outreachy kernel] " Julia Lawall 2016-02-23 22:36 ` Laura Garcia 2016-02-24 9:23 ` Julia Lawall 2016-02-24 16:47 ` Laura Garcia 2016-02-24 16:55 ` Julia Lawall
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.