ACK/Cmnt [SRU][F][PATCH 1/1] usercopy: mark dma-kmalloc caches as usercopy caches

Krzysztof Kozlowski krzysztof.kozlowski at
Wed Oct 6 10:04:18 UTC 2021

On 05/10/2021 16:10, Kleber Souza wrote:
> On 05.10.21 08:32, frank.heimes at wrote:
>> From: Vlastimil Babka <vbabka at>
>> BugLink:
>> Upstream commit : 49f2d2419d60a103752e5fbaf158cf8d07c0d884

Thanks Frank for the patch.

This should be removed. It suggest it came from upstream stable but it
did not.

>> We have seen a "usercopy: Kernel memory overwrite attempt detected to
>> SLUB object 'dma-kmalloc-1 k' (offset 0, size 11)!" error on s390x, as
>> IUCV uses kmalloc() with __GFP_DMA because of memory address
>> restrictions.  The issue has been discussed [2] and it has been noted
>> that if all the kmalloc caches are marked as usercopy, there's little
>> reason not to mark dma-kmalloc caches too.  The 'dma' part merely means
>> that __GFP_DMA is used to restrict memory address range.
>> As Jann Horn put it [3]:
>>   "I think dma-kmalloc slabs should be handled the same way as normal
>>    kmalloc slabs. When a dma-kmalloc allocation is freshly created, it is
>>    just normal kernel memory - even if it might later be used for DMA -,
>>    and it should be perfectly fine to copy_from_user() into such
>>    allocations at that point, and to copy_to_user() out of them at the
>>    end. If you look at the places where such allocations are created, you
>>    can see things like kmemdup(), memcpy() and so on - all normal
>>    operations that shouldn't conceptually be different from usercopy in
>>    any relevant way."
>> Thus this patch marks the dma-kmalloc-* caches as usercopy.
>> [1]
>> [2]
>> [3]
>> Signed-off-by: Vlastimil Babka <vbabka at>
>> Signed-off-by: Andrew Morton <akpm at>
>> Acked-by: Christian Borntraeger <borntraeger at>
>> Acked-by: Jiri Slaby <jslaby at>
>> Cc: Jann Horn <jannh at>
>> Cc: Christoph Hellwig <hch at>
>> Cc: Christopher Lameter <cl at>
>> Cc: Julian Wiedmann <jwi at>
>> Cc: Ursula Braun <ubraun at>
>> Cc: Alexander Viro <viro at>
>> Cc: David Windsor <dave at>
>> Cc: Pekka Enberg <penberg at>
>> Cc: David Rientjes <rientjes at>
>> Cc: Joonsoo Kim < at>
>> Cc: Andy Lutomirski <luto at>
>> Cc: "David S. Miller" <davem at>
>> Cc: Laura Abbott <labbott at>
>> Cc: Mark Rutland <mark.rutland at>
>> Cc: "Martin K. Petersen" <martin.petersen at>
>> Cc: Paolo Bonzini <pbonzini at>
>> Cc: Christoffer Dall <christoffer.dall at>
>> Cc: Dave Kleikamp <dave.kleikamp at>
>> Cc: Jan Kara <jack at>
>> Cc: Luis de Bethencourt <luisbg at>
>> Cc: Marc Zyngier <marc.zyngier at>
>> Cc: Rik van Riel <riel at>
>> Cc: Matthew Garrett <mjg59 at>
>> Cc: Michal Kubecek <mkubecek at>
>> Link:
>> Signed-off-by: Linus Torvalds <torvalds at>
>> (backported from commit 49f2d2419d60a103752e5fbaf158cf8d07c0d884)
>> Signed-off-by: Frank Heimes <frank.heimes at>
>> ---
>>   mm/slab_common.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 8c1ffbf7de45..76433e2187d0 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -1296,7 +1296,7 @@ void __init create_kmalloc_caches(slab_flags_t flags)
>>   			BUG_ON(!n);
>>   			kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
>> -				n, size, SLAB_CACHE_DMA | flags, 0, 0);
>> +				n, size, SLAB_CACHE_DMA | flags, 0, kmalloc_info[i].size);
> Although this is probably functionally the same, I'm wondering if we should
> keep both "size" and "usersize" parameters passed to create_kmalloc_cache()
> consistent.
> In the original patch (49f2d2419d60), the "size" is not calculated anymore using
> "kmalloc_size(i)", which was removed by dc0a7f7558dd (mm, slab: remove unused
> kmalloc_size()), but passing "kmalloc_info[i].size" instead. The same is passed
> as the "usersize" then.
> Both ways of calculating "usersize" seem equivalent (using either "kmalloc_size(i)"
> or "kmalloc_info[i].size"), but I didn't look very deeply to check if any patch
> changed any of them in the meantime. So in my opinion we should also use "size" for
> the backport to be on the safe side and do something like:
>    			kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
> -				n, size, SLAB_CACHE_DMA | flags, 0, 0);
> +				n, size, SLAB_CACHE_DMA | flags, 0, size);
> I'll not ACK nor NACK for now and wait for someone else to weight in.

It would be easier for future backports if we have here direct pick, so
Frank's version. It would be more logical to have a backport matching
v5.4 context, so what Kleber proposes.

IOW, I don't see perfect solution and since this was tested:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski at>

> Kleber

Best regards,

More information about the kernel-team mailing list