linux-kernel - Re: [PATCH net] virtio_net: adjust the execution order of function `virtnet

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEsVJcb2YYvfXYA0soE++cPEmQatkC0tB+shNKB=OTteWg@mail.gmail.com>
Date: Tue, 19 Aug 2025 10:48:37 +0800
From: Jason Wang <jasowang@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Junnan Wu <junnan01.wu@...sung.com>, andrew+netdev@...n.ch, davem@...emloft.net, 
	edumazet@...gle.com, eperezma@...hat.com, lei19.wang@...sung.com, 
	linux-kernel@...r.kernel.org, mst@...hat.com, netdev@...r.kernel.org, 
	pabeni@...hat.com, q1.huang@...sung.com, virtualization@...ts.linux.dev, 
	xuanzhuo@...ux.alibaba.com, ying123.xu@...sung.com
Subject: Re: [PATCH net] virtio_net: adjust the execution order of function
 `virtnet_close` during freeze

On Mon, Aug 18, 2025 at 11:39 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon, 18 Aug 2025 09:15:22 +0800 Junnan Wu wrote:
> > > > Yes, you are right. The commit of this fix tag is the first commit I
> > > > found which add function `virtnet_poll_cleantx`. Actually, we are not
> > > > sure whether this issue appears after this commit.
> > > >
> > > > In our side, this issue is found by chance in version 5.15.
> > > >
> > > > It's hard to find the key commit which cause this issue
> > > > for reason that the reproduction of this scenario is too complex.
> > >
> > > I think the problem needs to be more clearly understood, and then it
> > > will be easier to find the fixes tag. At the face of it the patch
> > > makes it look like close() doesn't reliably stop the device, which
> > > is highly odd.
> >
> > Yes, you are right. It is really strange that `close()` acts like
> > that, because current order has worked for long time. But panic call
> > stack in our env shows that the function `virtnet_close` and
> > `netif_device_detach` should have a correct execution order. And it
> > needs more time to find the fixes tag. I wonder that is it must have
> > fixes tag to merge?
> >
> > By the way, you mentioned that "the problem need to be more clearly
> > understood", did you mean the descriptions and sequences in commit
> > message are not easy to understand? Do you have some suggestions
> > about this?
>
> Perhaps Jason gets your explanation and will correct me, but to me it
> seems like the fix is based on trial and error rather than clear
> understanding of the problem. If you understood the problem clearly
> you should be able to find the Fixes tag without a problem..
>

+1

The code looks fine but the fixes tag needs to be correct.

Thanks