netdev - Re: [PATCH net V3 1/2] ptr_ring: try vmalloc() when kmalloc() fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180208063509-mutt-send-email-mst@kernel.org>
Date:   Thu, 8 Feb 2018 06:45:51 +0200
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net V3 1/2] ptr_ring: try vmalloc() when kmalloc() fails

On Thu, Feb 08, 2018 at 11:59:24AM +0800, Jason Wang wrote:
> This patch switch to use kvmalloc_array() for using a vmalloc()
> fallback to help in case kmalloc() fails.
> 
> Reported-by: syzbot+e4d4f9ddd4295539735d@...kaller.appspotmail.com
> Fixes: 2e0ab8ca83c12 ("ptr_ring: array based FIFO for pointers")

I guess the actual patch is the one that switches tun to ptr_ring.

In fact, I think the actual bugfix is patch 2/2. This specific one
just makes kmalloc less likely to fail but that's
not what syzbot reported.

Then I would add this patch on top to make kmalloc less likely to fail.

> Signed-off-by: Jason Wang <jasowang@...hat.com>

> ---
>  include/linux/ptr_ring.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> index 1883d61..2af71a7 100644
> --- a/include/linux/ptr_ring.h
> +++ b/include/linux/ptr_ring.h
> @@ -466,7 +466,7 @@ static inline int ptr_ring_consume_batched_bh(struct ptr_ring *r,
>  
>  static inline void **__ptr_ring_init_queue_alloc(unsigned int size, gfp_t gfp)
>  {
> -	return kcalloc(size, sizeof(void *), gfp);
> +	return kvmalloc_array(size, sizeof(void *), gfp | __GFP_ZERO);
>  }
>  
>  static inline void __ptr_ring_set_size(struct ptr_ring *r, int size)

This implies a bunch of limitations on the flags. From kvmalloc_node
docs:

 * Reclaim modifiers - __GFP_NORETRY and __GFP_NOFAIL are not supported.
 * __GFP_RETRY_MAYFAIL is supported, and it should be used only if kmalloc is
 * preferable to the vmalloc fallback, due to visible performance drawbacks.

Fine with all the current users, but if we go this way, please add
documentation so future users don't misuse this API.
Alternatively, test flags and call kvmalloc or kcalloc?

> @@ -601,7 +601,7 @@ static inline int ptr_ring_resize(struct ptr_ring *r, int size, gfp_t gfp,
>  	spin_unlock(&(r)->producer_lock);
>  	spin_unlock_irqrestore(&(r)->consumer_lock, flags);
>  
> -	kfree(old);
> +	kvfree(old);
>  
>  	return 0;
>  }
> @@ -641,7 +641,7 @@ static inline int ptr_ring_resize_multiple(struct ptr_ring **rings,
>  	}
>  
>  	for (i = 0; i < nrings; ++i)
> -		kfree(queues[i]);
> +		kvfree(queues[i]);
>  
>  	kfree(queues);
>  
> @@ -649,7 +649,7 @@ static inline int ptr_ring_resize_multiple(struct ptr_ring **rings,
>  
>  nomem:
>  	while (--i >= 0)
> -		kfree(queues[i]);
> +		kvfree(queues[i]);
>  
>  	kfree(queues);
>  
> @@ -664,7 +664,7 @@ static inline void ptr_ring_cleanup(struct ptr_ring *r, void (*destroy)(void *))
>  	if (destroy)
>  		while ((ptr = ptr_ring_consume(r)))
>  			destroy(ptr);
> -	kfree(r->queue);
> +	kvfree(r->queue);
>  }
>  
>  #endif /* _LINUX_PTR_RING_H  */
> -- 
> 2.7.4