netdev - Re: [PATCH net 3/3] hv_netvsc: Fix the default receive buffer size

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]

Date:   Fri, 8 Dec 2017 13:44:59 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     kys@...rosoft.com, haiyangz@...rosoft.com, sthemmin@...rosoft.com,
        devel@...uxdriverproject.org, netdev@...r.kernel.org
Subject: Re: [PATCH net 3/3] hv_netvsc: Fix the default receive buffer size

On Thu, Dec 07, 2017 at 04:10:55PM -0800, Stephen Hemminger wrote:
> From: Haiyang Zhang <haiyangz@...rosoft.com>
> 
> The intended size is 16 MB, and the default slot size is 1728.
> So, NETVSC_DEFAULT_RX should be 16*1024*1024 / 1728 = 9709.
> 
> Fixes: 5023a6db73196 ("netvsc: increase default receive buffer size")
> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index dc70de674ca9..edfcde5d3621 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -50,7 +50,7 @@
>  #define NETVSC_MIN_TX_SECTIONS	10
>  #define NETVSC_DEFAULT_TX	192	/* ~1M */
>  #define NETVSC_MIN_RX_SECTIONS	10	/* ~64K */
> -#define NETVSC_DEFAULT_RX	10485   /* Max ~16M */
> +#define NETVSC_DEFAULT_RX	9709    /* ~16M */

How does this bug look like to the user?  Memory corruption?

It's weird to me reviewing this code that the default sizes are
stored in netvsc_drv.c and the max sizes are stored in hyperv_net.h.
Could we move these to hyperv_net.h?  We could write it like:
#define NETVSC_DEFAULT_RX ((16 * 1024 * 1024) / NETVSC_RECV_SECTION_SIZE)

16MB is sort of a weird default because it's larger than the 15MB
allowed for legacy versions, but it's smaller than the 32MB you'd want
for the current versions.

regards,
dan carpenter