lkml.org 
[lkml]   [2018]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 04/13] mm: Use array_size() helpers for kmalloc()
    On Tue, May 08, 2018 at 05:42:20PM -0700, Kees Cook wrote:
    > @@ -499,6 +500,8 @@ static __always_inline void *kmalloc_large(size_t size, gfp_t flags)
    > */
    > static __always_inline void *kmalloc(size_t size, gfp_t flags)
    > {
    > + if (size == SIZE_MAX)
    > + return NULL;
    > if (__builtin_constant_p(size)) {
    > if (size > KMALLOC_MAX_CACHE_SIZE)
    > return kmalloc_large(size, flags);

    I don't like the add-checking-to-every-call-site part of this patch.
    Fine, the compiler will optimise it away if it can calculate it at compile
    time, but there are a lot of situations where it can't. You aren't
    adding any safety by doing this; trying to allocate SIZE_MAX bytes is
    guaranteed to fail, and it doesn't need to fail quickly.

    > @@ -624,11 +629,13 @@ int memcg_update_all_caches(int num_memcgs);
    > */
    > static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
    > {
    > - if (size != 0 && n > SIZE_MAX / size)
    > + size_t bytes = array_size(n, size);
    > +
    > + if (bytes == SIZE_MAX)
    > return NULL;
    > if (__builtin_constant_p(n) && __builtin_constant_p(size))
    > - return kmalloc(n * size, flags);
    > - return __kmalloc(n * size, flags);
    > + return kmalloc(bytes, flags);
    > + return __kmalloc(bytes, flags);
    > }
    >
    > /**
    > @@ -639,7 +646,9 @@ static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
    > */
    > static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
    > {
    > - return kmalloc_array(n, size, flags | __GFP_ZERO);
    > + size_t bytes = array_size(n, size);
    > +
    > + return kmalloc(bytes, flags | __GFP_ZERO);
    > }

    Hmm. I wonder why we have the kmalloc/__kmalloc "optimisation"
    in kmalloc_array, but not kcalloc. Bet we don't really need it in
    kmalloc_array. I'll do some testing.

    \
     
     \ /
      Last update: 2018-05-09 13:36    [W:4.099 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site