mbox series

[v3,0/3] mm/slub: Improve data handling of krealloc() when orig_size is enabled

Message ID 20241016154152.1376492-1-feng.tang@intel.com (mailing list archive)
Headers show
Series mm/slub: Improve data handling of krealloc() when orig_size is enabled | expand

Message

Feng Tang Oct. 16, 2024, 3:41 p.m. UTC
Danilo Krummrich's patch [1] raised one problem about krealloc() that
its caller doesn't pass the old request size, say the object is 64
bytes kmalloc one, but caller originally only requested 48 bytes. Then
when krealloc() shrinks or grows in the same object, or allocate a new
bigger object, it lacks this 'original size' information to do accurate
data preserving or zeroing (when __GFP_ZERO is set).

Thus with slub debug redzone and object tracking enabled, parts of the
object after krealloc() might contain redzone data instead of zeroes,
which is violating the __GFP_ZERO guarantees. Good thing is in this
case, kmalloc caches do have this 'orig_size' feature, which could be
used to improve the situation here.

To make the 'orig_size' accurate, we adjust some kasan/slub meta data
handling. Also add a slub kunit test case for krealloc().

Many thanks to syzbot and V, Narasimhan for detecting issues of the
v2 patches.

This is again linux-slab tree's 'for-6.13/fixes' branch

[1]. https://lore.kernel.org/lkml/20240812223707.32049-1-dakr@kernel.org/

Thanks,
Feng

Changelog:

  Since v2:
  * Fix NULL pointer issue related to big kmalloc object which has
    no associated slab (V, Narasimhan, syzbot)
  * Fix issue related handling for kfence allocated object (syzbot,
    Marco Elver)
  * drop the 0001 and 0003 patch whch have been merged to slab tree

  Since v1:
  * Drop the patch changing generic kunit code from this patchset,
    and will send it separately.
  * Separate the krealloc moving form slab_common.c to slub.c to a 
    new patch for better review (Danilo/Vlastimil)
  * Improve commit log and comments (Vlastimil/Danilo) 
  * Rework the kunit test case to remove its dependency over
    slub_debug (which is incomplete in v1) (Vlastimil)
  * Add ack and review tag from developers.



Feng Tang (3):
  mm/slub: Consider kfence case for get_orig_size()
  mm/slub: Improve redzone check and zeroing for krealloc()
  mm/slub, kunit: Add testcase for krealloc redzone and zeroing

 lib/slub_kunit.c | 42 +++++++++++++++++++++++
 mm/slub.c        | 87 +++++++++++++++++++++++++++++++++++-------------
 2 files changed, 105 insertions(+), 24 deletions(-)

Comments

Vlastimil Babka Oct. 18, 2024, 9:51 a.m. UTC | #1
On 10/16/24 17:41, Feng Tang wrote:
> Danilo Krummrich's patch [1] raised one problem about krealloc() that
> its caller doesn't pass the old request size, say the object is 64
> bytes kmalloc one, but caller originally only requested 48 bytes. Then
> when krealloc() shrinks or grows in the same object, or allocate a new
> bigger object, it lacks this 'original size' information to do accurate
> data preserving or zeroing (when __GFP_ZERO is set).
> 
> Thus with slub debug redzone and object tracking enabled, parts of the
> object after krealloc() might contain redzone data instead of zeroes,
> which is violating the __GFP_ZERO guarantees. Good thing is in this
> case, kmalloc caches do have this 'orig_size' feature, which could be
> used to improve the situation here.
> 
> To make the 'orig_size' accurate, we adjust some kasan/slub meta data
> handling. Also add a slub kunit test case for krealloc().
> 
> Many thanks to syzbot and V, Narasimhan for detecting issues of the
> v2 patches.
> 
> This is again linux-slab tree's 'for-6.13/fixes' branch

Thanks, added there.

Vlastimil

> [1]. https://lore.kernel.org/lkml/20240812223707.32049-1-dakr@kernel.org/
> 
> Thanks,
> Feng
> 
> Changelog:
> 
>   Since v2:
>   * Fix NULL pointer issue related to big kmalloc object which has
>     no associated slab (V, Narasimhan, syzbot)
>   * Fix issue related handling for kfence allocated object (syzbot,
>     Marco Elver)
>   * drop the 0001 and 0003 patch whch have been merged to slab tree
> 
>   Since v1:
>   * Drop the patch changing generic kunit code from this patchset,
>     and will send it separately.
>   * Separate the krealloc moving form slab_common.c to slub.c to a 
>     new patch for better review (Danilo/Vlastimil)
>   * Improve commit log and comments (Vlastimil/Danilo) 
>   * Rework the kunit test case to remove its dependency over
>     slub_debug (which is incomplete in v1) (Vlastimil)
>   * Add ack and review tag from developers.
> 
> 
> 
> Feng Tang (3):
>   mm/slub: Consider kfence case for get_orig_size()
>   mm/slub: Improve redzone check and zeroing for krealloc()
>   mm/slub, kunit: Add testcase for krealloc redzone and zeroing
> 
>  lib/slub_kunit.c | 42 +++++++++++++++++++++++
>  mm/slub.c        | 87 +++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 105 insertions(+), 24 deletions(-)
>