<div dir="ltr">Well, I'm sure this is a question for experienced kernel engineers - so I just stay tuned.<div> </div><div>But as a side note: My approach was only to get the single commit 49f2d2419d60 in as smooth as possible, means with as few changes as possible (incl. context), so that potential further cherry-picks or backports in that area are not harmed (or at least as little as possible). </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 5, 2021 at 4:10 PM Kleber Souza <<a href="mailto:kleber.souza@canonical.com">kleber.souza@canonical.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 05.10.21 08:32, <a href="mailto:frank.heimes@canonical.com" target="_blank">frank.heimes@canonical.com</a> wrote: > From: Vlastimil Babka <<a href="mailto:vbabka@suse.cz" target="_blank">vbabka@suse.cz</a>> > > BugLink: <a href="https://bugs.launchpad.net/bugs/1913442" rel="noreferrer" target="_blank">https://bugs.launchpad.net/bugs/1913442</a> > > Upstream commit : 49f2d2419d60a103752e5fbaf158cf8d07c0d884 > > 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] <a href="https://bugzilla.suse.com/show_bug.cgi?id=1156053" rel="noreferrer" target="_blank">https://bugzilla.suse.com/show_bug.cgi?id=1156053</a> > [2] <a href="https://lore.kernel.org/kernel-hardening/bfca96db-bbd0-d958-7732-76e36c667c68@suse.cz/" rel="noreferrer" target="_blank">https://lore.kernel.org/kernel-hardening/bfca96db-bbd0-d958-7732-76e36c667c68@suse.cz/</a> > [3] <a href="https://lore.kernel.org/kernel-hardening/CAG48ez1a4waGk9kB0WLaSbs4muSoK0AYAVk8=XYaKj4_+6e6Hg@mail.gmail.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/kernel-hardening/CAG48ez1a4waGk9kB0WLaSbs4muSoK0AYAVk8=XYaKj4_+6e6Hg@mail.gmail.com/</a> > > Signed-off-by: Vlastimil Babka <<a href="mailto:vbabka@suse.cz" target="_blank">vbabka@suse.cz</a>> > Signed-off-by: Andrew Morton <<a href="mailto:akpm@linux-foundation.org" target="_blank">akpm@linux-foundation.org</a>> > Acked-by: Christian Borntraeger <<a href="mailto:borntraeger@de.ibm.com" target="_blank">borntraeger@de.ibm.com</a>> > Acked-by: Jiri Slaby <<a href="mailto:jslaby@suse.cz" target="_blank">jslaby@suse.cz</a>> > Cc: Jann Horn <<a href="mailto:jannh@google.com" target="_blank">jannh@google.com</a>> > Cc: Christoph Hellwig <<a href="mailto:hch@infradead.org" target="_blank">hch@infradead.org</a>> > Cc: Christopher Lameter <<a href="mailto:cl@linux.com" target="_blank">cl@linux.com</a>> > Cc: Julian Wiedmann <<a href="mailto:jwi@linux.ibm.com" target="_blank">jwi@linux.ibm.com</a>> > Cc: Ursula Braun <<a href="mailto:ubraun@linux.ibm.com" target="_blank">ubraun@linux.ibm.com</a>> > Cc: Alexander Viro <<a href="mailto:viro@zeniv.linux.org.uk" target="_blank">viro@zeniv.linux.org.uk</a>> > Cc: David Windsor <<a href="mailto:dave@nullcore.net" target="_blank">dave@nullcore.net</a>> > Cc: Pekka Enberg <<a href="mailto:penberg@kernel.org" target="_blank">penberg@kernel.org</a>> > Cc: David Rientjes <<a href="mailto:rientjes@google.com" target="_blank">rientjes@google.com</a>> > Cc: Joonsoo Kim <<a href="mailto:iamjoonsoo.kim@lge.com" target="_blank">iamjoonsoo.kim@lge.com</a>> > Cc: Andy Lutomirski <<a href="mailto:luto@kernel.org" target="_blank">luto@kernel.org</a>> > Cc: "David S. Miller" <<a href="mailto:davem@davemloft.net" target="_blank">davem@davemloft.net</a>> > Cc: Laura Abbott <<a href="mailto:labbott@redhat.com" target="_blank">labbott@redhat.com</a>> > Cc: Mark Rutland <<a href="mailto:mark.rutland@arm.com" target="_blank">mark.rutland@arm.com</a>> > Cc: "Martin K. Petersen" <<a href="mailto:martin.petersen@oracle.com" target="_blank">martin.petersen@oracle.com</a>> > Cc: Paolo Bonzini <<a href="mailto:pbonzini@redhat.com" target="_blank">pbonzini@redhat.com</a>> > Cc: Christoffer Dall <<a href="mailto:christoffer.dall@linaro.org" target="_blank">christoffer.dall@linaro.org</a>> > Cc: Dave Kleikamp <<a href="mailto:dave.kleikamp@oracle.com" target="_blank">dave.kleikamp@oracle.com</a>> > Cc: Jan Kara <<a href="mailto:jack@suse.cz" target="_blank">jack@suse.cz</a>> > Cc: Luis de Bethencourt <<a href="mailto:luisbg@kernel.org" target="_blank">luisbg@kernel.org</a>> > Cc: Marc Zyngier <<a href="mailto:marc.zyngier@arm.com" target="_blank">marc.zyngier@arm.com</a>> > Cc: Rik van Riel <<a href="mailto:riel@surriel.com" target="_blank">riel@surriel.com</a>> > Cc: Matthew Garrett <<a href="mailto:mjg59@google.com" target="_blank">mjg59@google.com</a>> > Cc: Michal Kubecek <<a href="mailto:mkubecek@suse.cz" target="_blank">mkubecek@suse.cz</a>> > Link: <a href="http://lkml.kernel.org/r/7d810f6d-8085-ea2f-7805-47ba3842dc50@suse.cz" rel="noreferrer" target="_blank">http://lkml.kernel.org/r/7d810f6d-8085-ea2f-7805-47ba3842dc50@suse.cz</a> > Signed-off-by: Linus Torvalds <<a href="mailto:torvalds@linux-foundation.org" target="_blank">torvalds@linux-foundation.org</a>> > (backported from commit 49f2d2419d60a103752e5fbaf158cf8d07c0d884) > Signed-off-by: Frank Heimes <<a href="mailto:frank.heimes@canonical.com" target="_blank">frank.heimes@canonical.com</a>> > --- > 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. Kleber </blockquote></div>