[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 10 Oct 2013 12:06:44 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Static analyzers results on musl
On Fri, Oct 04, 2013 at 09:51:25PM +0400, Alexander Monakov wrote:
> - 2 sizeof mismatch warnings make sense
These have been fixed (as well as at least one other that was not
caught).
> - 19+1 "dead code" warnings are helpful
I think some of these have been fixed, but they are low priority.
> - "Out-of-bound array access" in glob.c appears to be a false positive (?)
At first I thought this was possibly a real overflow, but it seems to
be caused by the invalid use of [1] instead of [] for a flexible array
member in struct match. That's a bug in itself, so I'll look into
fixing it, but need to be careful not to mess up the allocation size
logic at the same time.
> - There are many "garbage"/"undefined" warnings where the variable in
> question is passed to a syscall by reference and expected to be initialized
> there, unless error is signalled; it's quite unfortunate to have many false
> positives like that
At least one of these seems to be a valid error:
http://port70.net/~nsz/musl/clang-2013-10-04/report-c1ebd3.html#EndPath
Unless the compiler takes advantage of the fact that accessing
indeterminate values is not valid, this one should have zero impact,
but needs to be fixed. I think reordering the operands of && would fix
it.
This one is also semi-valid:
http://port70.net/~nsz/musl/clang-2013-10-04/report-edc7bc.html#EndPath
But the code path it's taken is where the application has provided an
invalid stack address for the new thread, such that after aligning it
mod 16 and subtracting off __pthread_tsd_size, the resulting address
is null. However, valid pointer arithmetic can never result in a null
pointer, so I think this is actually a bug in clang's static analysis.
Please let me know if this analysis seems wrong.
> - I have not attempted to investigate "dereference of null" warnings
The ones in regex are pretty complex and I'm still unclear on whether
the code paths flagged by the analysis are actually possible. It
doesn't help that this is third-party code. As for wordexp, I need to
look again; it looked to me like the null pointer dereference path
might occur when there are errors communicating with the child
process.
> I also have results from another static analysis tool developed internally
> were I work. Here's a few hand-picked additional warnings. I ran the tool
> without updating git first, so the tree was from September 9 (commit ff4be70).
> Sorry about that.
>
> setenv.c:21 malloc return value not checked
Fixed.
> vfprintf.c:664
> vfwprint.c:354 va_end not called on error return path
Fixed.
> regcomp.c:767
> regcomp.c:807 sizeof mismatch; don't know why not flagged by clang
Fixed.
> getifaddrs.c:92 the code trusts the kernel that the fifth token would not be
> longer than IFNAMSIZ :)
Still pending whether we should consider this case.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
