linux-kernel - Re: [PATCH v2 2/4] ARM: amba: Fix race condition with driver

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:   Wed, 9 May 2018 11:39:54 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Adrian Salido <salidoa@...gle.com>,
        Nicolai Stange <nstange@...e.de>,
        Sasha Levin <Alexander.Levin@...rosoft.com>,
        Todd Kjos <tkjos@...roid.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] ARM: amba: Fix race condition with driver_override

On Thu, Apr 26, 2018 at 10:45:49AM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Thu, Apr 26, 2018 at 10:35 AM, Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> > On Thu, Apr 26, 2018 at 09:40:08AM +0200, Geert Uytterhoeven wrote:
> >> On Thu, Apr 26, 2018 at 9:04 AM, Greg Kroah-Hartman
> >> <gregkh@...uxfoundation.org> wrote:
> >> > On Wed, Apr 25, 2018 at 07:53:06PM +0200, Geert Uytterhoeven wrote:
> >> >> On Wed, Apr 25, 2018 at 6:06 PM, Greg Kroah-Hartman
> >> >> <gregkh@...uxfoundation.org> wrote:
> >> >> > On Tue, Apr 10, 2018 at 03:21:44PM +0200, Geert Uytterhoeven wrote:
> >> >> >> The driver_override implementation is susceptible to a race condition
> >> >> >> when different threads are reading vs storing a different driver
> >> >> >> override.  Add locking to avoid this race condition.
> >> >> >>
> >> >> >> Cfr. commits 6265539776a0810b ("driver core: platform: fix race
> >> >> >> condition with driver_override") and 9561475db680f714 ("PCI: Fix race
> >> >> >> condition with driver_override").
> >> >> >>
> >> >> >> Fixes: 3cf385713460eb2b ("ARM: 8256/1: driver coamba: add device binding path 'driver_override'")
> >> >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> >> >> >> Reviewed-by: Todd Kjos <tkjos@...gle.com>
> >> >> >> Cc: stable <stable@...r.kernel.org>
> >> >>
> >> >> > As this should go to stable kernels, I've fixed it up to apply without
> >> >> > patch 1 as that's not a real "fix" that anyone needs...
> >> >> >
> >> >> > Please try to remember to put fixes first, and then "trivial" things
> >> >> > later on in a series.
> >> >>
> >> >> I did it on purpose, as the fix is much more ugly without patch 1 applied.
> >> >> Can't you just take patch 1, too? More consistency is always nice, even for
> >> >> stable ;-)
> >> >
> >> > Consistency is nice, but when you have bug fixes that rely on "trivial"
> >> > patches, it's usually not nice :(
> >> >
> >> > I already committed patch 2 to my tree without 1, so let's leave it
> >> > as-is for now.
> >>
> >> Unfortunately the version you committed is buggy: the race condition
> >> also covers the NULL check removed by the trivial patch you skipped,
> >> so now you can get inconsistent behavior (no output or "(null)") on the
> >> same running kernel version...
> >>
> >> Please revert and apply both. Thanks!
> >
> > Ugh, you are right, sorry about that.
> >
> > I've reverted the offending patch, and added them in the correct order
> > now, I should have listened to you :)
> 
> Np, issue detected and fixed.
> Thanks!

So what about the patches you submitted to the patch system - should
I pick those up or not?

Please don't ask other maintainers to take patches that have been
submitted to the patch system without first changing their status,
they're liable to get applied anyway.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up