linux-kernel - Re: [PATCH] kernel/resource: Invalid memory access in __release

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: <20150421065927.GA7379@ulmo.nvidia.com>
Date:	Tue, 21 Apr 2015 08:59:29 +0200
From:	Thierry Reding <treding@...dia.com>
To:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
CC:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vivek Goyal <vgoyal@...hat.com>,
	Vinod Koul <vinod.koul@...el.com>, Cliff Wickman <cpw@....com>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Jakub Sitnicki <jsitnicki@...il.com>,
	"Mike Travis" <travis@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>
Subject: Re: [PATCH] kernel/resource: Invalid memory access in
 __release_resource

On Mon, Apr 20, 2015 at 10:49:28PM +0200, Ricardo Ribalda Delgado wrote:
> Hello
> 
> On Mon, Apr 20, 2015 at 10:36 PM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> 
> >> > From reading drivers/base/platform.c, it looks like the intent is
> >> > that platform device users would use these interfaces:
> >>
> >> I can take a look to modify OF to use insert_resource(), but I still
> >> think that no matter what, we should add this extra check, like the
> >> propossed patch or maybe with a BUG_ON()....
> >
> > I think it would be nicer to make OF use platform_device_add_resources()
> > and platform_device_add() because then there's less duplication of code.
> > But Grant might have had a reason for avoiding that.
> >
> > Bjorn
> 
> I think I am going to make two patches, one  modifying OF as you
> suggest, and another one
> adding a BUG_ON to release_resource. Then you can decide to apply one
> or two with the feedback from Grant

I don't see the point in using BUG_ON() here. That's going to crash the
system anyway, so you could just as well let it crash while it's trying
to dereference the parent pointer. Maybe a WARN_ON() along with an
appropriate error code would be better here.

As to the underlying problem, perhaps of_device_add() should be calling
platform_device_add() rather than device_add()? Essentially the OF glue
creates regular platform devices, so I think it should be reusing as
much of the platform code as possible. From a quick look that might be
somewhat hairy to do, but on the other hand these kinds of problems are
bound to happen over and over again because both implementations of
platform devices need to be manually kept in sync.

Thierry

Content of type "application/pgp-signature" skipped