| Message ID | 20210522080231.54760-1-geffrey.guo@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-net: fix the kzalloc/kfree mismatch problem | expand |
> -----Original Message----- > From: Max Gurtovoy [mailto:mgurtovoy@nvidia.com] > Sent: Sunday, May 23, 2021 15:25 > To: Guodeqing (A) <geffrey.guo@huawei.com>; mst@redhat.com > Cc: jasowang@redhat.com; davem@davemloft.net; kuba@kernel.org; > virtualization@lists.linux-foundation.org; netdev@vger.kernel.org > Subject: Re: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem > > > On 5/22/2021 11:02 AM, guodeqing wrote: > > If the virtio_net device does not suppurt the ctrl queue feature, the > > vi->ctrl was not allocated, so there is no need to free it. > > you don't need this check. > > from kfree doc: > > "If @objp is NULL, no operation is performed." > > This is not a bug. I've set vi->ctrl to be NULL in case !vi->has_cvq. > > yes, this is not a bug, the patch is just a optimization, because the vi->ctrl maybe be freed which was not allocated, this may give people a misunderstanding. Thanks. > > > > Here I adjust the initialization sequence and the check of vi->has_cvq > > to slove this problem. > > > > Fixes: 122b84a1267a ("virtio-net: don't allocate control_buf if not > supported") > > Signed-off-by: guodeqing <geffrey.guo@huawei.com> > > --- > > drivers/net/virtio_net.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index > > 9b6a4a875c55..894f894d3a29 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -2691,7 +2691,8 @@ static void virtnet_free_queues(struct > > virtnet_info *vi) > > > > kfree(vi->rq); > > kfree(vi->sq); > > - kfree(vi->ctrl); > > + if (vi->has_cvq) > > + kfree(vi->ctrl); > > } > > > > static void _free_receive_bufs(struct virtnet_info *vi) @@ -2870,13 > > +2871,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) > > { > > int i; > > > > - if (vi->has_cvq) { > > - vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); > > - if (!vi->ctrl) > > - goto err_ctrl; > > - } else { > > - vi->ctrl = NULL; > > - } > > vi->sq = kcalloc(vi->max_queue_pairs, sizeof(*vi->sq), GFP_KERNEL); > > if (!vi->sq) > > goto err_sq; > > @@ -2884,6 +2878,12 @@ static int virtnet_alloc_queues(struct > virtnet_info *vi) > > if (!vi->rq) > > goto err_rq; > > > > + if (vi->has_cvq) { > > + vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); > > + if (!vi->ctrl) > > + goto err_ctrl; > > + } > > + > > INIT_DELAYED_WORK(&vi->refill, refill_work); > > for (i = 0; i < vi->max_queue_pairs; i++) { > > vi->rq[i].pages = NULL; > > @@ -2902,11 +2902,11 @@ static int virtnet_alloc_queues(struct > > virtnet_info *vi) > > > > return 0; > > > > +err_ctrl: > > + kfree(vi->rq); > > err_rq: > > kfree(vi->sq); > > err_sq: > > - kfree(vi->ctrl); > > -err_ctrl: > > return -ENOMEM; > > } > >
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 9b6a4a875c55..894f894d3a29 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2691,7 +2691,8 @@ static void virtnet_free_queues(struct virtnet_info *vi) kfree(vi->rq); kfree(vi->sq); - kfree(vi->ctrl); + if (vi->has_cvq) + kfree(vi->ctrl); } static void _free_receive_bufs(struct virtnet_info *vi) @@ -2870,13 +2871,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) { int i; - if (vi->has_cvq) { - vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); - if (!vi->ctrl) - goto err_ctrl; - } else { - vi->ctrl = NULL; - } vi->sq = kcalloc(vi->max_queue_pairs, sizeof(*vi->sq), GFP_KERNEL); if (!vi->sq) goto err_sq; @@ -2884,6 +2878,12 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) if (!vi->rq) goto err_rq; + if (vi->has_cvq) { + vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); + if (!vi->ctrl) + goto err_ctrl; + } + INIT_DELAYED_WORK(&vi->refill, refill_work); for (i = 0; i < vi->max_queue_pairs; i++) { vi->rq[i].pages = NULL; @@ -2902,11 +2902,11 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) return 0; +err_ctrl: + kfree(vi->rq); err_rq: kfree(vi->sq); err_sq: - kfree(vi->ctrl); -err_ctrl: return -ENOMEM; }
If the virtio_net device does not suppurt the ctrl queue feature, the vi->ctrl was not allocated, so there is no need to free it. Here I adjust the initialization sequence and the check of vi->has_cvq to slove this problem. Fixes: 122b84a1267a ("virtio-net: don't allocate control_buf if not supported") Signed-off-by: guodeqing <geffrey.guo@huawei.com> --- drivers/net/virtio_net.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)