| Message ID | 63453491987be2b31062449bd59224faca9f546a.1625486802.git.wangyunjian@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] virtio_net: check virtqueue_add_sgs() return value | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: rusty@rustcorp.com.au; 2 maintainers not CCed: rusty@rustcorp.com.au virtualization@lists.linux-foundation.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 17 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
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? > 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. > > if (unlikely(!virtqueue_kick(vi->cvq))) > return vi->ctrl->status == VIRTIO_NET_OK; > -- > 2.23.0
> -----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;