netdev - RE: [PATCH] hv_netvsc: Fix potentionally overflow in netvsc_xdp

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 for Android: free password hash cracker in your pocket

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

Message-ID: <MW2SPR01MB07B9C0C7ABCE2F81950BAACAB79@MW2SPR01MB07.namprd21.prod.outlook.com>
Date:   Wed, 13 Oct 2021 12:44:11 +0000
From:   Haiyang Zhang <haiyangz@...rosoft.com>
To:     Jiasheng Jiang <jiasheng@...as.ac.cn>,
        KY Srinivasan <kys@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "hawk@...nel.org" <hawk@...nel.org>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "kafai@...com" <kafai@...com>,
        "songliubraving@...com" <songliubraving@...com>,
        "yhs@...com" <yhs@...com>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: RE: [PATCH] hv_netvsc: Fix potentionally overflow in
 netvsc_xdp_xmit()



> -----Original Message-----
> From: Jiasheng Jiang <jiasheng@...as.ac.cn>
> Sent: Tuesday, October 12, 2021 11:05 PM
> To: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Stephen Hemminger <sthemmin@...rosoft.com>;
> wei.liu@...nel.org; Dexuan Cui <decui@...rosoft.com>;
> davem@...emloft.net; kuba@...nel.org; ast@...nel.org;
> daniel@...earbox.net; hawk@...nel.org; john.fastabend@...il.com;
> andrii@...nel.org; kafai@...com; songliubraving@...com; yhs@...com;
> kpsingh@...nel.org
> Cc: linux-hyperv@...r.kernel.org; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; bpf@...r.kernel.org; Jiasheng Jiang
> <jiasheng@...as.ac.cn>
> Subject: [PATCH] hv_netvsc: Fix potentionally overflow in
> netvsc_xdp_xmit()
> 
> [Some people who received this message don't often get email from
> jiasheng@...as.ac.cn. Learn why this is important at
> http://aka.ms/LearnAboutSenderIdentification.]
> 
> Adding skb_rx_queue_recorded() to avoid the value of skb->queue_mapping
> to be 0. Otherwise the return value of skb_get_rx_queue() could be
> MAX_U16
> cause by overflow.
> 
> Fixes: 351e158 ("hv_netvsc: Add XDP support")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>
> ---
>  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 f682a55..e51201e 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -807,7 +807,7 @@ static void netvsc_xdp_xmit(struct sk_buff *skb,
> struct net_device *ndev)
>  {
>         int rc;
> 
> -       skb->queue_mapping = skb_get_rx_queue(skb);
> +       skb->queue_mapping = skb_rx_queue_recorded(skb) ?
> skb_get_rx_queue(skb) : 0;
>         __skb_push(skb, ETH_HLEN);
> 

netvsc_xdp_xmit() is only called from netvsc_recv_callback() 
and after skb_record_rx_queue(skb, q_idx) is called:

        skb_record_rx_queue(skb, q_idx);

	  ......

        if (act == XDP_TX) {
                netvsc_xdp_xmit(skb, net);
                return NVSP_STAT_SUCCESS;
        }

So the existing code doesn't need this patch.

To avoid future misusing of netvsc_xdp_xmit() in other places, you
may just add a comment -- "This function should only be called 
after skb_record_rx_queue()".

Thanks,

- Haiyang