netdev - Re: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YLeArcxMkty2n/Xz@unreal>
Date:   Wed, 2 Jun 2021 15:59:25 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Jason Wang <jasowang@...hat.com>
Cc:     Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
        "Guodeqing (A)" <geffrey.guo@...wei.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Max Gurtovoy <mgurtovoy@...dia.com>,
        "mst@...hat.com" <mst@...hat.com>
Subject: Re: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem

On Wed, Jun 02, 2021 at 03:19:46PM +0800, Jason Wang wrote:
> 
> 在 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@...wei.com> wrote:
> > > > > > -----Original Message-----
> > > > > > From: Max Gurtovoy [mailto:mgurtovoy@...dia.com]
> > > > > > Sent: Sunday, May 23, 2021 15:25
> > > > > > To: Guodeqing (A) <geffrey.guo@...wei.com>; mst@...hat.com
> > > > > > Cc: jasowang@...hat.com; davem@...emloft.net; kuba@...nel.org;
> > > > > > virtualization@...ts.linux-foundation.org; netdev@...r.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.

The key words are "side effect". Once you centralize everything, you
won't want to see "if (foo) kfree(foo)" spaghetti code.

Of course such construction doesn't break anything, but the idea is
to reduce useless code and not add it.

Thanks

> 
> Thanks
> 
> 
> > 
>