From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/4097 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Static analyzers results on musl Date: Fri, 4 Oct 2013 16:21:58 -0400 Message-ID: <20131004202158.GM20515@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 1380918127 317 80.91.229.3 (4 Oct 2013 20:22:07 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 4 Oct 2013 20:22:07 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-4101-gllmg-musl=m.gmane.org@lists.openwall.com Fri Oct 04 22:22:12 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 1VSBt6-00052w-KX for gllmg-musl@plane.gmane.org; Fri, 04 Oct 2013 22:22:12 +0200 Original-Received: (qmail 11588 invoked by uid 550); 4 Oct 2013 20:22:11 -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 11580 invoked from network); 4 Oct 2013 20:22:11 -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:4097 Archived-At: On Fri, Oct 04, 2013 at 09:51:25PM +0400, Alexander Monakov wrote: > Hello, > > >From reading recent archives, it appeared to me there was some interest in > applying source code analysis tools to musl. My co-workers helped me run a > couple of tools on musl, so here are the results. > > Szabolcs kindly assisted with hosting Clang Analyzer results at > > http://port70.net/~nsz/musl/clang-2013-10-04/ > > The analyzer was run on today's sources (commit 38a0a4d). The build with > make -j4 was interrupted at some point during building PIC objects; I presume > at that point all non-PIC code was built, and the analyzer saw all source > code, except maybe some #ifdef SHARED sections. > > My take on those: > - 2 sizeof mismatch warnings make sense Indeed, I think these are bugs, but it's likely they don't matter because the allocations are larger than needed rather than smaller. > - 19+1 "dead code" warnings are helpful Yes. > - "Out-of-bound array access" in glob.c appears to be a false positive (?) I'll need to look closer at this. It might be a real issue. > - 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 > - I have not attempted to investigate "dereference of null" warnings Some of these look like they might be valid errors. > 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 Definitely a bug. Fixing it. > getspnam_r.c I wonder if there's a window between opening the file and > pthread_cleanup_push where the handle would leak? (this is not what the tool > flagged) No, there are no calls to cancellation points in that interval. > vfprintf.c:664 > vfwprint.c:354 va_end not called on error return path There are several cases of this in other places too. It has no practical consequence, since the only possible implementation of va_end is a no-op, but it should be fixed to make the code formally correct. > regcomp.c:767 > regcomp.c:807 sizeof mismatch; don't know why not flagged by clang Presumably because it's using a custom allocation function clang does not know about. > getifaddrs.c:92 the code trusts the kernel that the fifth token would not be > longer than IFNAMSIZ :) This is an interesting theoretical issue we should probably adopt a policy on. Obviously you have to trust the kernel to _some_ extent, but there may be instances where it makes sense to validate data from the kernel. > There are a few warnings that return value of .*nl_langinfo.* is not checked > for NULL before use; presumably that is by design. nl_langinfo is not permitted to return NULL, so this warning makes no sense. Rich