linux-kernel - RE: [RFC PATCH v1] devres: align devres.data strictly only for devm

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: <CY4PR1201MB0120C3727E907005D24F5A03A12D0@CY4PR1201MB0120.namprd12.prod.outlook.com>
Date:   Fri, 20 Dec 2019 21:02:24 +0000
From:   Alexey Brodkin <Alexey.Brodkin@...opsys.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     Will Deacon <will@...nel.org>,
        Marc Gonzalez <marc.w.gonzalez@...e.fr>,
        Eugeniy Paltsev <Eugeniy.Paltsev@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Vineet Gupta <Vineet.Gupta1@...opsys.com>,
        "Dmitry Torokhov" <dmitry.torokhov@...il.com>,
        Rafael Wysocki <rjw@...ysocki.net>,
        LKML <linux-kernel@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Mark Brown <broonie@...nel.org>, Tejun Heo <tj@...nel.org>,
        arcml <linux-snps-arc@...ts.infradead.org>,
        Robin Murphy <robin.murphy@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [RFC PATCH v1] devres: align devres.data strictly only for
 devm_kmalloc()

Hi Peter,

> > Well it somehow used to work for quite some time now with the data-buffer
> > being allocated with 4 words offset (which is 16 bytes for 32-bit platform
> 
> 3 words, devres_node is 3 words.

Correct, but 4th word was implicitly there due to the fact
on most of 32-bit arches "long long" is aligned by 2 words.
 
> Which is exactly why we had to change it, the odd alignment caused ARC
> to explode.

I know that better than anybody else as it was my pain & grief :)
 
> > and 32 for 64-bit which is still much less than mentioned 128 bytes).
> > Or we just never managed to identify those rare cases when data corruption
> > really happened?
> 
> The races are rather rare methinks, you'd have to get a list-op
> concurrently with a DMA.
> 
> If you get the list corrupted, I'm thinking the crash is fairly likely,
> albeit really difficuly to debug.

So that alone IMHO is a good reason to not allow that thing to happen even
in theory.

> > > No matter which way round you allocate devres and data, by necessity
> > > they're always going to consume the same total amount of memory.
> >
> > So then the next option I guess is to separate meta-data from data buffers
> > completely. Are there any reasons to not do that
> 
> Dunno, should work just fine I think.
> 
> > other than the hack we're
> > discussing here (meta-data in the beginning of the buffer) used to work OK-ish?
> 
> If meta-data at the beginngin used to work, I don't see why meta-data at
> the end wouldn't work equally well. They'd be equally broken.

Agree. But if we imagine devm allocations are not used for DMA
(which is yet another case of interface usage which was never designed for
but alas this happens left and right) then move of the meta-data to the end of
the buffers solves [mostly my] problem... but given that DMA case we discuss
exists I'm not sure if this move actually worth spending time on.

-Alexey