From: "Guodeqing (A)" <geffrey.guo@huawei•com>
To: Max Gurtovoy <mgurtovoy@nvidia•com>,
Jason Wang <jasowang@redhat•com>,
Xuan Zhuo <xuanzhuo@linux•alibaba.com>
Cc: "davem@davemloft•net" <davem@davemloft•net>,
"kuba@kernel•org" <kuba@kernel•org>,
"virtualization@lists•linux-foundation.org"
<virtualization@lists•linux-foundation.org>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"mst@redhat•com" <mst@redhat•com>
Subject: RE: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem
Date: Tue, 25 May 2021 01:12:47 +0000 [thread overview]
Message-ID: <372caa22030e4d5d90e2bada303d8f77@huawei.com> (raw)
In-Reply-To: <1f23afa3-645e-81b6-76da-94c7806ef6ed@nvidia.com>
> -----Original Message-----
> From: Max Gurtovoy [mailto:mgurtovoy@nvidia•com]
> Sent: Monday, May 24, 2021 16:35
> To: Jason Wang <jasowang@redhat•com>; Xuan Zhuo
> <xuanzhuo@linux•alibaba.com>; Guodeqing (A) <geffrey.guo@huawei•com>
> Cc: davem@davemloft•net; kuba@kernel•org; virtualization@lists•linux-
> foundation.org; netdev@vger•kernel.org; mst@redhat•com
> Subject: Re: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem
>
>
> On 5/24/2021 5:37 AM, Jason Wang wrote:
> >
> > 在 2021/5/24 上午10:06, Xuan Zhuo 写道:
> >> On Mon, 24 May 2021 01:48:53 +0000, Guodeqing (A)
> >> <geffrey.guo@huawei•com> wrote:
> >>>
> >>>> -----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.
> >>
> >> I think it may be enough to add a comment, and the code does not need
> >> to be modified.
> >>
> >> Thanks.
> >
> >
> > Or even just leave the current code as is. A lot of kernel codes was
> > wrote under the assumption that kfree() should deal with NULL.
> >
> > Thanks
> >
> >
> exactly.
>
>
> >>
> >>>>> 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")
>
>
> "Fixes" line should be added only if you fix some bug.
>
Ok, I see.
Thanks.
>
> >>>>> 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;
> >>>>> }
> >>>>>
> >
next prev parent reply other threads:[~2021-05-25 1:12 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1621821978.04102-1-xuanzhuo@linux.alibaba.com>
2021-05-24 2:37 ` [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem Jason Wang
2021-05-24 8:34 ` Max Gurtovoy
2021-05-25 1:12 ` Guodeqing (A) [this message]
2021-05-25 1:11 ` Guodeqing (A)
2021-06-02 5:50 ` Leon Romanovsky
2021-06-02 7:19 ` Jason Wang
2021-06-02 12:59 ` Leon Romanovsky
2021-05-22 8:02 guodeqing
2021-05-23 7:24 ` Max Gurtovoy
2021-05-24 1:48 ` Guodeqing (A)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=372caa22030e4d5d90e2bada303d8f77@huawei.com \
--to=geffrey.guo@huawei$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jasowang@redhat$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=mgurtovoy@nvidia$(echo .)com \
--cc=mst@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=virtualization@lists$(echo .)linux-foundation.org \
--cc=xuanzhuo@linux$(echo .)alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox