[libvirt] [PATCH 3/7] nwfilter_ebiptables_driver.c: avoid NULL dereference

Stefan Berger stefanb at us.ibm.com
Thu Apr 15 13:40:28 UTC 2010


Jim Meyering <jim at meyering.net> wrote on 04/15/2010 01:36:21 AM:


> "Daniel P. Berrange", libvir-list
> 
> Stefan Berger wrote:
> > libvir-list-bounces at redhat.com wrote on 04/14/2010 01:40:17 PM:
> >
> >> Please respond to "Daniel P. Berrange"
> >>
> >> On Wed, Apr 14, 2010 at 06:02:32PM +0200, Jim Meyering wrote:
> >> > From: Jim Meyering <meyering at redhat.com>
[...]

> >> >          goto tear_down_tmpebchains;
> >> >
> >> >      for (i = 0; i < nruleInstances; i++)
> >> > +        sa_assert (inst);
> >
> > Due to this statement here I get segmentation faults for which there 
is no
> > reason. I have no idea why that is but I have to deactivate this 
> line for it to
> > work again.
> > The same is not true for the statement further above...  So strange.
> 
> How is STATIC_ANALYSIS defined in config.h?
> 
>     $ grep STATIC_AN config.h
>     #define STATIC_ANALYSIS 0

Also 0 here.

> 
> If it's not 0, then you must have one of these two envvars set:
> 
>   test -n "$CCC_ANALYZER_ANALYSIS$COVERITY_BUILD_COMMAND" && echo oops

got 'oops' here.

> 
> How is sa_assert defined for you?
> 
>     $ grep -C3 sa_assert src/internal.h
>     # if STATIC_ANALYSIS
>     #  undef NDEBUG /* Don't let a prior NDEBUG definition cause 
trouble.  */
>     #  include <assert.h>
>     #  define sa_assert(expr) assert (expr)
>     # else
>     #  define sa_assert(expr) /* empty */
>     # endif
> 
> With those, the net result in your file should be that
> sa_assert is a no-op.

Yes, I agree. My understanding also.

> 
> If you're still convinced that the segfault is due to that use
> of sa_assert, please send me preprocessed output for that file, i.e.,
> 
>     cd src
>     f=nwfilter_ebiptables_driver
>     touch nwfilter/$f.c
>     la=libvirt_driver_nwfilter_la
>     lo=$la-$f.lo
>     make AM_CPPFLAGS='-E -dD' $lo
>     mv .libs/$la-$f.o $f.i
> 
> The cpp-preprocessed output is now in
> 
>     src/nwfilter_ebiptables_driver.i
> 
> You should be able to see that sa_assert expands to nothing:
> 
>     $ grep sa_assert $f.i
>     #define sa_assert(expr)

Well, not quite true. I see the lonely semicolon there that's the 
remainder from sa_assert(); -> one would have to write sa_assert(), which 
isn't nice, either...

   if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
       goto tear_down_tmpebchains;

   for (i = 0; i < nruleInstances; i++)
       ;
       switch (inst[i]->ruleType) {


Regards,
    Stefan



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100415/3b4cbb88/attachment-0001.htm>


More information about the libvir-list mailing list