Messages in this thread | | | Date | Thu, 22 Sep 2022 08:45:19 -0500 | Subject | Re: [PATCH 03/12] net: ipa: Proactively round up to kmalloc bucket size | From | Alex Elder <> |
| |
On 9/21/22 10:10 PM, Kees Cook wrote: > Instead of discovering the kmalloc bucket size _after_ allocation, round > up proactively so the allocation is explicitly made for the full size, > allowing the compiler to correctly reason about the resulting size of > the buffer through the existing __alloc_size() hint. > > Cc: Alex Elder <elder@kernel.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: netdev@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/net/ipa/gsi_trans.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c > index 18e7e8c405be..cec968854dcf 100644 > --- a/drivers/net/ipa/gsi_trans.c > +++ b/drivers/net/ipa/gsi_trans.c > @@ -89,6 +89,7 @@ int gsi_trans_pool_init(struct gsi_trans_pool *pool, size_t size, u32 count, > u32 max_alloc) > { > void *virt; > + size_t allocate;
I don't care about this but the reverse Christmas tree convention would put the "allocate" variable definition above "virt".
Whether you fix that or not, this patch looks good to me.
Reviewed-by: Alex Elder <elder@linaro.org>
> if (!size) > return -EINVAL; > @@ -104,13 +105,15 @@ int gsi_trans_pool_init(struct gsi_trans_pool *pool, size_t size, u32 count, > * If there aren't enough entries starting at the free index, > * we just allocate free entries from the beginning of the pool. > */ > - virt = kcalloc(count + max_alloc - 1, size, GFP_KERNEL); > + allocate = size_mul(count + max_alloc - 1, size); > + allocate = kmalloc_size_roundup(allocate); > + virt = kzalloc(allocate, GFP_KERNEL); > if (!virt) > return -ENOMEM; > > pool->base = virt; > /* If the allocator gave us any extra memory, use it */ > - pool->count = ksize(pool->base) / size; > + pool->count = allocate / size; > pool->free = 0; > pool->max_alloc = max_alloc; > pool->size = size;
| |