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]

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

Hi Robin, Peter, all,

[snip]
 
> On 2019-12-20 2:06 pm, Peter Zijlstra wrote:
> > On Fri, Dec 20, 2019 at 11:19:27AM +0100, Marc Gonzalez wrote:
> >> Would anyone else have any suggestions, comments, insights, recommendations,
> >> improvements, guidance, or wisdom? :-)
> >
> > Flip devres upside down!
> 
> Which doesn't really help :(
> 
> > **WARNING, wear protective glasses when reading the below**
> >
> >
> > struct devres {
> > 	struct devres_node	node;
> > 	void			*data;
> > };
> >
> > /*
> >   * We place struct devres at the tail of the memory allocation
> >   * such that data retains the ARCH_KMALLOC_MINALIGN alignment.
> >   * struct devres itself is just 4 pointers and should therefore
> >   * only require trivial alignment.
> >   */
> > static inline struct devres *data2devres(void *data)
> > {
> > 	return (struct devres *)(data + ksize(data) - sizeof(struct devres));
> > }
> >
> > void *alloc_dr(...)
> > {
> > 	struct devres *dr;
> > 	void *data;
> >
> > 	data = kmalloc(size + sizeof(struct devres), GFP_KERNEL);
> 
> At this point, you'd still need to special-case devm_kmalloc() to ensure
> size is rounded up to the next ARCH_KMALLOC_MINALIGN granule, or you'd
> go back to the original problem of the struct devres fields potentially
> sharing a cache line with the data buffer. That needs to be avoided,
> because if the devres list is modified while the buffer is mapped for
> noncoherent DMA (which could legitimately happen as they are nominally
> distinct allocations with different owners) there's liable to be data
> corruption one way or the other.

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
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?

> 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 other than the hack we're
discussing here (meta-data in the beginning of the buffer) used to work OK-ish?

-Alexey