* [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() @ 2015-03-02 11:09 Pawel Wodkowski 2015-03-02 12:23 ` David Marchand 0 siblings, 1 reply; 7+ messages in thread From: Pawel Wodkowski @ 2015-03-02 11:09 UTC (permalink / raw) To: dev On failure devargs->args should not be accesed if devargs is NULL. Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> --- lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index 9b110f7..615945e 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) return 0; fail: - if (devargs->args) - free(devargs->args); if (buf) free(buf); - if (devargs) + if (devargs) { + free(devargs->args); free(devargs); + } + return -1; } -- 1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 11:09 [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() Pawel Wodkowski @ 2015-03-02 12:23 ` David Marchand 2015-03-02 14:40 ` Wiles, Keith 2015-03-02 18:39 ` Thomas Monjalon 0 siblings, 2 replies; 7+ messages in thread From: David Marchand @ 2015-03-02 12:23 UTC (permalink / raw) To: Pawel Wodkowski; +Cc: dev Hello Pawel, On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski <pawelx.wodkowski@intel.com > wrote: > On failure devargs->args should not be accesed if devargs is NULL. > accessed. > > Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> > --- > lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_devargs.c > b/lib/librte_eal/common/eal_common_devargs.c > index 9b110f7..615945e 100644 > --- a/lib/librte_eal/common/eal_common_devargs.c > +++ b/lib/librte_eal/common/eal_common_devargs.c > @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const > char *devargs_str) > return 0; > > fail: > - if (devargs->args) > - free(devargs->args); > if (buf) > free(buf); > - if (devargs) > + if (devargs) { > + free(devargs->args); > free(devargs); > + } > + > return -1; > } > > Fixes: c07691ae1089 ("devargs: remove limit on parameters length") Acked-by: David Marchand <david.marchand@6wind.com> -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 12:23 ` David Marchand @ 2015-03-02 14:40 ` Wiles, Keith 2015-03-02 14:55 ` Pawel Wodkowski 2015-03-02 18:39 ` Thomas Monjalon 1 sibling, 1 reply; 7+ messages in thread From: Wiles, Keith @ 2015-03-02 14:40 UTC (permalink / raw) To: David Marchand, Wodkowski, PawelX; +Cc: dev On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote: >Hello Pawel, > >On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski ><pawelx.wodkowski@intel.com >> wrote: > >> On failure devargs->args should not be accesed if devargs is NULL. >> > >accessed. > > >> >> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> >> --- >> lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/lib/librte_eal/common/eal_common_devargs.c >> b/lib/librte_eal/common/eal_common_devargs.c >> index 9b110f7..615945e 100644 >> --- a/lib/librte_eal/common/eal_common_devargs.c >> +++ b/lib/librte_eal/common/eal_common_devargs.c >> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, >>const >> char *devargs_str) >> return 0; >> >> fail: >> - if (devargs->args) >> - free(devargs->args); >> if (buf) >> free(buf); >> - if (devargs) >> + if (devargs) { >> + free(devargs->args); Do you not still need to check for args being NULL before calling free? >> free(devargs); >> + } >> + >> return -1; >> } >> >> >Fixes: c07691ae1089 ("devargs: remove limit on parameters length") >Acked-by: David Marchand <david.marchand@6wind.com> > >-- >David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 14:40 ` Wiles, Keith @ 2015-03-02 14:55 ` Pawel Wodkowski 2015-03-02 16:47 ` Wiles, Keith 0 siblings, 1 reply; 7+ messages in thread From: Pawel Wodkowski @ 2015-03-02 14:55 UTC (permalink / raw) To: Wiles, Keith, David Marchand; +Cc: dev On 2015-03-02 15:40, Wiles, Keith wrote: > > > On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote: > >> Hello Pawel, >> >> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski >> <pawelx.wodkowski@intel.com >>> wrote: >> >>> On failure devargs->args should not be accesed if devargs is NULL. >>> >> >> accessed. >> >> >>> >>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> >>> --- >>> lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/lib/librte_eal/common/eal_common_devargs.c >>> b/lib/librte_eal/common/eal_common_devargs.c >>> index 9b110f7..615945e 100644 >>> --- a/lib/librte_eal/common/eal_common_devargs.c >>> +++ b/lib/librte_eal/common/eal_common_devargs.c >>> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, >>> const >>> char *devargs_str) >>> return 0; >>> >>> fail: >>> - if (devargs->args) >>> - free(devargs->args); >>> if (buf) >>> free(buf); >>> - if (devargs) >>> + if (devargs) { >>> + free(devargs->args); > > Do you not still need to check for args being NULL before calling free? No, there is no need for that. The same for buf. This NOP check is common practice in DPDK. I woul be good to clean this in whole library in separate patch set. I recommend to read free() doc before doing another 'if (foo != NULL) free(foo)' http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html -- Pawel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 14:55 ` Pawel Wodkowski @ 2015-03-02 16:47 ` Wiles, Keith 2015-03-02 17:35 ` Pawel Wodkowski 0 siblings, 1 reply; 7+ messages in thread From: Wiles, Keith @ 2015-03-02 16:47 UTC (permalink / raw) To: Wodkowski, PawelX, David Marchand; +Cc: dev On 3/2/15, 8:55 AM, "Wodkowski, PawelX" <pawelx.wodkowski@intel.com> wrote: >On 2015-03-02 15:40, Wiles, Keith wrote: >> >> >> On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote: >> >>> Hello Pawel, >>> >>> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski >>> <pawelx.wodkowski@intel.com >>>> wrote: >>> >>>> On failure devargs->args should not be accesed if devargs is NULL. >>>> >>> >>> accessed. >>> >>> >>>> >>>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> >>>> --- >>>> lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/lib/librte_eal/common/eal_common_devargs.c >>>> b/lib/librte_eal/common/eal_common_devargs.c >>>> index 9b110f7..615945e 100644 >>>> --- a/lib/librte_eal/common/eal_common_devargs.c >>>> +++ b/lib/librte_eal/common/eal_common_devargs.c >>>> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, >>>> const >>>> char *devargs_str) >>>> return 0; >>>> >>>> fail: >>>> - if (devargs->args) >>>> - free(devargs->args); >>>> if (buf) >>>> free(buf); >>>> - if (devargs) >>>> + if (devargs) { >>>> + free(devargs->args); >> >> Do you not still need to check for args being NULL before calling free? > >No, there is no need for that. The same for buf. This NOP check is >common practice in DPDK. I woul be good to clean this in whole library >in separate patch set. > >I recommend to read free() doc before doing another 'if (foo != NULL) >free(foo)' > >http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html OK, did not realize this was changed. Do we know if all of the OSes DPDK is built supports this free style? I know that VxWorks did not support this free() method and I did port DPDK to that OS, but it is not a supported platform for DPDK. If some OS does not support passing NULL (and is supported by DPDK) to free, then we need to abstract the free into a macro to allow those systems to work correctly. I would expect using a macro for free would also help if all frees were reworked to not test for NULL. ++Keith > >-- >Pawel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 16:47 ` Wiles, Keith @ 2015-03-02 17:35 ` Pawel Wodkowski 0 siblings, 0 replies; 7+ messages in thread From: Pawel Wodkowski @ 2015-03-02 17:35 UTC (permalink / raw) To: Wiles, Keith, David Marchand; +Cc: dev On 2015-03-02 17:47, Wiles, Keith wrote: > > > On 3/2/15, 8:55 AM, "Wodkowski, PawelX" <pawelx.wodkowski@intel.com> wrote: > >> On 2015-03-02 15:40, Wiles, Keith wrote: >>> >>> >>> On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote: >>> >>>> Hello Pawel, >>>> >>>> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski >>>> <pawelx.wodkowski@intel.com >>>>> wrote: >>>> >>>>> On failure devargs->args should not be accesed if devargs is NULL. >>>>> >>>> >>>> accessed. >>>> >>>> >>>>> >>>>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com> >>>>> --- >>>>> lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- >>>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/lib/librte_eal/common/eal_common_devargs.c >>>>> b/lib/librte_eal/common/eal_common_devargs.c >>>>> index 9b110f7..615945e 100644 >>>>> --- a/lib/librte_eal/common/eal_common_devargs.c >>>>> +++ b/lib/librte_eal/common/eal_common_devargs.c >>>>> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, >>>>> const >>>>> char *devargs_str) >>>>> return 0; >>>>> >>>>> fail: >>>>> - if (devargs->args) >>>>> - free(devargs->args); >>>>> if (buf) >>>>> free(buf); >>>>> - if (devargs) >>>>> + if (devargs) { >>>>> + free(devargs->args); >>> >>> Do you not still need to check for args being NULL before calling free? >> >> No, there is no need for that. The same for buf. This NOP check is >> common practice in DPDK. I woul be good to clean this in whole library >> in separate patch set. >> >> I recommend to read free() doc before doing another 'if (foo != NULL) >> free(foo)' >> >> http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html > > OK, did not realize this was changed. Do we know if all of the OSes DPDK > is built supports this free style? > > I know that VxWorks did not support this free() method and I did port DPDK > to that OS, but it is not a supported platform for DPDK. > > If some OS does not support passing NULL (and is supported by DPDK) to > free, then we need to abstract the free into a macro to allow those > systems to work correctly. I would expect using a macro for free would > also help if all frees were reworked to not test for NULL. > This is standard C behaviour (since ANSI C?) and VxWorks claim to be compatible with it. If they lie, why bother? > ++Keith >> >> -- >> Pawel > -- Pawel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() 2015-03-02 12:23 ` David Marchand 2015-03-02 14:40 ` Wiles, Keith @ 2015-03-02 18:39 ` Thomas Monjalon 1 sibling, 0 replies; 7+ messages in thread From: Thomas Monjalon @ 2015-03-02 18:39 UTC (permalink / raw) To: Pawel Wodkowski; +Cc: dev 2015-03-02 13:23, David Marchand: > On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski <pawelx.wodkowski@intel.com > > wrote: > > > On failure devargs->args should not be accesed if devargs is NULL. > > accessed. > > Fixes: c07691ae1089 ("devargs: remove limit on parameters length") > Acked-by: David Marchand <david.marchand@6wind.com> Applied, thanks ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-03-02 18:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-02 11:09 [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add() Pawel Wodkowski 2015-03-02 12:23 ` David Marchand 2015-03-02 14:40 ` Wiles, Keith 2015-03-02 14:55 ` Pawel Wodkowski 2015-03-02 16:47 ` Wiles, Keith 2015-03-02 17:35 ` Pawel Wodkowski 2015-03-02 18:39 ` Thomas Monjalon
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).