From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/4118 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Static analyzers results on musl Date: Thu, 10 Oct 2013 12:06:44 -0400 Message-ID: <20131010160644.GO20515@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1381421216 26410 80.91.229.3 (10 Oct 2013 16:06:56 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 10 Oct 2013 16:06:56 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-4122-gllmg-musl=m.gmane.org@lists.openwall.com Thu Oct 10 18:07:02 2013 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1VUIlR-0003Ux-Cl for gllmg-musl@plane.gmane.org; Thu, 10 Oct 2013 18:07:01 +0200 Original-Received: (qmail 14296 invoked by uid 550); 10 Oct 2013 16:06:59 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 14276 invoked from network); 10 Oct 2013 16:06:58 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:4118 Archived-At: 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