linux-ext4 - Re: [PATCH] ext2/e2fsprogs: fix cppcheck warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 12 Aug 2018 20:23:28 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     ykp@...tonmail.ch
Cc:     "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext2/e2fsprogs: fix cppcheck warnings

On Sun, Aug 12, 2018 at 07:47:15PM +0000, ykp@...tonmail.ch wrote:
> From bbcbf9936c739e53327a9bee4790a167f34fc04a Mon Sep 17 00:00:00 2001
> From: Vladyslav Tsilytskyi <ykp@...tonmail.ch>
> Date: Sun, 12 Aug 2018 21:34:42 +0200
> Subject: [PATCH] Fix static analysis warnings.
> 
> Prevented file descriptor leaks in localealias.c
> "!length" code in tdb.c didn't make any sense because variable was
> uninitialized.
> Swapped blocks in dosio.c to prevent "part" variable leak if any
> return from previous block executed.

None of these changes are actually compiled on Linux.  In fact,
dosio.c is **never** compiled; it's not even mentioned in any of the
Makefiles.  The dosio.c file is one that I'm actually quite tempted to
just delete, since it's likely nt_io.c is more likely to be usable.

I'm also extremely tempted to just delete all of the intl directory.
That directory is never compiled for Linux, since the libintl library
is provided by glibc.  And it's badly out of date.  It's from
gettext-0.14.1, and the latest version of gettext is 0.19.1.  The
problem is integrating the intl sources into the e2fsprogs build
system.  It's not *hard* work per se, but it's finicky, and it's only
useful if you want to build e2fsprogs with I18N support on say,
Solaris, AIX, NetBSD, etc.  And I just don't care that much.  The more
we diverge from the intl sources from the GNU gettext page, the harder
it's going to sync up with newer versions.

Fixing these sorts of problems really isn't worth it.  That's because
I really don't like to apply super-complex patches without *very*
careful desk checking, especially if I can't test the changes easily.
And these are just resource leaks on error cases, and none of these
are likely to be serious leaks (e.g., they are one-off leaks, and not
something that would be continually leaking and causing long-running
programs to eventually run out of memory).  The argument of silencing
warnings to make it easier to spot "real" bugs is also not very
compelling for cppcheck, because the level of checking it does is very
low quality.  We can do much better using GCC -Wall, Clang warnings,
and Coverity.   So.... why?

The tdb patch does fix a real bug, although it's completely irrelevant
for Linux (since we have strdup, some we don't need the replacement).
Fortunately it's a simple enough patch that it's obviously correct
from inspection.  This is why I told you privately this was the only
fix that I considered worth it.

Cheers,

					- Ted