Re: slub - extended kmalloc redzone and dma alignment

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Apr 09, 2025 at 02:22:10PM +0200, Vlastimil Babka wrote:
> On 4/9/25 1:11 PM, Catalin Marinas wrote:
> > On Wed, Apr 09, 2025 at 10:51:43AM +0200, Vlastimil Babka wrote:
> >> On 4/8/25 5:07 PM, Catalin Marinas wrote:
> >>> Assuming I got kmalloc redzoning right, I think there's still a
> >>> potential issue. Let's say we have a system with 128-byte DMA alignment
> >>> required (the largest cache line size). We do a kmalloc(104) and
> >>> kmalloc_size_roundup() returns 128, so all seems good to the DMA code.
> >>> However, kmalloc() redzones from 104 to 128 as it tracks the original
> >>> size. The DMA bouncing doesn't spot it since the
> >>> kmalloc_size_roundup(104) is aligned to 128.
> >>
> >> Note that kmalloc_size_roundup() is supposed to be used *before*
> >> kmalloc(), such as dma_resv_list_alloc() does. Then there's no issue as
> >> no redzoning would not be done between 104 and 128, there would be only
> >> the additional redzone at 128+.
> > 
> > Yes, if people use it this way. devm_kmalloc() via alloc_dr() also seems
> > to be handling this. However, given the original report, I assume there
> 
> We can probably ignore my original private discussion as motivation as
> it wasn't confirmed (and I'm not sure it will) that it was really a case
> involving DMA alignment. It was just something I thought might be
> possible explanation and wanted to doublecheck with people more
> knowledgeable.
> 
> Unless you mean original report as 120ee599b5bf ("staging: octeon-usb:
> prevent memory corruption") that Feng mentioned.

I was referring to your private discussion. IIUC the one Feng mentioned
was about the SLOB allocator which I recall did not guarantee natural
alignment for power-of-two allocations.

> > are drivers that have a problem with redzoning at the end of the buffer.
> 
> So I'm not aware of any issues reported due to the extended redzoning.

Thanks for confirming. I guess we can ignore this potential issue then
as long as drivers take care of the alignment or use devm_kmalloc().

> > I did a quick test with kmem_cache_create() of 104 bytes with
> > SLAB_HWCACHE_ALIGN (64 bytes) and it has a similar problem with the
> > redzone from byte 104 onwards. Here we don't have the equivalent of
> > kmalloc_size_roundup() that a driver can use.
> 
> AFAIK SLAB_HWCACHE_ALIGN exists for performance reasons, not to provide
> dma guarantees as kmalloc(). So I'd say users of kmem_cache_create()
> would have to do their own rounding - you mentioned
> dma_get_cache_alignment()? And there's an align parameter too when
> creating caches.

I just checked and the align parameter only ensures the start of the
buffer, the redzone start is not aligned.

Anyway, as in the other subthread with Petr, I think most architectures
would benefit from an update to the DMA cache maintenance to avoid
corrupting the redzone.

-- 
Catalin