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: <9be1d523-e92c-836b-b79d-37e880d092a0@arm.com>
Date:   Fri, 20 Dec 2019 15:01:03 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     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>,
        Alexey Brodkin <alexey.brodkin@...opsys.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>
Subject: Re: [RFC PATCH v1] devres: align devres.data strictly only for
 devm_kmalloc()

/me rouses from holiday mode...

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.

No matter which way round you allocate devres and data, by necessity 
they're always going to consume the same total amount of memory.

Robin.

> 	dr = data2devres(data);
> 	WARN_ON((unsigned long)dr & __alignof(*dr)-1);
> 	INIT_LIST_HEAD(&dr->node.entry);
> 	dr->node.release = release;
> 	dr->data = data;
> 
> 	return dr;
> }
> 
> void devres_free(void *data)
> {
> 	if (data) {
> 		struct devres *dr = data2devres(data);
> 		BUG_ON(!list_empty(dr->node.entry));
> 		kfree(data);
> 	}
> }
> 
> static int release_nodes(...)
> {
> 	...
> 	list_for_each_entry_safe_reverse(dr, ...) {
> 		...
> 		kfree(dr->data);
> 	}
> }
>