netdev - Re: net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev

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: <CAB=NE6VUiWoiTe+uM=0NrhPx3fVDwXYaAKSZjzK3wT8sh7cazw@mail.gmail.com>
Date:	Tue, 2 Dec 2014 13:45:26 -0500
From:	"Luis R. Rodriguez" <mcgrof@...e.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	Julia Lawall <julia.lawall@...6.fr>,
	SF Markus Elfring <elfring@...rs.sourceforge.net>,
	Lino Sanfilippo <LinoSanfilippo@....de>,
	Olof Johansson <olof@...om.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"backports@...r.kernel.org" <backports@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: Re: net-PA Semi: Deletion of unnecessary checks before the function
 call "pci_dev_put"

On Tue, Dec 2, 2014 at 11:53 AM, Johannes Berg
<johannes@...solutions.net> wrote:
> On Mon, 2014-12-01 at 21:34 +0100, Julia Lawall wrote:
>
>> > So this kind of evolution is no problem for the (automated) backports
>> > using the backports project - although it can be difficult to detect
>> > such a thing is needed.
>>
>> That is exactly the problem...
>
> I'm not convinced though that it should stop such progress in mainline.

I believe this case requires a bit more information explained as to
why it was explained. The "form" of change this patch has is of the
type that can crash systems if the NULL pointer check on the caller
implementation was only added later. We might be able to grammatically
check for this situation in the future if we had a white list / black
list / kernel revision where the NULL check was added but for now we
don't have that and as such care is just required on the developer in
consideration for backports.

It should be up to the maintainer to appreciate the gains of doing
something differently to make it easier for backporting. I obviously
think its a good thing to consider, its extra work though, so only if
the maintainer has some appreciation for backporting would this make
sense.

In this particular case I've reviewed Julia's concern and I've
determined that the patch is safe up to at least v2.6.12-rc2 (which is
where our git history begins on Linus' tree), this is because the
check for NULL has been there since then:

git show 1da177e drivers/pci/pci-driver.c

+void pci_dev_put(struct pci_dev *dev)
+{
+       if (dev)
+               put_device(&dev->dev);
+}

So this type of wide collateral evolution should not cause panics.
Because of this:

Acked-by: Luis R. Rodriguez <mcgrof@...e.com>

But note -- I still think its only good for us to vet these, if we
can't why not? If the maintainer doesn't give a shit that's different,
but if there are folks out there willing to help with vetting then
well, why not :)

PS. Including something like historical vetting as I did above on the
commit log should help folks.

 Luis
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html