| Message ID | 63453491987be2b31062449bd59224faca9f546a.1625486802.git.wangyunjian@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net] virtio_net: check virtqueue_add_sgs() return value | expand |
> -----Original Message----- > From: Michael S. Tsirkin [mailto:mst@redhat.com] > Sent: Tuesday, July 6, 2021 2:08 AM > To: wangyunjian <wangyunjian@huawei.com> > Cc: kuba@kernel.org; davem@davemloft.net; netdev@vger.kernel.org; > jasowang@redhat.com; dingxiaoxiong <dingxiaoxiong@huawei.com> > Subject: Re: [PATCH net] virtio_net: check virtqueue_add_sgs() return value > > On Mon, Jul 05, 2021 at 09:53:39PM +0800, wangyunjian wrote: > > From: Yunjian Wang <wangyunjian@huawei.com> > > > > As virtqueue_add_sgs() can fail, we should check the return value. > > > > Addresses-Coverity-ID: 1464439 ("Unchecked return value") > > Fixes: a7c58146cf9a ("virtio_net: don't crash if virtqueue is > > broken.") > > What does this have to do with it? It's this commit that removes the check. So delete this fix tag? > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > drivers/net/virtio_net.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index > > b0b81458ca94..2b852578551e 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -1743,6 +1743,7 @@ static bool virtnet_send_command(struct > > virtnet_info *vi, u8 class, u8 cmd, { > > struct scatterlist *sgs[4], hdr, stat; > > unsigned out_num = 0, tmp; > > + int ret; > > > > /* Caller should know better */ > > BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)); @@ > > -1762,7 +1763,9 @@ static bool virtnet_send_command(struct virtnet_info > *vi, u8 class, u8 cmd, > > sgs[out_num] = &stat; > > > > BUG_ON(out_num + 1 > ARRAY_SIZE(sgs)); > > - virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC); > > + ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC); > > + if (ret < 0) > > + return false; > > and maybe dev_warn here. these things should not happen. OK, I will add it in next version. Thanks Yunjian > > > > > > if (unlikely(!virtqueue_kick(vi->cvq))) > > return vi->ctrl->status == VIRTIO_NET_OK; > > -- > > 2.23.0
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index b0b81458ca94..2b852578551e 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1743,6 +1743,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd, { struct scatterlist *sgs[4], hdr, stat; unsigned out_num = 0, tmp; + int ret; /* Caller should know better */ BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)); @@ -1762,7 +1763,9 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd, sgs[out_num] = &stat; BUG_ON(out_num + 1 > ARRAY_SIZE(sgs)); - virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC); + ret = virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC); + if (ret < 0) + return false; if (unlikely(!virtqueue_kick(vi->cvq))) return vi->ctrl->status == VIRTIO_NET_OK;