Virtualization - Jun 2021 - [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem

On Mon, May 24, 2021 at 10:37:14AM +0800, Jason Wang
wrote:
> 
> ? 2021/5/24 ??10:06, Xuan Zhuo ??:
> > On Mon, 24 May 2021 01:48:53 +0000, Guodeqing (A) <geffrey.guo at
huawei.com> wrote:
> > > 
> > > > -----Original Message-----
> > > > From: Max Gurtovoy [mailto:mgurtovoy at nvidia.com]
> > > > Sent: Sunday, May 23, 2021 15:25
> > > > To: Guodeqing (A) <geffrey.guo at huawei.com>; mst at
redhat.com
> > > > Cc: jasowang at redhat.com; davem at davemloft.net; kuba at
kernel.org;
> > > > virtualization at lists.linux-foundation.org; netdev at
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.
It is not assumption but standard practice that can be seen as side
effect of "7) Centralized exiting of functions" section of
coding-style.rst.

Thanks

? 2021/6/2 ??1:50, Leon Romanovsky ??:
> On Mon, May 24, 2021 at 10:37:14AM +0800, Jason Wang wrote:
>> ? 2021/5/24 ??10:06, Xuan Zhuo ??:
>>> On Mon, 24 May 2021 01:48:53 +0000, Guodeqing (A) <geffrey.guo
at huawei.com> wrote:
>>>>> -----Original Message-----
>>>>> From: Max Gurtovoy [mailto:mgurtovoy at nvidia.com]
>>>>> Sent: Sunday, May 23, 2021 15:25
>>>>> To: Guodeqing (A) <geffrey.guo at huawei.com>; mst at
redhat.com
>>>>> Cc: jasowang at redhat.com; davem at davemloft.net; kuba at
kernel.org;
>>>>> virtualization at lists.linux-foundation.org; netdev at
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.
> It is not assumption but standard practice that can be seen as side
> effect of "7) Centralized exiting of functions" section of
coding-style.rst.
>
> Thanks

I don't see the connection to the centralized exiting.

Something like:

if (foo)
 ??? kfree(foo);

won't break the centralization.

Thanks

>