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

Message-Id: <20250819033326.3602994-1-junnan01.wu@samsung.com>
Date: Tue, 19 Aug 2025 11:33:26 +0800
From: Junnan Wu <junnan01.wu@...sung.com>
To: jasowang@...hat.com
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	eperezma@...hat.com, junnan01.wu@...sung.com, kuba@...nel.org,
	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 Tue, 19 Aug 2025 10:48:37 +0800 Jason Wang wrote:
> 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

Well, I will do some works to find out the fixes tag.
Once there's progress, I will let you know.