On Fri, Jul 05, 2013 at 03:55:22PM +0200, Pablo Neira Ayuso wrote:
> 
> Looks good, thanks.

Thanks, thanks a lot for the speedy fix-up of things. Do you follow
coverity reports for netfilter too?

> 
> Should not happen either, but added the close as you suggested.

OSMO_ASSERT(link->accept_cb) maybe? Coverity is still not happy
about the ret (mostly because it doesn't find an assignment but
then I would probably need to build every project as one thing).

E.g. we don't check the return value of accept_cb, but if you
want to, I can close this as a false positive now.


> I don't have any msidn card. It seems we don't have any ->close
> callback in the line set to close that socket, but I prefer to leave
> as is by now until I/someone else can confirm this.

I think 'sk' is only used to gain information about the mISDN device
(it is a bit racy, as at the time we use it the card might be gone,
I assume we can just close the sk after the last ioctl).

> Those strings are set in the configuration path, I have fix it, no
> such an "elevated risk" as coverity spotted.

thanks.

> Let me know if you have any issue with those.

Coverity found another thing (so apparently it 'learns')


src/input/dahdi.c

404        if (line->port_nr > ARRAY_SIZE(span_cfgs))
405                return;
406
        
CID 1042368 (#1 of 1): Out-of-bounds read (OVERRUN)
3. overrun-local: Overrunning array "span_cfgs" of 128 4-byte elements at 
element index 128 (byte offset 512) using index "line->port_nr" (which 
evaluates to 128).
407        scfg = span_cfgs[line->port_nr];

So I think this needs to be a >=. Please use CID in the commit message
when fixing it (or in case you are busy and ack that >= is the right
fix I will make the commit).


holger

Reply via email to