Re: [PATCH v2 0/5] mm/slub: Improve data handling of krealloc() when orig_size is enabled

[Vlastimil Babka]

On 10/4/24 11:18, Vlastimil Babka wrote:
> On 10/4/24 08:44, Marco Elver wrote:
> 
> I think it's commit d0a38fad51cc7 doing in __do_krealloc()
> 
> -               ks = ksize(p);
> +
> +               s = virt_to_cache(p);
> +               orig_size = get_orig_size(s, (void *)p);
> +               ks = s->object_size;
> 
> so for kfence objects we don't get their actual allocation size but the
> potentially larger bucket size?
> 
> I guess we could do:
> 
> ks = kfence_ksize(p) ?: s->object_size;
> 
> ?

Hmm this probably is not the whole story, we also have:

-               memcpy(ret, kasan_reset_tag(p), ks);
+               if (orig_size)
+                       memcpy(ret, kasan_reset_tag(p), orig_size);

orig_size for kfence will be again s->object_size so the memcpy might be a
(read) buffer overflow from a kfence allocation.

I think get_orig_size() should perhaps return kfence_ksize(p) for kfence
allocations, in addition to the change above.

Or alternatively we don't change get_orig_size() (in a different commit) at
all, but __do_krealloc() will have an "if is_kfence_address()" that sets
both orig_size and ks to kfence_ksize(p) appropriately. That might be easier
to follow.

But either way means rewriting 2 commits. I think it's indeed better to drop
the series now from -next and submit a v3.

Vlastimil

>> Thanks,
>> -- Marco
>