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.
