This patch fixs coverity issue by adding initialization step before using temporary virtio header. Coverity issue: 366181, 366123 Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers in Rx-mrg") Cc: stable@dpdk.org Signed-off-by: Marvin Liu <yong.liu@intel.com> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 583bf379c6..fe464b3088 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, hdr_mbuf = m; hdr_addr = buf_addr; - if (unlikely(buf_len < dev->vhost_hlen)) + if (unlikely(buf_len < dev->vhost_hlen)) { + memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf)); hdr = &tmp_hdr; - else + } else hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr; VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n", -- 2.17.1
On 3/3/21 8:27 AM, Marvin Liu wrote:
> This patch fixs coverity issue by adding initialization step before
> using temporary virtio header.
>
> Coverity issue: 366181, 366123
> Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers in Rx-mrg")
> Cc: stable@dpdk.org
>
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
>
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 583bf379c6..fe464b3088 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
>
> hdr_mbuf = m;
> hdr_addr = buf_addr;
> - if (unlikely(buf_len < dev->vhost_hlen))
> + if (unlikely(buf_len < dev->vhost_hlen)) {
> + memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf));
> hdr = &tmp_hdr;
> - else
> + } else
> hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr;
>
> VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n",
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin > Sent: Wednesday, March 24, 2021 5:55 PM > To: Marvin Liu <yong.liu@intel.com>; chenbo.xia@intel.com > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] vhost: fix accessing uninitialized variables > > > > On 3/3/21 8:27 AM, Marvin Liu wrote: > > This patch fixs coverity issue by adding initialization step before > > using temporary virtio header. > > > > Coverity issue: 366181, 366123 > > Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers > > in Rx-mrg") > > Cc: stable@dpdk.org > > > > Signed-off-by: Marvin Liu <yong.liu@intel.com> > > > > diff --git a/lib/librte_vhost/virtio_net.c > > b/lib/librte_vhost/virtio_net.c index 583bf379c6..fe464b3088 100644 > > --- a/lib/librte_vhost/virtio_net.c > > +++ b/lib/librte_vhost/virtio_net.c > > @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct > > vhost_virtqueue *vq, > > > > hdr_mbuf = m; > > hdr_addr = buf_addr; > > - if (unlikely(buf_len < dev->vhost_hlen)) > > + if (unlikely(buf_len < dev->vhost_hlen)) { > > + memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf)); > > hdr = &tmp_hdr; > > - else > > + } else > > hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr; > > > > VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n", > > I think it's better to revise it in this way: diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 583bf37..ccb73b9 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -420,6 +420,8 @@ net_hdr->csum_offset = (offsetof(struct rte_sctp_hdr, cksum)); break; + default: + ASSIGN_UNLESS_EQUAL(net_hdr->csum_offset, 0); } > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Thanks, > Maxime
> -----Original Message----- > From: wangyunjian <wangyunjian@huawei.com> > Sent: Saturday, March 27, 2021 6:06 PM > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Liu, Yong > <yong.liu@intel.com>; Xia, Chenbo <chenbo.xia@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] vhost: fix accessing uninitialized variables > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maxime Coquelin > > Sent: Wednesday, March 24, 2021 5:55 PM > > To: Marvin Liu <yong.liu@intel.com>; chenbo.xia@intel.com > > Cc: dev@dpdk.org; stable@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] vhost: fix accessing uninitialized variables > > > > > > > > On 3/3/21 8:27 AM, Marvin Liu wrote: > > > This patch fixs coverity issue by adding initialization step before > > > using temporary virtio header. > > > > > > Coverity issue: 366181, 366123 > > > Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers > > > in Rx-mrg") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Marvin Liu <yong.liu@intel.com> > > > > > > diff --git a/lib/librte_vhost/virtio_net.c > > > b/lib/librte_vhost/virtio_net.c index 583bf379c6..fe464b3088 100644 > > > --- a/lib/librte_vhost/virtio_net.c > > > +++ b/lib/librte_vhost/virtio_net.c > > > @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, > struct > > > vhost_virtqueue *vq, > > > > > > hdr_mbuf = m; > > > hdr_addr = buf_addr; > > > - if (unlikely(buf_len < dev->vhost_hlen)) > > > + if (unlikely(buf_len < dev->vhost_hlen)) { > > > + memset(&tmp_hdr, 0, sizeof(struct > virtio_net_hdr_mrg_rxbuf)); > > > hdr = &tmp_hdr; > > > - else > > > + } else > > > hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr; > > > > > > VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n", > > > > > I think it's better to revise it in this way: > Thanks, yunjian. This patch for reported coverity issue. The problem came from the read of net_hdr->csum_offset when using macro ASSIGN_UNLESS_EQUAL. When net_hdr not completed in the first buffer, temporary net_hdr will be used which hasn't been initialized. Regards, Marvin > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index 583bf37..ccb73b9 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -420,6 +420,8 @@ > net_hdr->csum_offset = (offsetof(struct rte_sctp_hdr, > cksum)); > break; > + default: > + ASSIGN_UNLESS_EQUAL(net_hdr->csum_offset, 0); > } > > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > Thanks, > > Maxime
Hi Marvin, > -----Original Message----- > From: Liu, Yong <yong.liu@intel.com> > Sent: Wednesday, March 3, 2021 3:28 PM > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com> > Cc: dev@dpdk.org; Liu, Yong <yong.liu@intel.com>; stable@dpdk.org > Subject: [PATCH] vhost: fix accessing uninitialized variables > > This patch fixs coverity issue by adding initialization step before > using temporary virtio header. > > Coverity issue: 366181, 366123 > Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers in Rx- > mrg") > Cc: stable@dpdk.org > > Signed-off-by: Marvin Liu <yong.liu@intel.com> > > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index 583bf379c6..fe464b3088 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct > vhost_virtqueue *vq, You should apply the same fix to async_mbuf_to_desc. Maybe you did not notice: one coverity issue is in copy_mbuf_to_desc, but another in async_mbuf_to_desc :) Thanks, Chenbo > > hdr_mbuf = m; > hdr_addr = buf_addr; > - if (unlikely(buf_len < dev->vhost_hlen)) > + if (unlikely(buf_len < dev->vhost_hlen)) { > + memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf)); > hdr = &tmp_hdr; > - else > + } else > hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr; > > VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n", > -- > 2.17.1
> -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Tuesday, April 6, 2021 2:18 PM > To: Liu, Yong <yong.liu@intel.com>; maxime.coquelin@redhat.com > Cc: dev@dpdk.org; stable@dpdk.org > Subject: RE: [PATCH] vhost: fix accessing uninitialized variables > > Hi Marvin, > > > -----Original Message----- > > From: Liu, Yong <yong.liu@intel.com> > > Sent: Wednesday, March 3, 2021 3:28 PM > > To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com> > > Cc: dev@dpdk.org; Liu, Yong <yong.liu@intel.com>; stable@dpdk.org > > Subject: [PATCH] vhost: fix accessing uninitialized variables > > > > This patch fixs coverity issue by adding initialization step before > > using temporary virtio header. > > > > Coverity issue: 366181, 366123 > > Fixes: fb3815cc614d ("vhost: handle virtually non-contiguous buffers in Rx- > > mrg") > > Cc: stable@dpdk.org > > > > Signed-off-by: Marvin Liu <yong.liu@intel.com> > > > > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > > index 583bf379c6..fe464b3088 100644 > > --- a/lib/librte_vhost/virtio_net.c > > +++ b/lib/librte_vhost/virtio_net.c > > @@ -808,9 +808,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct > > vhost_virtqueue *vq, > > You should apply the same fix to async_mbuf_to_desc. > Maybe you did not notice: one coverity issue is in copy_mbuf_to_desc, but > another > in async_mbuf_to_desc :) > Thanks for remind, will fix function async_mbuf_to_desc in next version. Regards, Marvin > Thanks, > Chenbo > > > > > hdr_mbuf = m; > > hdr_addr = buf_addr; > > -if (unlikely(buf_len < dev->vhost_hlen)) > > +if (unlikely(buf_len < dev->vhost_hlen)) { > > +memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf)); > > hdr = &tmp_hdr; > > -else > > +} else > > hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr; > > > > VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n", > > -- > > 2.17.1 >