| Message ID | 20220923202822.2667581-14-keescook@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | slab: Introduce kmalloc_size_roundup() | expand |
On 9/23/22 22:28, Kees Cook wrote: > Round up allocations with kmalloc_size_roundup() so that mempool's use > of ksize() is always accurate and no special handling of the memory is > needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: linux-mm@kvack.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > mm/mempool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/mempool.c b/mm/mempool.c > index 96488b13a1ef..0f3107b28e6b 100644 > --- a/mm/mempool.c > +++ b/mm/mempool.c > @@ -526,7 +526,7 @@ EXPORT_SYMBOL(mempool_free_slab); > */ > void *mempool_kmalloc(gfp_t gfp_mask, void *pool_data) > { > - size_t size = (size_t)pool_data; > + size_t size = kmalloc_size_roundup((size_t)pool_data); Hm it is kinda wasteful to call into kmalloc_size_roundup for every allocation that has the same input. We could do it just once in mempool_init_node() for adjusting pool->pool_data ? But looking more closely, I wonder why poison_element() and kasan_unpoison_element() in mm/mempool.c even have to use ksize()/__ksize() and not just operate on the requested size (again, pool->pool_data). If no kmalloc mempool's users use ksize() to write beyond requested size, then we don't have to unpoison/poison that area either? > return kmalloc(size, gfp_mask); > } > EXPORT_SYMBOL(mempool_kmalloc);
On Mon, Sep 26, 2022 at 03:50:43PM +0200, Vlastimil Babka wrote: > On 9/23/22 22:28, Kees Cook wrote: > > Round up allocations with kmalloc_size_roundup() so that mempool's use > > of ksize() is always accurate and no special handling of the memory is > > needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: linux-mm@kvack.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > mm/mempool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/mempool.c b/mm/mempool.c > > index 96488b13a1ef..0f3107b28e6b 100644 > > --- a/mm/mempool.c > > +++ b/mm/mempool.c > > @@ -526,7 +526,7 @@ EXPORT_SYMBOL(mempool_free_slab); > > */ > > void *mempool_kmalloc(gfp_t gfp_mask, void *pool_data) > > { > > - size_t size = (size_t)pool_data; > > + size_t size = kmalloc_size_roundup((size_t)pool_data); > > Hm it is kinda wasteful to call into kmalloc_size_roundup for every > allocation that has the same input. We could do it just once in > mempool_init_node() for adjusting pool->pool_data ? > > But looking more closely, I wonder why poison_element() and > kasan_unpoison_element() in mm/mempool.c even have to use ksize()/__ksize() > and not just operate on the requested size (again, pool->pool_data). If no > kmalloc mempool's users use ksize() to write beyond requested size, then we > don't have to unpoison/poison that area either? Yeah, I think that's a fair point. I will adjust this.
diff --git a/mm/mempool.c b/mm/mempool.c index 96488b13a1ef..0f3107b28e6b 100644 --- a/mm/mempool.c +++ b/mm/mempool.c @@ -526,7 +526,7 @@ EXPORT_SYMBOL(mempool_free_slab); */ void *mempool_kmalloc(gfp_t gfp_mask, void *pool_data) { - size_t size = (size_t)pool_data; + size_t size = kmalloc_size_roundup((size_t)pool_data); return kmalloc(size, gfp_mask); } EXPORT_SYMBOL(mempool_kmalloc);
Round up allocations with kmalloc_size_roundup() so that mempool's use of ksize() is always accurate and no special handling of the memory is needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org Signed-off-by: Kees Cook <keescook@chromium.org> --- mm/mempool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)