linux-kernel - Re: [PATCH 02/10] drivers/base: Use ARCH_DMA_MINALIGN instead of ARCH_KMALLOC

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:   Mon, 11 Apr 2022 18:39:37 +0100
From:   Catalin Marinas <catalin.marinas@....com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-mm <linux-mm@...ck.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH 02/10] drivers/base: Use ARCH_DMA_MINALIGN instead of
 ARCH_KMALLOC_MINALIGN

On Mon, Apr 11, 2022 at 05:57:08PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 6, 2022 at 2:30 PM Catalin Marinas <catalin.marinas@....com> wrote:
> > ARCH_DMA_MINALIGN represents the minimum (static) alignment for safe DMA
> > operations while ARCH_KMALLOC_MINALIGN is the minimum kmalloc() objects
> > alignment.
> 
> ...
> 
> > -        * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> > +        * Thus we use ARCH_DMA_MINALIGN here and get at least the same
> >          * buffer alignment as if it was allocated by plain kmalloc().
> 
> But then it becomes not true either, because the kmalloc() has other
> alignment constraints.

Maybe the comment could be improved a bit but I think it's still valid.
After this patch, struct devres becomes:

struct devres {
	struct devres_node		node;
	u8 __aligned(ARCH_DMA_MINALIGN) data[];
};

While we no longer guarantee the ARCH_DMA_MINALIGN alignment (which is
too big on most arm64 SoCs), what we need is for devres.data[] to be
aligned to the newly introduced arch_kmalloc_minalign(). This would give
us the DMA safety guarantees.

Since devres.data[] is at an offset multiple of ARCH_DMA_MINALIGN, in
order for the array to be aligned to arch_kmalloc_minalign(), all we
need is for ARCH_DMA_MINALIGN to be a multiple of
arch_kmalloc_minalign(). I actually had to write down some simple
equations to convince myself.

devres.data[] is at an offset multiple of ARCH_DMA_MINALIGN (after this
patch), even when struct devres is included in another structure, so we
have:

  offsetof(struct devres, data) = m * ARCH_DMA_MINALIGN

ARCH_DMA_MINALIGN is a power of two while arch_kmalloc_minalign() is
also a power of two, equal to or less than ARCH_DMA_MINALIGN:

  ARCH_DMA_MINALIGN = n * arch_kmalloc_minalign()

A kmalloc()'ed object of struct devres (or a container of) is aligned to
arch_kmalloc_minalign() by definition so:

  kmalloc() = p * arch_kmalloc_minalign()

>From the above, we can conclude that the data[] pointer is at a multiple
of arch_kmalloc_minalign():

  devres.data = (p + m * n) * arch_kmalloc_minalign()

Where m, n, p are all positive integers (n is also power of two).

If we did not change the devres structure, the alignment of
ARCH_KMALLOC_MINALIGN would no no be longer sufficient since the dynamic
arch_kmalloc_minalign() can be greater than ARCH_KMALLOC_MINALIGN on
specific SoCs (the first offsetof equation is no longer true).

-- 
Catalin