netdev - Re: [patch] net: moxa: fix an error code

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]

Message-ID: <20160302121526.GS5273@mwanda>
Date:	Wed, 2 Mar 2016 15:15:26 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Jonas Jensen <jonas.jensen@...il.com>,
	Luis de Bethencourt <luis@...ethencourt.com>,
	françois romieu <romieu@...zoreil.com>,
	netdev@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [patch] net: moxa: fix an error code

On Wed, Mar 02, 2016 at 12:36:05PM +0100, Arnd Bergmann wrote:
> The uninitialized warning here is about a type mismatch preventing
> gcc from noticing that two conditions are the same, I'm not sure
> if this is a bug in gcc, or required by the C standard.

I wouldn't call it a bug, because everyone has to make trade offs
between how fast the program runs and how accurate it is.  And trade
offs between how ambitious your warnings are vs how many false positives
you can tolerate.

Anyway, I feel like we should just work around GCC on a case by case
basis instead of always using PTR_ERR_OR_ZERO().  The next version of
GCC will fix some false positives and introduce new ones...  Next time
using PTR_ERR_OR_ZERO() could cause warnings instead of fixing them.

Smatch works in a different way and it parse the code correctly.  But
Smatch is slow and sometimes runs out of memory and gives up trying to
parse large functions.  Smatch sees the two returns and tries to figure
out the implications of each (uninitialized vs initialized).  If you
change the code to:

	error = PTR_ERR_OR_ZERO(hash);

	if (!error)
		*leaf_out = be64_to_cpu(*(hash + index));

	return error;

then Smatch still breaks that up into two separate returns which imply
initialized vs uninitialized.

regards,
dan carpenter