Re: slub - extended kmalloc redzone and dma alignment

[Feng Tang <feng.tang@linux.alibaba.com>]

On Wed, Apr 09, 2025 at 03:30:16PM +0100, Catalin Marinas wrote:
> 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.

Me either.

> 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().

Yes, I agree it's better to let driver take care of the alignment part.
IMHO, touching the memory beyond its original requested size is kind of
abusing, no matter it's software intentional or 'unexpected' hardware
behavior. kmalloc_size_roundup() patchset was initially introduced to
help reducing potential similar issues:
https://lore.kernel.org/lkml/20220922031013.2150682-1-keescook@chromium.org/t/#u

Thanks,
Feng

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