linux-kernel - Re: [PATCH v11 3/4] add FPGA manager core

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] [day] [month] [year] [list]

Message-ID: <20150925100036.GM4953@mwanda>
Date:	Fri, 25 Sep 2015 13:00:36 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	atull <atull@...nsource.altera.com>
Cc:	mark.rutland@....com, linux-doc@...r.kernel.org,
	linus.walleij@...aro.org, pantelis.antoniou@...sulko.com,
	hpa@...or.com, s.trumtrar@...gutronix.de,
	devel@...verdev.osuosl.org, sameo@...ux.intel.com, nico@...aro.org,
	iws@...o.caltech.edu, michal.simek@...inx.com, kyle.teske@...com,
	jgunthorpe@...idianresearch.com, grant.likely@...aro.org,
	davidb@...eaurora.org, rubini@...dd.com, cesarb@...arb.net,
	devicetree@...r.kernel.org, jason@...edaemon.net,
	pawel.moll@....com, ijc+devicetree@...lion.org.uk,
	Josh Cartwright <joshc@...com>, broonie@...nel.org,
	gregkh@...uxfoundation.org, philip@...ister.org,
	Petr Cvek <petr.cvek@....cz>, dinguyen@...nsource.altera.com,
	monstr@...str.eu, Pavel Machek <pavel@...x.de>,
	linux-kernel@...r.kernel.org, balbi@...com,
	delicious.quinoa@...il.com, robh+dt@...nel.org, rob@...dley.net,
	galak@...eaurora.org, akpm@...ux-foundation.org,
	davem@...emloft.net, m.chehab@...sung.com
Subject: Re: [PATCH v11 3/4] add FPGA manager core

On Thu, Sep 24, 2015 at 03:47:26PM -0500, atull wrote:
> Interesting.  The amount of code bloat here compiles down to about two
> machine instructions (in two places).  Actually a little more since I should
> be using IS_ERR_OR_NULL.  But the main question is whether I should do
> it at all.
> 

They kernel already has too many bogus checks for IS_ERR().  It's a very
common bug to check for IS_ERR() when you should be checking for NULL.

-	foo = some_allocator();
+	foo = kmalloc();
	if (IS_ERR(foo))
		goto fail;

I have a static checker for "warn: 'foo' isn't an ERR_PTR" but I haven't
published it because too much code has impossible checks.

> The behaviour I should drive here is that the user will do their own error
> checking.  After they get a pointer to a FPGA manager using
> of_fpga_mgr_get(), they should check it and not assume that
> fpga_mgr_firmware_load() will do it for them, i.e.
> 
> 	mgr = of_fpga_mgr_get(mgr_node);
> 	if (IS_ERR(mgr))
> 		return PTR_ERR(mgr);
> 	fpga_mgr_firmware_load(mgr, flags, path);
> 

I don't understand completely how of_fpga_mgr_get() ever returns NULL.
A lot of the of_ functions return ERR_PTRs if OF_ is compiled in but
they return NULL if it's not.  I think this is so people can build with
COMPILE_TEST so we get more coverage with static analysis?

> I could take out these NULL pointer checks and it won't hurt anything unless
> someone is just using the functions badly, in which case: kablooey.

Linux devs are very good about doing error checking.  An early kablooey
is what we want for people who don't.

Also if you provide a sanity check then Markus Elfring will remove all
the error checking in the callers.  Don't do it.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/