linux-kernel - Re: [PATCH] kernel/resource.c: fix muxed resource handling in __request

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]

Date:	Sat, 20 Feb 2016 14:15:49 -0800
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Vincent Pelletier <plr.vincent@...il.com>,
	Simon Guinot <simon.guinot@...uanux.org>,
	Alan Cox <alan@...ux.intel.com>,
	Giel van Schijndel <me@...tis.eu>,
	<linux-gpio@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Vincent Donnefort <vdonnefort@...il.com>,
	Yoann Sculo <yoann@...lo.fr>
Subject: Re: [PATCH] kernel/resource.c: fix muxed resource handling in __request_region()

On February 20, 2016 9:12:01 AM Linus Torvalds 
<torvalds@...ux-foundation.org> wrote:

> On Fri, Feb 19, 2016 at 3:25 PM, Jesse Barnes <jbarnes@...tuousgeek.org> wrote:
>> +Linus (the de-facto resource guy).
>>
>> On 02/19/2016 01:10 PM, Vincent Pelletier wrote:
>>> Tested-by: Vincent Pelletier <plr.vincent@...il.com>
>
> Hmm.
>
> So I'm not entirely happy with the patch, because I think the problem
> with using a possibly free'd parent resource at restart still exists.
>
> As far as I can tell, if we hit the IORESOURCE_MUXED case *after* we
> have successfully delved into a resource that wasn't busy, we will
> have updated "parent" in a previous iteration of the loop, and we'll
> not use the original parent when we then re-start after the sleep. So
> quite frankly, I suspect any user of MUXED memory regions is still
> fundamentally buggy, and IORESOURCE_MUXED has always been a hacky and
> broken thing.
>
> That said, I ended up applying the patch anyway, even if I despise it.
> For all I know, muxed users never end up having those non-busy
> sub-resources in practice, and maybe there is some serialization at
> the top level for the drivers that use it. So if testing has shown
> that it helps some actual case, I'll believe the testing. But the code
> still looks rather debatable, and the whole IORESOURCE_MUXED approach
> looks broken.
>
> Jesse, that came in through you and the drm tree, I think. Alan is
> marked as author, and there are other people who actually use and can
> test the code. Can you guys think about the code a bit more.
>
> I'm wondering if the *real* fix ends up being to reset the 'parent'
> pointer to the original top-level parent after the sleep?
>
> To recap: the patch is in my tree, but I'm not all that happy about it.

Thanks, yeah i think testing wins in this case.  I'll revisit the muxed 
stuff; I do
remember being dubious at the time, but iirc Alan needed it for something,
and others had been pushing for these sorts of usages for awhile even though
we have some good alternatives in the form of bus and platform drivers that
can manage the appropriate serialization and keep things from stomping
on one another.

(And sorry if this message comes across in some bullshit format, I'm trying
out a new ChromeOS based mail client for fun here.)

Thanks,
Jesse