* Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 7.0-stable tree
From: gregkh @ 2026-06-16 9:53 UTC (permalink / raw)
To: AVKrasnov, edumazet, eperezma, gregkh, jasowang, kuba, mst,
sgarzare, stefanha, virtualization, xuanzhuo
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
vsock/virtio: fix potential unbounded skb queue
to the 7.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
vsock-virtio-fix-potential-unbounded-skb-queue.patch
and it can be found in the queue-7.0 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 059b7dbd20a6f0c539a45ddff1573cb8946685b5 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 30 Apr 2026 12:26:52 +0000
Subject: vsock/virtio: fix potential unbounded skb queue
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Eric Dumazet <edumazet@google.com>
commit 059b7dbd20a6f0c539a45ddff1573cb8946685b5 upstream.
virtio_transport_inc_rx_pkt() checks vvs->rx_bytes + len > vvs->buf_alloc.
virtio_transport_recv_enqueue() skips coalescing for packets
with VIRTIO_VSOCK_SEQ_EOM.
If fed with packets with len == 0 and VIRTIO_VSOCK_SEQ_EOM,
a very large number of packets can be queued
because vvs->rx_bytes stays at 0.
Fix this by estimating the skb metadata size:
(Number of skbs in the queue) * SKB_TRUESIZE(0)
Fixes: 077706165717 ("virtio/vsock: don't use skbuff state to account credit")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: "Eugenio Pérez" <eperezma@redhat.com>
Cc: virtualization@lists.linux.dev
Link: https://patch.msgid.link/20260430122653.554058-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/vmw_vsock/virtio_transport_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -425,7 +425,9 @@ static int virtio_transport_send_pkt_inf
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
u32 len)
{
- if (vvs->buf_used + len > vvs->buf_alloc)
+ u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+
+ if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
return false;
vvs->rx_bytes += len;
Patches currently in stable-queue which might be from edumazet@google.com are
queue-7.0/ipv6-sit-reload-inner-ipv6-header-after-gso-offloads.patch
queue-7.0/ieee802154-6lowpan-only-accept-ipv6-packets-in-lowpa.patch
queue-7.0/udp-clear-skb-dev-before-running-a-sockmap-verdict.patch
queue-7.0/inet-frags-fix-use-after-free-caused-by-the-fqdir_pre_exit-flush.patch
queue-7.0/net-add-pskb_may_pull-to-skb_gro_receive_list.patch
queue-7.0/net-sched-act_api-use-rcu-with-deferred-freeing-for-.patch
queue-7.0/bonding-annotate-data-races-arcound-churn-variables.patch
queue-7.0/ip6_vti-fix-incorrect-tunnel-matching-in-vti6_tnl_lo.patch
queue-7.0/tcp-restrict-so_attach_filter-to-priv-users.patch
queue-7.0/vsock-virtio-fix-potential-unbounded-skb-queue.patch
queue-7.0/ipv6-mcast-fix-use-after-free-when-processing-mld-queries.patch
queue-7.0/tcp-add-preempt_-disable-enable-_nested-in-reqsk_que.patch
queue-7.0/ip6_vti-set-netns_immutable-on-the-fallback-device.patch
queue-7.0/rxrpc-fix-the-ack-parser-to-extract-the-sack-table-for-parsing.patch
queue-7.0/ipv4-restrict-ipopt_ssrr-and-ipopt_lsrr-options.patch
^ permalink raw reply
* Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.18-stable tree
From: gregkh @ 2026-06-16 9:50 UTC (permalink / raw)
To: AVKrasnov, edumazet, eperezma, gregkh, jasowang, kuba, mst,
sgarzare, stefanha, virtualization, xuanzhuo
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
vsock/virtio: fix potential unbounded skb queue
to the 6.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
vsock-virtio-fix-potential-unbounded-skb-queue.patch
and it can be found in the queue-6.18 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 059b7dbd20a6f0c539a45ddff1573cb8946685b5 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 30 Apr 2026 12:26:52 +0000
Subject: vsock/virtio: fix potential unbounded skb queue
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Eric Dumazet <edumazet@google.com>
commit 059b7dbd20a6f0c539a45ddff1573cb8946685b5 upstream.
virtio_transport_inc_rx_pkt() checks vvs->rx_bytes + len > vvs->buf_alloc.
virtio_transport_recv_enqueue() skips coalescing for packets
with VIRTIO_VSOCK_SEQ_EOM.
If fed with packets with len == 0 and VIRTIO_VSOCK_SEQ_EOM,
a very large number of packets can be queued
because vvs->rx_bytes stays at 0.
Fix this by estimating the skb metadata size:
(Number of skbs in the queue) * SKB_TRUESIZE(0)
Fixes: 077706165717 ("virtio/vsock: don't use skbuff state to account credit")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: "Eugenio Pérez" <eperezma@redhat.com>
Cc: virtualization@lists.linux.dev
Link: https://patch.msgid.link/20260430122653.554058-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/vmw_vsock/virtio_transport_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -425,7 +425,9 @@ static int virtio_transport_send_pkt_inf
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
u32 len)
{
- if (vvs->buf_used + len > vvs->buf_alloc)
+ u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+
+ if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
return false;
vvs->rx_bytes += len;
Patches currently in stable-queue which might be from edumazet@google.com are
queue-6.18/ipv6-sit-reload-inner-ipv6-header-after-gso-offloads.patch
queue-6.18/ieee802154-6lowpan-only-accept-ipv6-packets-in-lowpa.patch
queue-6.18/udp-clear-skb-dev-before-running-a-sockmap-verdict.patch
queue-6.18/inet-frags-fix-use-after-free-caused-by-the-fqdir_pre_exit-flush.patch
queue-6.18/net-add-pskb_may_pull-to-skb_gro_receive_list.patch
queue-6.18/net-sched-act_api-use-rcu-with-deferred-freeing-for-.patch
queue-6.18/ip6_vti-fix-incorrect-tunnel-matching-in-vti6_tnl_lo.patch
queue-6.18/tcp-restrict-so_attach_filter-to-priv-users.patch
queue-6.18/vsock-virtio-fix-potential-unbounded-skb-queue.patch
queue-6.18/ipv6-mcast-fix-use-after-free-when-processing-mld-queries.patch
queue-6.18/ip6_vti-set-netns_immutable-on-the-fallback-device.patch
queue-6.18/rxrpc-fix-the-ack-parser-to-extract-the-sack-table-for-parsing.patch
queue-6.18/ipv4-restrict-ipopt_ssrr-and-ipopt_lsrr-options.patch
^ permalink raw reply
* Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.12-stable tree
From: gregkh @ 2026-06-16 9:48 UTC (permalink / raw)
To: AVKrasnov, edumazet, eperezma, gregkh, jasowang, kuba, mst,
sgarzare, stefanha, virtualization, xuanzhuo
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
vsock/virtio: fix potential unbounded skb queue
to the 6.12-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
vsock-virtio-fix-potential-unbounded-skb-queue.patch
and it can be found in the queue-6.12 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 059b7dbd20a6f0c539a45ddff1573cb8946685b5 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 30 Apr 2026 12:26:52 +0000
Subject: vsock/virtio: fix potential unbounded skb queue
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Eric Dumazet <edumazet@google.com>
commit 059b7dbd20a6f0c539a45ddff1573cb8946685b5 upstream.
virtio_transport_inc_rx_pkt() checks vvs->rx_bytes + len > vvs->buf_alloc.
virtio_transport_recv_enqueue() skips coalescing for packets
with VIRTIO_VSOCK_SEQ_EOM.
If fed with packets with len == 0 and VIRTIO_VSOCK_SEQ_EOM,
a very large number of packets can be queued
because vvs->rx_bytes stays at 0.
Fix this by estimating the skb metadata size:
(Number of skbs in the queue) * SKB_TRUESIZE(0)
Fixes: 077706165717 ("virtio/vsock: don't use skbuff state to account credit")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: "Eugenio Pérez" <eperezma@redhat.com>
Cc: virtualization@lists.linux.dev
Link: https://patch.msgid.link/20260430122653.554058-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/vmw_vsock/virtio_transport_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -430,7 +430,9 @@ static int virtio_transport_send_pkt_inf
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
u32 len)
{
- if (vvs->buf_used + len > vvs->buf_alloc)
+ u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+
+ if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
return false;
vvs->rx_bytes += len;
Patches currently in stable-queue which might be from edumazet@google.com are
queue-6.12/ipv6-sit-reload-inner-ipv6-header-after-gso-offloads.patch
queue-6.12/ieee802154-6lowpan-only-accept-ipv6-packets-in-lowpa.patch
queue-6.12/udp-clear-skb-dev-before-running-a-sockmap-verdict.patch
queue-6.12/inet-frags-fix-use-after-free-caused-by-the-fqdir_pre_exit-flush.patch
queue-6.12/net-add-pskb_may_pull-to-skb_gro_receive_list.patch
queue-6.12/net_sched-act_pedit-use-rcu-in-tcf_pedit_dump.patch
queue-6.12/net-sched-act_api-use-rcu-with-deferred-freeing-for-.patch
queue-6.12/ip6_vti-fix-incorrect-tunnel-matching-in-vti6_tnl_lo.patch
queue-6.12/tcp-restrict-so_attach_filter-to-priv-users.patch
queue-6.12/vsock-virtio-fix-potential-unbounded-skb-queue.patch
queue-6.12/ipv6-mcast-fix-use-after-free-when-processing-mld-queries.patch
queue-6.12/ipv4-restrict-ipopt_ssrr-and-ipopt_lsrr-options.patch
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Greg KH @ 2026-06-16 9:43 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <CAGxU2F6PDqHKLsW97qLUg+7hWq=iYk5qDAGUuxWSbdkyEDmsQw@mail.gmail.com>
On Tue, Jun 16, 2026 at 10:36:43AM +0200, Stefano Garzarella wrote:
> On Tue, 16 Jun 2026 at 10:00, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 16, 2026 at 09:52:32AM +0200, Stefano Garzarella wrote:
> > > On Tue, Jun 16, 2026 at 10:17:31AM +0530, Greg KH wrote:
> > > > On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
> > > > > On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
> > > > > > > > What's the status of that fix?
> > > > > > >
> > > > > > > Stefano posted v3 and is working on v4.
> > > > > > >
> > > > > > > > Should it be reverted elsewhere?
> > > > > > >
> > > > > > > Donnu. With the change we have no DoS but the socket gets silently
> > > > > > > broken. Eric felt given the brokenness is upstream already it's better
> > > > > > > to work on a fix on top, not revert.
> > > > > >
> > > > > > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
> > > > > > follow-up once it lands upstream.
> > > > >
> > > > > FYI v4 is now merged in the net tree, so I guess they will land upstream
> > > > > soon. I CCed stable on both patches:
> > > > >
> > > > > a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> > > > > c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
> > > > > buf_alloc")
> > > > >
> > > > > Both are related, but the second is the main fix of this patch.
> > > >
> > > > THe second one doesn't apply at all :(
> > > >
> > >
> > > The second one is the fix of the patch originally added to stable queue by
> > > this thread, so should be applied on top of it (commit 059b7dbd20a6
> > > ("vsock/virtio: fix potential unbounded skb queue")).
> > >
> > > I'm working on improving memory management, but for now I think it makes
> > > sense to backport all three to the stable branches.
> > >
> > > So, in summary:
> > > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> > > a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> > > c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full buf_alloc")
> >
> > Again, this last one fails to apply everywhere :(
>
> Again, c6087c5aaad6 depends on 059b7dbd20a6 (as also indicated by the
> Fixes tag in the patch description).
>
> I don't know what you meant with "everywhere", but I just run `git
> cherry-pick 059b7dbd20a6 c6087c5aaad6` on linux-6.12.y, linux-6.18.y,
> and linux-7.0.y without any issue.
Sorry, I was just searching for the short-id, which is in commits
already in those trees. The real commit worked, sorry for the
confusion.
> On linux-6.6.y it's failing because we are missing zero-copy support in
> AF_VSOCK. So, I guess we didn't backport commit 45ca7e9f0730
> ("vsock/virtio: fix `rx_bytes` accounting for stream sockets") because
> there were conflicts. That patch is needed to apply commit 059b7dbd20a6
> ("vsock/virtio: fix potential unbounded skb queue") cleanly.
That commit does not backport cleanly to 6.6.y, so I still need a patch
series for that tree.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: Cornelia Huck @ 2026-06-16 9:16 UTC (permalink / raw)
To: Peter Oberparleiter, Halil Pasic, William Bezenah, vneethv
Cc: linux-s390, farman, hca, gor, agordeev, borntraeger, svens,
mjrosato, virtualization, kvm, linux-kernel
In-Reply-To: <2e543ef5-1aa8-4ddc-a68a-103c7bdfe58d@linux.ibm.com>
On Tue, Jun 16 2026, Peter Oberparleiter <oberpar@linux.ibm.com> wrote:
> On 15.06.2026 23:42, Halil Pasic wrote:
>> On Mon, 15 Jun 2026 16:01:55 -0400
>> William Bezenah <wbezenah@linux.ibm.com> wrote:
>>
>>> On 6/15/2026 10:58 AM, Cornelia Huck wrote:
>>>> On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
>>>>
>>>>> On Fri, 12 Jun 2026 17:54:07 +0200
>>>>> William Bezenah <wbezenah@linux.ibm.com> wrote:
>>>>>
>>>>>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
>>>>>> subchannel based on DNV"), subchannel behavior following a device
>>>>>> detach has been updated and results in -EINVAL being propagated
>>>>>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
>>>>>> in cio/device_ops. In the end, the virtio driver has no ability to
>>>>>> react to the difference between device and subchannel states here,
>>>>>> and during detach, both -ENODEV and -EINVAL indicate the device
>>>>>> cannot be used and should not be treated as errors requiring
>>>>>> attention. Update error handling in virtio_ccw_del_vq() and
>>>>>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
>>>>>> -ENODEV.
>>>>> Hi William!
>>>>>
>>>>> Are you saying that ccw_device_start() started returning -EINVAL
>>>>> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
>>>>> DNV")? Or did I somehow read the paragraph wrong?
>>>>>
>>>>> The funcition ccw_device_start is documented to return:
>>>>> * Returns:
>>>>> * %0, if the operation was successful;
>>>>> * -%EBUSY, if the device is busy, or status pending;
>>>>> * -%EACCES, if no path specified in @lpm is operational;
>>>>> * -%ENODEV, if the device is not operational.
>>>>> and the commit message does not say a thing about introducing -EINVAL to
>>>>> the mix.
>>>> The function may return -EINVAL for non-enabled subchannels
>>>> (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
>>>> I'd expect it not to be enabled in that case anyway.
>>>
>>> Yep, that's at least how I've come to understand what changed. The
>>> function ccw_device_start_timeout_key() has always returned -EINVAL
>>> for non-enabled subchannels (pmcw.ena == 0), though it's not
>>> documented in the header.
>>
>> Wasn't his -EINVAL actually introduced by commit:
>> 823d494ac111 ("[S390] pm: ccw bus power management callbacks")?
>
> In the context of virtio-ccw added in 2012, an EINVAL return code
> introduced in 2009 might be considered "always" :)
:)
I'm wondering whether we should still expect to hit the "ssch with
ena==0" situation, given that pm support has been removed again in the
meanwhile. (Well, other than in situations like this, where it is a
follow-up to other problems.) IOW, can callers expect not to see
-EINVAL, unless they are doing something really stupid?
>
>>> What changed with commit 8c58a229688c is that cio_update_schib() now
>>> updates the schib even when DNV=0, rather than returning early as it
>>> did previously. Somehow this update results in pmcw.ena == 0 in
>>> ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
>>> moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
>>> where it returned -ENODEV.
>>
>> Sounds fishy to me. As far as I understand the DNV takes precedence over
>> all other pieces of PMCW.
>
> And you're right about that! The Principles of Operation states (p. 15-4
> in SA22-7832-14 [1]) that the contents of all other fields in the PMCW
> are unpredictable when DNV is 0, therefore 8c58a229688c is in error.
>
> I'll work with Vineeth to determine how to fix this issue, potentially
> via manually clearing some relevant SCHIB fields instead of copying the
> unpredictable results of the STSCH instruction.
Can't you zero the whole SCHIB, or do you still need some of the
measurement block things for cleanup?
>
>>> So the commit didn't introduce -EINVAL as a new return value, rather,
>>> it changed the subchannel lifecycle such that existing paths now
>>> propagate -EINVAL rather than -ENODEV during the device detach
>>> scenario.
>>
>> I'm not convinced returning -EINVAL in the given situation is the
>> right thing to do. Peter, would you mind to chime in?
>
> I tend to agree that an attempt to start I/O for a subchannel that has
> DNV 0 should result in ENODEV rather than EINVAL, though the latter is
> still valid when a driver tries to start I/O on a subchannel that is not
> enabled for I/O.
>
> We'll make sure to design the fix for 8c58a229688c in away that ENODEV
> will be returned when DNV is 0. Assuming that this is the only situation
> where virtio-ccw's ccw_io_helper() receives -EINVAL from
> ccw_device_start__timeout_key() during detach, the subject patch should
> no longer be necessary.
I agree, I'd not expect to get -EINVAL in ccw_io_helper().
^ permalink raw reply
* Re: [PATCH] vhost/net: fix clear_user start address in VHOST_GET_FEATURES_ARRAY
From: rom.wang @ 2026-06-16 9:01 UTC (permalink / raw)
To: r4o5m6e8o
Cc: eperezma, jasowang, kvm, linux-kernel, mst, netdev, pabeni,
virtualization, wangyufeng
In-Reply-To: <20260526080336.61296-1-r4o5m6e8o@163.com>
Gentle ping. Any comments on this patch?
Thanks
Yufeng Wang
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Stefano Garzarella @ 2026-06-16 8:36 UTC (permalink / raw)
To: Greg KH
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <2026061607-risotto-getaway-c57f@gregkh>
On Tue, 16 Jun 2026 at 10:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 16, 2026 at 09:52:32AM +0200, Stefano Garzarella wrote:
> > On Tue, Jun 16, 2026 at 10:17:31AM +0530, Greg KH wrote:
> > > On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
> > > > On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
> > > > > > > What's the status of that fix?
> > > > > >
> > > > > > Stefano posted v3 and is working on v4.
> > > > > >
> > > > > > > Should it be reverted elsewhere?
> > > > > >
> > > > > > Donnu. With the change we have no DoS but the socket gets silently
> > > > > > broken. Eric felt given the brokenness is upstream already it's better
> > > > > > to work on a fix on top, not revert.
> > > > >
> > > > > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
> > > > > follow-up once it lands upstream.
> > > >
> > > > FYI v4 is now merged in the net tree, so I guess they will land upstream
> > > > soon. I CCed stable on both patches:
> > > >
> > > > a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> > > > c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
> > > > buf_alloc")
> > > >
> > > > Both are related, but the second is the main fix of this patch.
> > >
> > > THe second one doesn't apply at all :(
> > >
> >
> > The second one is the fix of the patch originally added to stable queue by
> > this thread, so should be applied on top of it (commit 059b7dbd20a6
> > ("vsock/virtio: fix potential unbounded skb queue")).
> >
> > I'm working on improving memory management, but for now I think it makes
> > sense to backport all three to the stable branches.
> >
> > So, in summary:
> > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> > a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> > c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full buf_alloc")
>
> Again, this last one fails to apply everywhere :(
Again, c6087c5aaad6 depends on 059b7dbd20a6 (as also indicated by the
Fixes tag in the patch description).
I don't know what you meant with "everywhere", but I just run `git
cherry-pick 059b7dbd20a6 c6087c5aaad6` on linux-6.12.y, linux-6.18.y,
and linux-7.0.y without any issue.
On linux-6.6.y it's failing because we are missing zero-copy support in
AF_VSOCK. So, I guess we didn't backport commit 45ca7e9f0730
("vsock/virtio: fix `rx_bytes` accounting for stream sockets") because
there were conflicts. That patch is needed to apply commit 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue") cleanly.
Stefano
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: support xsk wake up
From: Eugenio Perez Martin @ 2026-06-16 8:35 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Menglong Dong, xuanzhuo, mst, jasowang, andrew+netdev, davem,
edumazet, pabeni, netdev, virtualization, linux-kernel
In-Reply-To: <20260613144612.0c5b7ba4@kernel.org>
On Sat, Jun 13, 2026 at 11:46 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 10 Jun 2026 10:27:28 +0200 Eugenio Perez Martin wrote:
> > And the From and Signed-off-by emails don't match, which I'm not sure is valid.
>
> It's clearly the same person. Please focus on the code, not trivial
> process issues.
>
> Quoting documentation:
>
> Reviewer guidance
> -----------------
>
> [...]
>
> Reviewers are highly encouraged to do more in-depth review of submissions
> and not focus exclusively on process issues, trivial or subjective
> matters like code formatting, tags etc.
>
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#reviewer-guidance
>
Ack'd, it was just a nitpick since the fixes tag was already needed.
Thanks for the doc pointer, I agree with that so I'll try to avoid
these nits in the future!
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Greg KH @ 2026-06-16 7:59 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <ajD_FBEak8hKNdIK@sgarzare-redhat>
On Tue, Jun 16, 2026 at 09:52:32AM +0200, Stefano Garzarella wrote:
> On Tue, Jun 16, 2026 at 10:17:31AM +0530, Greg KH wrote:
> > On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
> > > On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
> > > > > > What's the status of that fix?
> > > > >
> > > > > Stefano posted v3 and is working on v4.
> > > > >
> > > > > > Should it be reverted elsewhere?
> > > > >
> > > > > Donnu. With the change we have no DoS but the socket gets silently
> > > > > broken. Eric felt given the brokenness is upstream already it's better
> > > > > to work on a fix on top, not revert.
> > > >
> > > > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
> > > > follow-up once it lands upstream.
> > >
> > > FYI v4 is now merged in the net tree, so I guess they will land upstream
> > > soon. I CCed stable on both patches:
> > >
> > > a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> > > c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
> > > buf_alloc")
> > >
> > > Both are related, but the second is the main fix of this patch.
> >
> > THe second one doesn't apply at all :(
> >
>
> The second one is the fix of the patch originally added to stable queue by
> this thread, so should be applied on top of it (commit 059b7dbd20a6
> ("vsock/virtio: fix potential unbounded skb queue")).
>
> I'm working on improving memory management, but for now I think it makes
> sense to backport all three to the stable branches.
>
> So, in summary:
> 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full buf_alloc")
Again, this last one fails to apply everywhere :(
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: Peter Oberparleiter @ 2026-06-16 7:58 UTC (permalink / raw)
To: Halil Pasic, William Bezenah, vneethv
Cc: Cornelia Huck, linux-s390, farman, hca, gor, agordeev,
borntraeger, svens, mjrosato, virtualization, kvm, linux-kernel
In-Reply-To: <20260615234246.0ec5d947.pasic@linux.ibm.com>
On 15.06.2026 23:42, Halil Pasic wrote:
> On Mon, 15 Jun 2026 16:01:55 -0400
> William Bezenah <wbezenah@linux.ibm.com> wrote:
>
>> On 6/15/2026 10:58 AM, Cornelia Huck wrote:
>>> On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
>>>
>>>> On Fri, 12 Jun 2026 17:54:07 +0200
>>>> William Bezenah <wbezenah@linux.ibm.com> wrote:
>>>>
>>>>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
>>>>> subchannel based on DNV"), subchannel behavior following a device
>>>>> detach has been updated and results in -EINVAL being propagated
>>>>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
>>>>> in cio/device_ops. In the end, the virtio driver has no ability to
>>>>> react to the difference between device and subchannel states here,
>>>>> and during detach, both -ENODEV and -EINVAL indicate the device
>>>>> cannot be used and should not be treated as errors requiring
>>>>> attention. Update error handling in virtio_ccw_del_vq() and
>>>>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
>>>>> -ENODEV.
>>>> Hi William!
>>>>
>>>> Are you saying that ccw_device_start() started returning -EINVAL
>>>> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
>>>> DNV")? Or did I somehow read the paragraph wrong?
>>>>
>>>> The funcition ccw_device_start is documented to return:
>>>> * Returns:
>>>> * %0, if the operation was successful;
>>>> * -%EBUSY, if the device is busy, or status pending;
>>>> * -%EACCES, if no path specified in @lpm is operational;
>>>> * -%ENODEV, if the device is not operational.
>>>> and the commit message does not say a thing about introducing -EINVAL to
>>>> the mix.
>>> The function may return -EINVAL for non-enabled subchannels
>>> (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
>>> I'd expect it not to be enabled in that case anyway.
>>
>> Yep, that's at least how I've come to understand what changed. The
>> function ccw_device_start_timeout_key() has always returned -EINVAL
>> for non-enabled subchannels (pmcw.ena == 0), though it's not
>> documented in the header.
>
> Wasn't his -EINVAL actually introduced by commit:
> 823d494ac111 ("[S390] pm: ccw bus power management callbacks")?
In the context of virtio-ccw added in 2012, an EINVAL return code
introduced in 2009 might be considered "always" :)
>> What changed with commit 8c58a229688c is that cio_update_schib() now
>> updates the schib even when DNV=0, rather than returning early as it
>> did previously. Somehow this update results in pmcw.ena == 0 in
>> ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
>> moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
>> where it returned -ENODEV.
>
> Sounds fishy to me. As far as I understand the DNV takes precedence over
> all other pieces of PMCW.
And you're right about that! The Principles of Operation states (p. 15-4
in SA22-7832-14 [1]) that the contents of all other fields in the PMCW
are unpredictable when DNV is 0, therefore 8c58a229688c is in error.
I'll work with Vineeth to determine how to fix this issue, potentially
via manually clearing some relevant SCHIB fields instead of copying the
unpredictable results of the STSCH instruction.
>> So the commit didn't introduce -EINVAL as a new return value, rather,
>> it changed the subchannel lifecycle such that existing paths now
>> propagate -EINVAL rather than -ENODEV during the device detach
>> scenario.
>
> I'm not convinced returning -EINVAL in the given situation is the
> right thing to do. Peter, would you mind to chime in?
I tend to agree that an attempt to start I/O for a subchannel that has
DNV 0 should result in ENODEV rather than EINVAL, though the latter is
still valid when a driver tries to start I/O on a subchannel that is not
enabled for I/O.
We'll make sure to design the fix for 8c58a229688c in away that ENODEV
will be returned when DNV is 0. Assuming that this is the only situation
where virtio-ccw's ccw_io_helper() receives -EINVAL from
ccw_device_start__timeout_key() during detach, the subject patch should
no longer be necessary.
[1] https://www.ibm.com/docs/en/module_1678991624569/pdf/SA22-7832-14.pdf
--
Peter Oberparleiter
Linux on IBM Z Development - IBM Germany R&D
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Stefano Garzarella @ 2026-06-16 7:52 UTC (permalink / raw)
To: Greg KH
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <2026061624-harbor-capture-a5bf@gregkh>
On Tue, Jun 16, 2026 at 10:17:31AM +0530, Greg KH wrote:
>On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
>> On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
>> > > > What's the status of that fix?
>> > >
>> > > Stefano posted v3 and is working on v4.
>> > >
>> > > > Should it be reverted elsewhere?
>> > >
>> > > Donnu. With the change we have no DoS but the socket gets silently
>> > > broken. Eric felt given the brokenness is upstream already it's better
>> > > to work on a fix on top, not revert.
>> >
>> > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
>> > follow-up once it lands upstream.
>>
>> FYI v4 is now merged in the net tree, so I guess they will land upstream
>> soon. I CCed stable on both patches:
>>
>> a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
>> c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
>> buf_alloc")
>>
>> Both are related, but the second is the main fix of this patch.
>
>THe second one doesn't apply at all :(
>
The second one is the fix of the patch originally added to stable queue
by this thread, so should be applied on top of it (commit 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue")).
I'm working on improving memory management, but for now I think it makes
sense to backport all three to the stable branches.
So, in summary:
059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full buf_alloc")
Thanks,
Stefano
^ permalink raw reply
* Re:Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Xuan Zhuo @ 2026-06-16 6:49 UTC (permalink / raw)
To: Lange Tang
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com,
mst@redhat.com
In-Reply-To: <4118686.4d22.19ecf0ad78e.Coremail.lange_tang@163.com>
On Tue, 16 Jun 2026 14:07:34 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> At 2026-06-16 11:27:12, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> >> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> >> >> From: Longjun Tang <tanglongjun@kylinos.cn>
> >> >>
> >> >> When busy-poll is active, napi_schedule_prep() returns false in
> >> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> >> >> may keep firing irqs until the next poll round reaches
> >> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> >> >> it will lead to a large number of spurious interrupts. Explicitly
> >> >> disable callbacks in this case to prevent spurious interrupts.
> >> >>
> >> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> >> >> ---
> >> >> drivers/net/virtio_net.c | 2 ++
> >> >> 1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> >> index f4adcfee7a80..6d675fddc59b 100644
> >> >> --- a/drivers/net/virtio_net.c
> >> >> +++ b/drivers/net/virtio_net.c
> >> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> >> if (napi_schedule_prep(napi)) {
> >> >> virtqueue_disable_cb(vq);
> >> >> __napi_schedule(napi);
> >> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> >> >> + virtqueue_disable_cb(vq);
> >> >
> >> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
> >> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
> >> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
> >>
> >> How about rewrite it like this?
> >> static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> struct virtqueue *vq)
> >> {
> >> virtqueue_disable_cb(vq);
> >> if (napi_schedule_prep(napi))
> >> __napi_schedule(napi);
> >> }
> >> Any comments are welcome.
> >
> >
> >Another CPU could be running NAPI and has just enabled the callbacks (cb).
> >Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
> >CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
> >re-enable the cb.
> >
> >Thanks.
>
> Regarding the case you described, when NAPI on another CPU exits, the virtqueue_napi_complete func
> will be executed to re-enable cb. and if there is still unconsumed data in the virtqueue, virtqueue_napi_schedule
> will be called again to schedule NAPI.
>
> In summary, I think that the disable_cb and __napi_schedule within the virtqueue_napi_schedule func do not need to be bound together.
>
> Any comments are welcome. Thinks.
<Your code>
static void virtqueue_napi_schedule(struct napi_struct *napi,
struct virtqueue *vq)
{
|static bool virtqueue_napi_complete(struct napi_struct *napi,
| struct virtqueue *vq, int processed)
|{
| int opaque;
|
| opaque = virtqueue_enable_cb_prepare(vq);
|
virtqueue_disable_cb(vq); |
if (napi_schedule_prep(napi)) |
__napi_schedule(napi); |
| if (napi_complete_done(napi, processed)) {
| if (unlikely(virtqueue_poll(vq, opaque)))
| virtqueue_napi_schedule(napi, vq);
| else
| return true; // return directly
| } else {
| virtqueue_disable_cb(vq);
| }
|
| return false;
|}
}
1. new packets (notified by irq) are consumed by napi before virtqueue_napi_complete
2. poll is not called by irq, maybe xsk wake up. So irq is not disabled.
Thanks.
>
> >
> >
> >> >
> >> >
> >> >> }
> >> >> }
> >> >>
> >> >> --
> >> >> 2.25.1
> >> >>
> >>
>
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: David Hildenbrand (Arm) @ 2026-06-16 6:56 UTC (permalink / raw)
To: Miaohe Lin, Michael S. Tsirkin
Cc: Zi Yan, Andrew Morton, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <d7e97bec-72bf-7d56-3811-6d9c41fefb35@huawei.com>
>>
>>
>> Assume that we enlighten all non-atomics to grab the rcu read lock, such as
>
> These non-atomics are defined and used because they want to avoid atomic ops overhead?
> So I'm afraid using rcu read lock in these places would lead to unexpected overhead.
It should be cheaper than atomics IIUC. Further, I assume that some pages could
batch over multiple such operations (esp. page freeing path when we process tail
pages).
With !CONFIG_PREEMPT_RCU it's simply preempt_disable()/preempt_enable(), which
is either a NOP or just adjusting the preempt counter of the current thread. Cheap.
With CONFIG_PREEMPT_RCU we mostly increment current->rcu_read_lock_nesting. But
there might be a function call involved (did not look into the details). So that
variant should be slightly more expensive.
We'd have to measure what an addition rcu read lock would cost in there. that
should be fairly easy to benchmark.
>>
>> Maybe that would work. There would still be issues to solve
>>
>> (a) We don't hold the mf_mutex on all call paths, but we really need it so a
>> page_test_set_hwpoison() cannot race in weird ways with the other primitives I think.
>>
>> (b) There are some leftover SetPageHWPoison etc. instances. The ones in
>> arch/x86/kernel/cpu/mce/core.c likely cannot grab the mutex, but maybe they are
>> corner cases either way and we can document the situation.
>>
>>
>> Further, while I assume the synchronize_rcu() on the MCE path should be fine
>> (who cares about performance there?), I don't know if the added RCU read lock
>> on some paths could be noticable.
>>
>> So one idea worth discussing, but I am sure there are more problems.
>
> I think this is a good idea, although there are some remaining issues.
> But such race should be really rare, is it worth all this effort? Could we
> simply aim to resolve, not to be flawless? I.e. could we simply check
> and re-set the hwpoison flag at the end of memory_failure handling to
> simply avoid losing hwpoison flag as a best-effort attempt? Would it be
> acceptable?
Hacky. Sufficient for the hypervisor to suspend the nonatomic-setting CPU at the
wrong time to still trigger the same behavior.
I think, either we fix it properly, or we redesign hwpoison handling to deal
with setting/clearing becoming stale at some random point in the future.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Miaohe Lin @ 2026-06-16 6:32 UTC (permalink / raw)
To: David Hildenbrand (Arm), Michael S. Tsirkin
Cc: Zi Yan, Andrew Morton, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <be1b20ed-5b75-46b8-b7be-3c9b029f016b@kernel.org>
On 2026/6/15 18:54, David Hildenbrand (Arm) wrote:
> On 6/15/26 05:29, Miaohe Lin wrote:
>> On 2026/6/11 21:20, David Hildenbrand (Arm) wrote:
>>> On 6/11/26 09:36, Miaohe Lin wrote:
>>>>
>>>> Agree, it's not worth to do so.
>>>>
>>>>
>>>> Since memory_failure might be the only place, this change would be unacceptable.
>>>> We should come up with a better solution. Maybe we can try repeating SetPageHWPoison
>>>> and ClearPageHWPoison at a first attempt though it looks somewhat weird to me and makes
>>>> code more complicated.
>>>
>>> And I am fairly sure we could still have some remaining races ... it's shaky.
>>
>> I have to agree it's shaky.
>
> Right, just let writing task reschedule after reading the flags,
> but before writing the flags.
>
>> Any suggestion for next step?
>
> We have various code that assumes that no concurrent writes are
> possible, and consequently, we use no atomics.
>
> __free_pages_prepare() is just one user.
>
> Then we have __folio_set_locked(), __folio_clear_active()
> and __folio_clear_unevictable().
>
> But also __folio_mark_uptodate(), which is called rather frequently.
>
> page_cpupid_reset_last() is also a thing, but it mostly falls
> under __free_pages_prepare() handling.
>
> ... and __split_folio_to_order() also messes with flags directly without atomics.
>
>
> Many of these are only possible for frozen pages (refcount == 0). I think
> only __folio_set_locked() and __folio_mark_uptodate() are called on
> non-frozen pages, when there is the expectation that nobody will concurrently
> use atomics that would be bad (e.g., don't trylock if not an lru page).
>
Thanks David! This information is really helpful!
>
> We don't want to use atomics at these places just to please memory failure code.
Bad news. We have more places racing with memory failure code.
>
> Would it be sufficient to know in memory-failure code that concurrent
> handling succeeded?
I think so, that would be useful.
>
>
> Assume that we enlighten all non-atomics to grab the rcu read lock, such as
These non-atomics are defined and used because they want to avoid atomic ops overhead?
So I'm afraid using rcu read lock in these places would lead to unexpected overhead.
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 7223f6f4e2b4..3c3852b60bbd 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -803,10 +803,30 @@ static inline bool PageUptodate(const struct page *page)
> return folio_test_uptodate(page_folio(page));
> }
>
> +#ifdef CONFIG_MEMORY_FAILURE
> +static inline void page_flags_modify_nonatomic_begin(void)
> +{
> + rcu_read_lock();
> +}
> +static inline void page_flags_modify_nonatomic_end(void)
> +{
> + rcu_read_unlock();
> +}
> +#else
> +static inline void page_flags_modify_nonatomic_begin(void)
> +{
> +}
> +static inline void page_flags_modify_nonatomic_end(void)
> +{
> +}
> +#endif
> +
> static __always_inline void __folio_mark_uptodate(struct folio *folio)
> {
> smp_wmb();
> + page_flags_modify_nonatomic_begin();
> __set_bit(PG_uptodate, folio_flags(folio, 0));
> + page_flags_modify_nonatomic_end();
> }
>
>
> And then we have some retry logic such as:
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 51508a55c405..1123c40aaf43 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -162,6 +162,62 @@ static struct rb_root_cached pfn_space_itree = RB_ROOT_CACHED;
>
> static DEFINE_MUTEX(pfn_space_lock);
>
> +static bool page_test_set_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (true) {
> + /* Already set -> not our problem. */
> + if (TestSetPageHWPoison(page))
> + return true;
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + /* Setting the flag was sticky. */
> + if (PageHWPoison(page))
> + return false;
> + }
> +}
> +
> +static bool page_test_clear_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (true) {
> + /* Already clear -> not our problem. */
> + if (!TestClearPageHWPoison(page))
> + return false;
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + /* Clearing the flag was sticky. */
> + if (!PageHWPoison(page))
> + return true;
> + }
> +}
> +
> +static void page_set_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (!PageHWPoison(page)) {
> + SetPageHWPoison(page);
> +
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + }
> +}
> +
> +static void page_clear_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (PageHWPoison(page)) {
> + ClearPageHWPoison(page);
> +
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + }
> +}
> +
> /*
> * Return values:
> * 1: the page is dissolved (if needed) and taken off from buddy,
> @@ -199,7 +255,7 @@ static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, boo
> return false;
> }
>
> - SetPageHWPoison(page);
> + page_set_hwpoison(page);
> if (release)
> put_page(page);
> page_ref_inc(page);
> @@ -1744,7 +1800,7 @@ static int mf_generic_kill_procs(unsigned long long pfn, int flags,
> * Use this flag as an indication that the dax page has been
> * remapped UC to prevent speculative consumption of poison.
> */
> - SetPageHWPoison(&folio->page);
> + page_set_hwpoison(&folio->page);
>
> /*
> * Unlike System-RAM there is no possibility to swap in a
> @@ -1789,7 +1845,7 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
> goto unlock;
>
> if (!pre_remove)
> - SetPageHWPoison(page);
> + page_set_hwpoison(page);
>
> /*
> * The pre_remove case is revoking access, the memory is still
> @@ -1866,7 +1922,7 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
> head = llist_del_all(raw_hwp_list_head(folio));
> llist_for_each_entry_safe(p, next, head, node) {
> if (move_flag)
> - SetPageHWPoison(p->page);
> + page_set_hwpoison(p->page);
> else
> num_poisoned_pages_sub(page_to_pfn(p->page), 1);
> kfree(p);
> @@ -2380,7 +2436,7 @@ int memory_failure(unsigned long pfn, int flags)
> if (res != -ENOENT)
> goto unlock_mutex;
>
> - if (TestSetPageHWPoison(p)) {
> + if (page_test_set_hwpoison(p)) {
> res = -EHWPOISON;
> if (flags & MF_ACTION_REQUIRED)
> res = kill_accessing_process(current, pfn, flags);
> @@ -2410,7 +2466,7 @@ int memory_failure(unsigned long pfn, int flags)
> } else {
> /* We lost the race, try again */
> if (retry) {
> - ClearPageHWPoison(p);
> + page_clear_hwpoison(p);
> retry = false;
> goto try_again;
> }
> @@ -2431,7 +2487,7 @@ int memory_failure(unsigned long pfn, int flags)
> /* filter pages that are protected from hwpoison test by users */
> folio_lock(folio);
> if (hwpoison_filter(p)) {
> - ClearPageHWPoison(p);
> + page_clear_hwpoison(p);
> folio_unlock(folio);
> folio_put(folio);
> res = -EOPNOTSUPP;
> @@ -2751,7 +2807,7 @@ int unpoison_memory(unsigned long pfn)
> }
>
> folio_put(folio);
> - if (TestClearPageHWPoison(p)) {
> + if (page_test_clear_hwpoison(p)) {
> folio_put(folio);
> ret = 0;
> }
>
>
> Maybe that would work. There would still be issues to solve
>
> (a) We don't hold the mf_mutex on all call paths, but we really need it so a
> page_test_set_hwpoison() cannot race in weird ways with the other primitives I think.
>
> (b) There are some leftover SetPageHWPoison etc. instances. The ones in
> arch/x86/kernel/cpu/mce/core.c likely cannot grab the mutex, but maybe they are
> corner cases either way and we can document the situation.
>
>
> Further, while I assume the synchronize_rcu() on the MCE path should be fine
> (who cares about performance there?), I don't know if the added RCU read lock
> on some paths could be noticable.
>
> So one idea worth discussing, but I am sure there are more problems.
I think this is a good idea, although there are some remaining issues.
But such race should be really rare, is it worth all this effort? Could we
simply aim to resolve, not to be flawless? I.e. could we simply check
and re-set the hwpoison flag at the end of memory_failure handling to
simply avoid losing hwpoison flag as a best-effort attempt? Would it be
acceptable?
Thanks.
.
^ permalink raw reply
* Re:Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Lange Tang @ 2026-06-16 6:07 UTC (permalink / raw)
To: xuanzhuo@linux.alibaba.com, mst@redhat.com
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com
In-Reply-To: <1781580432.712892-1-xuanzhuo@linux.alibaba.com>
At 2026-06-16 11:27:12, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
>> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
>> >> From: Longjun Tang <tanglongjun@kylinos.cn>
>> >>
>> >> When busy-poll is active, napi_schedule_prep() returns false in
>> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
>> >> may keep firing irqs until the next poll round reaches
>> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
>> >> it will lead to a large number of spurious interrupts. Explicitly
>> >> disable callbacks in this case to prevent spurious interrupts.
>> >>
>> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
>> >> ---
>> >> drivers/net/virtio_net.c | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> >> index f4adcfee7a80..6d675fddc59b 100644
>> >> --- a/drivers/net/virtio_net.c
>> >> +++ b/drivers/net/virtio_net.c
>> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
>> >> if (napi_schedule_prep(napi)) {
>> >> virtqueue_disable_cb(vq);
>> >> __napi_schedule(napi);
>> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
>> >> + virtqueue_disable_cb(vq);
>> >
>> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
>> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
>> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
>>
>> How about rewrite it like this?
>> static void virtqueue_napi_schedule(struct napi_struct *napi,
>> struct virtqueue *vq)
>> {
>> virtqueue_disable_cb(vq);
>> if (napi_schedule_prep(napi))
>> __napi_schedule(napi);
>> }
>> Any comments are welcome.
>
>
>Another CPU could be running NAPI and has just enabled the callbacks (cb).
>Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
>CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
>re-enable the cb.
>
>Thanks.
Regarding the case you described, when NAPI on another CPU exits, the virtqueue_napi_complete func
will be executed to re-enable cb. and if there is still unconsumed data in the virtqueue, virtqueue_napi_schedule
will be called again to schedule NAPI.
In summary, I think that the disable_cb and __napi_schedule within the virtqueue_napi_schedule func do not need to be bound together.
Any comments are welcome. Thinks.
>
>
>> >
>> >
>> >> }
>> >> }
>> >>
>> >> --
>> >> 2.25.1
>> >>
>>
^ permalink raw reply
* Re: [PATCH net v4] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 5:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260616003903-mutt-send-email-mst@kernel.org>
On Mon, Jun 15, 2026 at 9:40 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 15, 2026 at 09:28:37PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> >
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> > ---
> > v4: use easy to understand math to compute the max_len
> > v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
>
> I still feel 2/2 is good defence in depth but it can be
> pursued separately.
Thanks, Michael. I'll leave 2/2 out of this series.
Appreciate the review.
Xiang
>
> > v2: add additiona check as 2/2
> >
> > drivers/net/virtio_net.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..8f4562316aaa 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> > + sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
> > --
> > 2.43.0
>
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Greg KH @ 2026-06-16 4:47 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <ag8EvTp29B-Q3nCq@sgarzare-redhat>
On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
> On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
> > > > What's the status of that fix?
> > >
> > > Stefano posted v3 and is working on v4.
> > >
> > > > Should it be reverted elsewhere?
> > >
> > > Donnu. With the change we have no DoS but the socket gets silently
> > > broken. Eric felt given the brokenness is upstream already it's better
> > > to work on a fix on top, not revert.
> >
> > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
> > follow-up once it lands upstream.
>
> FYI v4 is now merged in the net tree, so I guess they will land upstream
> soon. I CCed stable on both patches:
>
> a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
> buf_alloc")
>
> Both are related, but the second is the main fix of this patch.
THe second one doesn't apply at all :(
^ permalink raw reply
* Re: [PATCH net v4] virtio-net: fix len check in receive_big()
From: Michael S. Tsirkin @ 2026-06-16 4:39 UTC (permalink / raw)
To: Xiang Mei
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260616042837.2249468-1-xmei5@asu.edu>
On Mon, Jun 15, 2026 at 09:28:37PM -0700, Xiang Mei wrote:
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
>
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v4: use easy to understand math to compute the max_len
> v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
I still feel 2/2 is good defence in depth but it can be
pursued separately.
> v2: add additiona check as 2/2
>
> drivers/net/virtio_net.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..8f4562316aaa 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> + sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH net v3] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 4:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260614152904-mutt-send-email-mst@kernel.org>
On Sun, Jun 14, 2026 at 12:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Jun 13, 2026 at 01:15:02PM -0700, Xiang Mei wrote:
> > On Wed, Jun 10, 2026 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Jun 10, 2026 at 07:46:16PM -0700, Xiang Mei wrote:
> > > > receive_big() bounds the device-announced length by
> > > > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > > > add_recvbuf_big() sets sg[1] to start at offset
> > > > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > > > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > > > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > > > check allows for the common hdr_len == 12 case.
> > > >
> > > > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > > > then walks one frag past the page chain, storing a NULL page->private
> > > > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > > > write past the static frag array and a NULL frag handed up the rx path.
> > > >
> > > > Bound len by the size add_recvbuf_big() actually advertised.
> > > >
> > > > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > > > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > > > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > > > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > >
> > > Thanks for the patch! Something small to improve:
> > >
> > > > ---
> > > > v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
> > > >
> > > > drivers/net/virtio_net.c | 8 +++++---
> > > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index f4adcfee7a80..afe73eda1491 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > > > struct virtnet_rq_stats *stats)
> > > > {
> > > > struct page *page = buf;
> > > > + unsigned long max_len;
> > >
> > > Assignment can happen here?
> > >
> > > > struct sk_buff *skb;
> > > >
> > > > /* Make sure that len does not exceed the size allocated in
> > > > * add_recvbuf_big.
> > > > */
> > > > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > > > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > > > + vi->big_packets_num_skbfrags * PAGE_SIZE;
> > >
> > > Took me a while to figure out what is going on, but I finally
> > > understand:
> > >
> > >
> > > Reducing
> > > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE
> > >
> > > (what we allocated)
> > >
> > > by sizeof(struct padded_vnet_hdr) - vi->hdr_len
> > >
> > >
> > > right?
> > >
> > > So clearer as:
> > >
> > >
> > > unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> > > sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> > >
> > Right, that's the same value. Yours reads better!
> >
> > I'll fold this into the next respin. One thing I'd like to settle
> > first: David suggested storing this in a vi field computed once at the
> > probe (it's a per-device constant) and just comparing len against it
> > on the datapath, instead of re-deriving it in receive_big() each time.
> > I'll wait for his take on that and send a single v4 that covers both.
> >
> > Xiang
>
> I don't mind.
Thanks, Michael,
V4 has been sent.
Xiang
>
> > >
> > >
> > >
> > > > + if (unlikely(len > max_len)) {
> > > > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > > > - dev->name, len,
> > > > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > > > + dev->name, len, max_len);
> > > > goto err;
> > > > }
> > > >
> > > > --
> > > > 2.43.0
> > >
>
^ permalink raw reply
* [PATCH net v4] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 4:28 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei
receive_big() bounds the device-announced length by
(big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
add_recvbuf_big() sets sg[1] to start at offset
sizeof(struct padded_vnet_hdr) into the first page, so the chain
actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
check allows for the common hdr_len == 12 case.
A malicious virtio backend can announce a len in that gap. page_to_skb()
then walks one frag past the page chain, storing a NULL page->private
into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
write past the static frag array and a NULL frag handed up the rx path.
Bound len by the size add_recvbuf_big() actually advertised.
Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
v4: use easy to understand math to compute the max_len
v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
v2: add additiona check as 2/2
drivers/net/virtio_net.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f4adcfee7a80..8f4562316aaa 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
struct virtnet_rq_stats *stats)
{
struct page *page = buf;
+ unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
+ sizeof(struct padded_vnet_hdr) + vi->hdr_len;
struct sk_buff *skb;
/* Make sure that len does not exceed the size allocated in
* add_recvbuf_big.
*/
- if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
+ if (unlikely(len > max_len)) {
pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
- dev->name, len,
- (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
+ dev->name, len, max_len);
goto err;
}
--
2.43.0
^ permalink raw reply related
* Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Xuan Zhuo @ 2026-06-16 3:27 UTC (permalink / raw)
To: Lange Tang
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com,
mst@redhat.com
In-Reply-To: <19692a81.3001.19ece5f8ddc.Coremail.lange_tang@163.com>
On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> >> From: Longjun Tang <tanglongjun@kylinos.cn>
> >>
> >> When busy-poll is active, napi_schedule_prep() returns false in
> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> >> may keep firing irqs until the next poll round reaches
> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> >> it will lead to a large number of spurious interrupts. Explicitly
> >> disable callbacks in this case to prevent spurious interrupts.
> >>
> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> >> ---
> >> drivers/net/virtio_net.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index f4adcfee7a80..6d675fddc59b 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> if (napi_schedule_prep(napi)) {
> >> virtqueue_disable_cb(vq);
> >> __napi_schedule(napi);
> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> >> + virtqueue_disable_cb(vq);
> >
> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
>
> How about rewrite it like this?
> static void virtqueue_napi_schedule(struct napi_struct *napi,
> struct virtqueue *vq)
> {
> virtqueue_disable_cb(vq);
> if (napi_schedule_prep(napi))
> __napi_schedule(napi);
> }
> Any comments are welcome.
Another CPU could be running NAPI and has just enabled the callbacks (cb).
Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
re-enable the cb.
Thanks.
> >
> >Thanks.
> >
> >> }
> >> }
> >>
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply
* Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Lange Tang @ 2026-06-16 3:00 UTC (permalink / raw)
To: xuanzhuo@linux.alibaba.com, mst@redhat.com
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com
In-Reply-To: <1781517700.4206195-1-xuanzhuo@linux.alibaba.com>
At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
>> From: Longjun Tang <tanglongjun@kylinos.cn>
>>
>> When busy-poll is active, napi_schedule_prep() returns false in
>> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
>> may keep firing irqs until the next poll round reaches
>> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
>> it will lead to a large number of spurious interrupts. Explicitly
>> disable callbacks in this case to prevent spurious interrupts.
>>
>> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
>> ---
>> drivers/net/virtio_net.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index f4adcfee7a80..6d675fddc59b 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
>> if (napi_schedule_prep(napi)) {
>> virtqueue_disable_cb(vq);
>> __napi_schedule(napi);
>> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
>> + virtqueue_disable_cb(vq);
>
>I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
>drivers. The NIC driver should remain agnostic to busy polling. I think we need
>a better way, maybe we should rewrite virtqueue_napi_schedule instead.
How about rewrite it like this?
static void virtqueue_napi_schedule(struct napi_struct *napi,
struct virtqueue *vq)
{
virtqueue_disable_cb(vq);
if (napi_schedule_prep(napi))
__napi_schedule(napi);
}
Any comments are welcome.
>
>Thanks.
>
>> }
>> }
>>
>> --
>> 2.25.1
>>
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] virtio_net: xsk: fix race in rx wake up
From: Menglong Dong @ 2026-06-16 1:48 UTC (permalink / raw)
To: menglong8.dong, Xuan Zhuo
Cc: mst, jasowang, andrew+netdev, davem, edumazet, kuba, pabeni,
minhquangbui99, kerneljasonxing, netdev, virtualization,
linux-kernel, eperezma
In-Reply-To: <1781491685.0613394-1-xuanzhuo@linux.alibaba.com>
On 2026/6/15 10:48 Xuan Zhuo <xuanzhuo@linux.alibaba.com> write:
> On Thu, 11 Jun 2026 10:56:43 +0800, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <dongml2@chinatelecom.cn>
> >
> > During packet receiving in virtio-net, the rq can be empty, which means
> > "rq->vq->num_free == virtqueue_get_vring_size(rq->vq)", in
> > virtnet_add_recvbuf_xsk(), if we are using xsk. Meanwhile, the fill ring
> > can be empty too, which means we can't allocate anything from
> > xsk_buff_alloc_batch(). Then, we will set the XDP_RING_NEED_WAKEUP flag.
> >
[...]
> >
> > + need_wakeup = xsk_uses_need_wakeup(pool);
> > xsk_buffs = rq->xsk_buffs;
> >
> > + /* If both rq->vq and fill ring are empty, and then the user submit
> > + * all the chunks to the fill ring and check the wake up flag
> > + * after xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(),
> > + * we will lose the chance to wake up the rx napi, so we have to
> > + * set the need_wakeup flag here.
> > + */
> > + if (need_wakeup && virtqueue_get_vring_size(rq->vq) == rq->vq->num_free)
> > + xsk_set_rx_need_wakeup(pool);
>
> Is Condition A here too strict? We should trigger the wakeup under a wider range
> of scenarios.
Hi, Xuan. Thinks for your reviewing :)
The logic here is a addition logic to the origin wake up logic, which I planed
to fix a race condition. However, this race condition seems not likely to happen,
as we discussed in this thread:
https://lore.kernel.org/netdev/rHZz5_ylT4WggoZ-Ic2Q4w@linux.dev/
So this patch is not necessary, and I'll send the 2nd patch standalone.
Thanks!
Menglong Dong
>
> > +
> > num = xsk_buff_alloc_batch(pool, xsk_buffs, rq->vq->num_free);
> > if (!num) {
> > - if (xsk_uses_need_wakeup(pool)) {
> > + if (need_wakeup) {
> > xsk_set_rx_need_wakeup(pool);
> > /* Return 0 instead of -ENOMEM so that NAPI is
> > * descheduled.
> > @@ -1341,8 +1352,6 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> > }
> >
> > return -ENOMEM;
> > - } else {
> > - xsk_clear_rx_need_wakeup(pool);
> > }
> >
> > len = xsk_pool_get_rx_frame_size(pool) + vi->hdr_len;
> > @@ -1363,6 +1372,16 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> > goto err;
> > }
> >
> > + if (need_wakeup) {
> > + if (rq->vq->num_free)
> > + /* We have free buffers, so we'd better wake up the
> > + * rx napi as soon as possible.
> > + */
> > + xsk_set_rx_need_wakeup(pool);
>
> Is the purpose of waking up RX NAPI to invoke try_fill_recv? However,
> virtnet_poll does not call try_fill_recv directly. it is done
> conditionally.
>
> Thanks.
>
>
> > + else
> > + xsk_clear_rx_need_wakeup(pool);
> > + }
> > +
> > return num;
> >
> > err:
> > --
> > 2.54.0
> >
>
>
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: Halil Pasic @ 2026-06-15 21:42 UTC (permalink / raw)
To: William Bezenah
Cc: Cornelia Huck, linux-s390, farman, hca, gor, agordeev,
borntraeger, svens, mjrosato, vneethv, oberpar, virtualization,
kvm, linux-kernel, Halil Pasic
In-Reply-To: <4d7fc371-4357-496f-9774-1f7a7c1a3091@linux.ibm.com>
On Mon, 15 Jun 2026 16:01:55 -0400
William Bezenah <wbezenah@linux.ibm.com> wrote:
> On 6/15/2026 10:58 AM, Cornelia Huck wrote:
> > On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> >> On Fri, 12 Jun 2026 17:54:07 +0200
> >> William Bezenah <wbezenah@linux.ibm.com> wrote:
> >>
> >>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
> >>> subchannel based on DNV"), subchannel behavior following a device
> >>> detach has been updated and results in -EINVAL being propagated
> >>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
> >>> in cio/device_ops. In the end, the virtio driver has no ability to
> >>> react to the difference between device and subchannel states here,
> >>> and during detach, both -ENODEV and -EINVAL indicate the device
> >>> cannot be used and should not be treated as errors requiring
> >>> attention. Update error handling in virtio_ccw_del_vq() and
> >>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
> >>> -ENODEV.
> >> Hi William!
> >>
> >> Are you saying that ccw_device_start() started returning -EINVAL
> >> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
> >> DNV")? Or did I somehow read the paragraph wrong?
> >>
> >> The funcition ccw_device_start is documented to return:
> >> * Returns:
> >> * %0, if the operation was successful;
> >> * -%EBUSY, if the device is busy, or status pending;
> >> * -%EACCES, if no path specified in @lpm is operational;
> >> * -%ENODEV, if the device is not operational.
> >> and the commit message does not say a thing about introducing -EINVAL to
> >> the mix.
> > The function may return -EINVAL for non-enabled subchannels
> > (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
> > I'd expect it not to be enabled in that case anyway.
>
> Yep, that's at least how I've come to understand what changed. The
> function ccw_device_start_timeout_key() has always returned -EINVAL
> for non-enabled subchannels (pmcw.ena == 0), though it's not
> documented in the header.
Wasn't his -EINVAL actually introduced by commit:
823d494ac111 ("[S390] pm: ccw bus power management callbacks")?
>
> What changed with commit 8c58a229688c is that cio_update_schib() now
> updates the schib even when DNV=0, rather than returning early as it
> did previously. Somehow this update results in pmcw.ena == 0 in
> ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
> moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
> where it returned -ENODEV.
Sounds fishy to me. As far as I understand the DNV takes precedence over
all other pieces of PMCW.
>
> So the commit didn't introduce -EINVAL as a new return value, rather,
> it changed the subchannel lifecycle such that existing paths now
> propagate -EINVAL rather than -ENODEV during the device detach
> scenario.
>
I'm not convinced returning -EINVAL in the given situation is the
right thing to do. Peter, would you mind to chime in?
Regards,
Halil
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: William Bezenah @ 2026-06-15 20:01 UTC (permalink / raw)
To: Cornelia Huck, Halil Pasic
Cc: linux-s390, farman, hca, gor, agordeev, borntraeger, svens,
mjrosato, vneethv, oberpar, virtualization, kvm, linux-kernel
In-Reply-To: <875x3jn94r.fsf@redhat.com>
On 6/15/2026 10:58 AM, Cornelia Huck wrote:
> On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On Fri, 12 Jun 2026 17:54:07 +0200
>> William Bezenah <wbezenah@linux.ibm.com> wrote:
>>
>>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
>>> subchannel based on DNV"), subchannel behavior following a device
>>> detach has been updated and results in -EINVAL being propagated
>>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
>>> in cio/device_ops. In the end, the virtio driver has no ability to
>>> react to the difference between device and subchannel states here,
>>> and during detach, both -ENODEV and -EINVAL indicate the device
>>> cannot be used and should not be treated as errors requiring
>>> attention. Update error handling in virtio_ccw_del_vq() and
>>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
>>> -ENODEV.
>> Hi William!
>>
>> Are you saying that ccw_device_start() started returning -EINVAL
>> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
>> DNV")? Or did I somehow read the paragraph wrong?
>>
>> The funcition ccw_device_start is documented to return:
>> * Returns:
>> * %0, if the operation was successful;
>> * -%EBUSY, if the device is busy, or status pending;
>> * -%EACCES, if no path specified in @lpm is operational;
>> * -%ENODEV, if the device is not operational.
>> and the commit message does not say a thing about introducing -EINVAL to
>> the mix.
> The function may return -EINVAL for non-enabled subchannels
> (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
> I'd expect it not to be enabled in that case anyway.
Yep, that's at least how I've come to understand what changed. The
function ccw_device_start_timeout_key() has always returned -EINVAL
for non-enabled subchannels (pmcw.ena == 0), though it's not
documented in the header.
What changed with commit 8c58a229688c is that cio_update_schib() now
updates the schib even when DNV=0, rather than returning early as it
did previously. Somehow this update results in pmcw.ena == 0 in
ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
where it returned -ENODEV.
So the commit didn't introduce -EINVAL as a new return value, rather,
it changed the subchannel lifecycle such that existing paths now
propagate -EINVAL rather than -ENODEV during the device detach
scenario.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox