Re: [PATCH] kernel/resource: Invalid memory access in __release_resource

[Thierry Reding]

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@xxxxxxxxxx> 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

Attachment: pgp8Ax9c4h8z5.pgp
Description: PGP signature