linux-kernel - Re: [PATCH 1/2] kernel/resource: Add new flag IORESOURCE

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: <20150609111320.F299FC40580@trevor.secretlab.ca>
Date:	Tue, 09 Jun 2015 12:13:20 +0100
From:	Grant Likely <grant.likely@...aro.org>
To:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
Cc:	Rob Herring <robherring2@...il.com>,
	Rob Herring <robh+dt@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Vivek Goyal <vgoyal@...hat.com>,
	Jakub Sitnicki <jsitnicki@...il.com>,
	Mike Travis <travis@....com>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Thierry Reding <treding@...dia.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Tejun Heo <tj@...nel.org>, Cliff Wickman <cpw@....com>
Subject: Re: [PATCH 1/2] kernel/resource: Add new flag IORESOURCE_SHARED

On Mon, 8 Jun 2015 22:02:06 +0200
, Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
 wrote:
> Hello Grant
> 
> On Mon, Jun 8, 2015 at 8:23 PM, Grant Likely <grant.likely@...aro.org> wrote:
> > On Fri,  5 Jun 2015 12:51:17 +0200
> > , Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
> >  wrote:
> >> Some device tree platforms have not defined correctly their memory
> >> resources (i.e. Overlapping or duplication of resources).
> >> To avoid this issue we have historically avoided to add their resources to
> >> the resource tree. This leads to code duplication and oops when trying to
> >> unload dynamically a device tree (feature introduced recently).
> >>
> >> This new flag tells the resource system that a resource can be shared by
> >> multiple owners, so we can support device trees with problems at the
> >> same time that we do not duplicate code or crash when unloading the
> >> device tree.
> >>
> >> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
> >> ---
> >
> > I'm really not comfortable with this change. The resource tree code is
> > complicated enough as is. Adding this exception case quite probably adds
> > corner cases that aren't property dealt with. If two regions overlay,
> > and then request_region is called? Which region does it become a child
> > of? And that's just off the top of my head. I don't want to hack in
> > changes to the resource code for what is a corner case.
> 
> I see your concern, perhaps you could provide a testcase and we can
> find out if it fails or not. So far I have tested a device tree with
> two devices on the same memory region, each device managed by a
> driver.

Actually, you need to provide the test case. You need to show that
you've thought through all the implications and corner cases on the
resource code. This is a non-trivial change to the how the resource code
works, and you need to demonstrate that your really understand the
implications of what you are doing.

Start with the example I pointed out. When a driver does a
request_mem_region(), which resource does it end up being a parent of if
the regions overlap? Can you write a unittest that demonstrates the code
has the correct behaviour? Will a driver end up getting the wrong
device's resource structure as the parent? (hint: yes it will)

> I can load and unload the device tree perfectly.

Merely making it work for your use-case isn't the issue. It's whether or
not making this change will break the core behavour of the resource
code. 

g.
--
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/