[bug report] hv_netvsc: Copy packets sent by Hyper-V out of the receive buffer

[bug report] hv_netvsc: Copy packets sent by Hyper-V out of the receive buffer

[Dan Carpenter]

Hello Andrea Parri (Microsoft),

This is a semi-automatic email about new static checker warnings.

The patch 0ba35fe91ce3: "hv_netvsc: Copy packets sent by Hyper-V out
of the receive buffer" from Jan 26, 2021, leads to the following
Smatch complaint:

    drivers/net/hyperv/rndis_filter.c:468 rsc_add_data()
    error: we previously assumed 'csum_info' could be null (see line 460)

drivers/net/hyperv/rndis_filter.c
   459			}
   460			if (csum_info != NULL) {
                            ^^^^^^^^^^^^^^^^^
"csum_info" can be NULL.

   461				memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));
   462				nvchan->rsc.ppi_flags |= NVSC_RSC_CSUM_INFO;
   463			} else {
   464				nvchan->rsc.ppi_flags &= ~NVSC_RSC_CSUM_INFO;
   465			}
   466			nvchan->rsc.pktlen = len;
   467			if (hash_info != NULL) {
   468				nvchan->rsc.csum_info = *csum_info;
                                                        ^^^^^^^^^^^
Unchecked dereference.  Plus this has "csum_info" on both sides of the
assignment so maybe it is a copy and paste error?  This is equivalent to
the "memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));"
in the ealier if statement but stated as an assignment instead of a
memcpy().

   469				nvchan->rsc.ppi_flags |= NVSC_RSC_HASH_INFO;
   470			} else {

regards,
dan carpenter

Re: [bug report] hv_netvsc: Copy packets sent by Hyper-V out of the receive buffer

[Andrea Parri]

Hi Dan,

On Wed, Feb 03, 2021 at 01:10:41PM +0300, Dan Carpenter wrote:
> Hello Andrea Parri (Microsoft),
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 0ba35fe91ce3: "hv_netvsc: Copy packets sent by Hyper-V out
> of the receive buffer" from Jan 26, 2021, leads to the following
> Smatch complaint:
> 
>     drivers/net/hyperv/rndis_filter.c:468 rsc_add_data()
>     error: we previously assumed 'csum_info' could be null (see line 460)
> 
> drivers/net/hyperv/rndis_filter.c
>    459			}
>    460			if (csum_info != NULL) {
>                             ^^^^^^^^^^^^^^^^^
> "csum_info" can be NULL.
> 
>    461				memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));
>    462				nvchan->rsc.ppi_flags |= NVSC_RSC_CSUM_INFO;
>    463			} else {
>    464				nvchan->rsc.ppi_flags &= ~NVSC_RSC_CSUM_INFO;
>    465			}
>    466			nvchan->rsc.pktlen = len;
>    467			if (hash_info != NULL) {
>    468				nvchan->rsc.csum_info = *csum_info;
>                                                         ^^^^^^^^^^^
> Unchecked dereference.  Plus this has "csum_info" on both sides of the
> assignment so maybe it is a copy and paste error?  This is equivalent to
> the "memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));"
> in the ealier if statement but stated as an assignment instead of a
> memcpy().

Right, I did realize about the error and I'm about to send a fix for it.

Thank you for the report,

  Andrea

Re: [bug report] hv_netvsc: Copy packets sent by Hyper-V out of the receive buffer

[Andrea Parri]

On Wed, Feb 03, 2021 at 11:45:44AM +0100, Andrea Parri wrote:
> Hi Dan,
> 
> On Wed, Feb 03, 2021 at 01:10:41PM +0300, Dan Carpenter wrote:
> > Hello Andrea Parri (Microsoft),
> > 
> > This is a semi-automatic email about new static checker warnings.
> > 
> > The patch 0ba35fe91ce3: "hv_netvsc: Copy packets sent by Hyper-V out
> > of the receive buffer" from Jan 26, 2021, leads to the following
> > Smatch complaint:
> > 
> >     drivers/net/hyperv/rndis_filter.c:468 rsc_add_data()
> >     error: we previously assumed 'csum_info' could be null (see line 460)
> > 
> > drivers/net/hyperv/rndis_filter.c
> >    459			}
> >    460			if (csum_info != NULL) {
> >                             ^^^^^^^^^^^^^^^^^
> > "csum_info" can be NULL.
> > 
> >    461				memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));
> >    462				nvchan->rsc.ppi_flags |= NVSC_RSC_CSUM_INFO;
> >    463			} else {
> >    464				nvchan->rsc.ppi_flags &= ~NVSC_RSC_CSUM_INFO;
> >    465			}
> >    466			nvchan->rsc.pktlen = len;
> >    467			if (hash_info != NULL) {
> >    468				nvchan->rsc.csum_info = *csum_info;
> >                                                         ^^^^^^^^^^^
> > Unchecked dereference.  Plus this has "csum_info" on both sides of the
> > assignment so maybe it is a copy and paste error?  This is equivalent to
> > the "memcpy(&nvchan->rsc.csum_info, csum_info, sizeof(*csum_info));"
> > in the ealier if statement but stated as an assignment instead of a
> > memcpy().
> 
> Right, I did realize about the error and I'm about to send a fix for it.

For reference,

  https://lkml.kernel.org/r/20210203113513.558864-3-parri.andrea@gmail.com/

  Andrea