Re: slub - extended kmalloc redzone and dma alignment

[Vlastimil Babka <vbabka@suse.cz>]

On 4/7/25 09:21, Feng Tang wrote:
> On Sun, Apr 06, 2025 at 10:02:40PM +0800, Feng Tang wrote:
> [...]
>> > I can remember this series, as well as my confusion why 192-byte
>> > kmalloc caches were missing on arm64.
>> > 
>> > Nevertheless, I believe ARCH_DMA_MINALIGN is required to avoid putting
>> > a DMA buffer on the same cache line as some other data that might be
>> > _written_ by the CPU while the corresponding main memory is modified by
>> > another bus-mastering device.
>> > 
>> > Consider this layout:
>> > 
>> >  ... | DMA buffer | other data | ...
>> >      ^                         ^
>> >      +-------------------------+-- cache line boundaries
>> > 
>> > When you prepare for DMA, you make sure that the DMA buffer is not
>> > cached by the CPU, so you flush the cache line (from all levels). Then
>> > you tell the device to write into the DMA buffer. However, before the
>> > device finishes the DMA transaction, the CPU accesses "other data",
>> > loading this cache line from main memory with partial results. Worse,
>> > if the CPU writes to "other data", it may write the cache line back
>> > into main memory, racing with the device writing to DMA buffer, and you
>> > end up with corrupted data in DMA buffer.
>> > 
>> > But redzone poisoning should happen long before the DMA buffer cache
>> > line is flushed. The device will not overwrite it unless it was given
>> > wrong buffer length for the transaction, but then that would be a bug
>> > that I'd rather detect.
>> 
>> I alaso tend to think it's better for slub to detect these kind of DMA
>> 'overflow'. We've added slub kunit test case for these in commmit 
>> 6cd6d33ca41f ("mm/slub, kunit: Add a test case for kmalloc redzone check),
>> which was inspired by a similar DMA related bug as described in
>> commit 120ee599b5bf ("staging: octeon-usb: prevent memory corruption")

OK so besides Petr's explanation that was about cache (in)coherency and is
AFAIK tied to ARCH_DMA_MINALIGN, there is possibility of DMA that will
really write garbage beyond the buffer that's not word aligned. Can we
assume that this was really a bug in the usage and ensuring word alignment
(not ARCH_DMA_MINALIGN alignment) is required from a different layer than
kmalloc() itself? In that case it would be best to keep the reporting as it is.

> I'm not familiar with DMA stuff, but Vlastimil's idea does make it
> easier for driver developer to write a driver to be used on different 
> ARCHs, which have different DMA alignment requirement. Say if the minimal
> safe size is 8 bytes, the driver can just request 8 bytes and
> ARCH_DMA_MINALIGN will automatically chose the right size for it, which
> can save memory for ARCHs with smaller alignment requirement. Meanwhile
> it does sacrifice part of the redzone check ability, so I don't have
> preference here :)

Let's clarify first who's expected to ensure the word alignment for DMA, if
it's not kmalloc() then I'd rather resist moving it there :)

Thanks,
Vlastimil

> Thanks,
> Feng