* Re: [PATCH resend v6 17/30] mm: page_alloc: propagate PageReported flag across buddy splits
From: Michael S. Tsirkin @ 2026-05-12 15:46 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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
In-Reply-To: <agNJ2BFRVTKINliT@gourry-fedora-PF4VCD3F>
On Tue, May 12, 2026 at 11:40:08AM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:03:19AM -0400, Michael S. Tsirkin wrote:
> > When a reported free page is split via expand() to satisfy a
> > smaller allocation, the sub-pages placed back on the free lists
> > lose the PageReported flag. This means they will be unnecessarily
> > re-reported to the hypervisor in the next reporting cycle, wasting
> > work.
> >
>
> Is this issue generated by the changes in the overall patch set, or an
> issue present regardless of the proposed zeroing avoidance feature?
>
> If the former, then maybe just submit as a standalone patch, seems like
> a useful improvement regardless.
The former but then the only issue is more reporting, and
I couldn't find a benchmark that would quantify the performance
difference.
So the value of pushing a fix by itself is pretty low.
> If fundamentally tied to the zeroing-avoidance feature, then nudging
> this to the front or end of the series (depending on conflicts) would
> help focus the review of the core new feature.
>
> ~Gregory
I can nudge it forward sure.
--
MST
^ permalink raw reply
* Re: [PATCH resend v6 17/30] mm: page_alloc: propagate PageReported flag across buddy splits
From: Gregory Price @ 2026-05-12 15:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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
In-Reply-To: <6d98a35c94fa44b550afa66923c620511e2826f0.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:03:19AM -0400, Michael S. Tsirkin wrote:
> When a reported free page is split via expand() to satisfy a
> smaller allocation, the sub-pages placed back on the free lists
> lose the PageReported flag. This means they will be unnecessarily
> re-reported to the hypervisor in the next reporting cycle, wasting
> work.
>
Is this issue generated by the changes in the overall patch set, or an
issue present regardless of the proposed zeroing avoidance feature?
If the former, then maybe just submit as a standalone patch, seems like
a useful improvement regardless.
If fundamentally tied to the zeroing-avoidance feature, then nudging
this to the front or end of the series (depending on conflicts) would
help focus the review of the core new feature.
~Gregory
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Stefano Garzarella @ 2026-05-12 15:39 UTC (permalink / raw)
To: Polina Vishneva
Cc: den@openvz.org, virtualization@lists.linux.dev,
stefanha@redhat.com, eperezma@redhat.com,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mst@redhat.com, kvm@vger.kernel.org, jasowang@redhat.com
In-Reply-To: <962b26d2d1daa9411fb71efab6af2c75d1c5f0d0.camel@virtuozzo.com>
On Tue, May 12, 2026 at 02:32:14PM +0000, Polina Vishneva wrote:
>On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
>> On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
>> > From: "Denis V. Lunev" <den@openvz.org>
>> >
>> > When the host initiates an AF_VSOCK connect() to a guest that has not
>> > yet loaded the virtio-vsock transport (i.e. still booting), the caller
>> > blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
>> > vhost_transport_do_send_pkt() silently exits when
>> > vhost_vq_get_backend(vq) returns NULL.
>>
>> Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
>
>It can, but it might be difficult to find a correct timeout.
>
>And, generally, there's no way to distinguish "the guest hasn't yet initialized
>the vq" from "the guest is up and running, but didn't reply to connect() in
>time". That's exactly what this patch is attempting to fix.
Okay, so please mention this in the commit message, I mean why
SO_VM_SOCKETS_CONNECT_TIMEOUT can't really help.
>
>>
>> >
>> > If the guest doesn't start listening within this timeout, connect()
>> > returns ETIMEDOUT.
>> >
>> > This delay is usually pointless and it doesn't well align with our
I still don't understand why this is pointless. If an application wants
to wait while sleeping, it can simply increase the timeout long enough
to wait for the VM to start up and use a single `connect()` call,
instead of continuing to try and wasting CPU cycles unnecessarily.
Hmm, or maybe not, because the driver will definitely be initialized
before the application that wants to listen on that port, so it will
respond that no one is listening, and the `connect()` call will fail
with an `ECONNRESET` error in any case. Right?
If it is the case, is the following line in the commit description
correct?
If the guest doesn't start listening within this timeout, connect()
returns ETIMEDOUT.
I mean, also if the application starts to listen within the timeout, I
think the connect() will fail in any case as I pointed out above (this
should be another point in favour of this change)
BTW, I think we should explain this more clearly both here and briefly
in the code as well.
>> > behavior at other initialization stages: for example, if a connection is
>> > attempted when the guest driver is already loaded, but when nothing is
>> > listening yet, it returns ECONNRESET immediately without any wait.
>> >
>> > Fix this by checking the RX virtqueue backend in
>> > vhost_transport_send_pkt() before queuing. If the backend is NULL,
>> > return -ECONNREFUSED immediately.
>> >
>> > Signed-off-by: Denis V. Lunev <den@openvz.org>
>> > Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>> > Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>> > ---
>> > drivers/vhost/vsock.c | 10 ++++++++++
>> > 1 file changed, 10 insertions(+)
>> >
>> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>> > index 1d8ec6bed53e..a3f218292c3a 100644
>> > --- a/drivers/vhost/vsock.c
>> > +++ b/drivers/vhost/vsock.c
>> > @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
>> > return -ENODEV;
>> > }
>> >
>> > + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
>> > + * private_data without vq->mutex is deliberate: even if the backend becomes
>> > + * NULL right after that check, do_send_pkt() checks it under the mutex.
>> > + */
>> > + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
>>
>> Why not using vhost_vq_get_backend() ?
>
>Because it locks the mutex, which is slow and unacceptable in this hot
>path.
ehm, sorry, which mutex are you talking about?
I see just a comment about the mutex to be acquired by the caller, but I
don't see any lock there.
>
>>
>> Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
>
>It's racy, but as described here in the comment and in the commit message,
>any possible race outcome is covered by the subsequent checks.
Okay, so what is the point to call READ_ONCE()?
>
>> > {
>> > + rcu_read_unlock();
>> > + kfree_skb(skb);
>> > + return -ECONNREFUSED;
>>
>> This is a generic send_pkt, is it okay to return ECONNREFUSED in any
>> case?
>
>EHOSTUNREACH would probably be better.
>All the current send_pkt functions only return ENODEV, but it has different
>semantics: they mean that the local device isn't yet ready, while there we're
>dealing with the opposite end not being ready.
In the AF_VSOCK prespective, I see ENODEV like the transport is not
ready, so I think it can eventually fit here too, but also EHOSTUNREACH
is fine, for sure better than ECONNREFUSED.
Thanks,
Stefano
>
>Best regards, Polina.
>
>>
>> Thanks,
>> Stefano
>>
>> > + }
>> > +
>> > if (virtio_vsock_skb_reply(skb))
>> > atomic_inc(&vsock->queued_replies);
>> >
>> >
>> > base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
>> > --
>> > 2.53.0
>> >
^ permalink raw reply
* Re: [RFC PATCH v3 3/7] hq-spinlock: add contention detection
From: WEI-HONG, YE @ 2026-05-12 14:49 UTC (permalink / raw)
To: fedorov.nikita
Cc: linux-kernel, linux-arm-kernel, linux-arch, linux-mm,
virtualization
In-Reply-To: <20260415164459.2904963-4-fedorov.nikita@h-partners.com>
Hi,
I may be missing something, but this patch adds
`update_counters_qspinlock()` for remote handoff accounting.
I tried commenting it out locally, and the kernel still built successfully
with `CONFIG_HQSPINLOCKS=y` and `CONFIG_HQSPINLOCKS_DEBUG=y`. I also could
not find a call site for it in the series, so it looks like this helper may
currently be unreferenced.
Is it still needed, or was the call site possibly missed in this RFC version?
Cheers,
Hong
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Polina Vishneva @ 2026-05-12 14:32 UTC (permalink / raw)
To: sgarzare@redhat.com
Cc: den@openvz.org, virtualization@lists.linux.dev,
stefanha@redhat.com, eperezma@redhat.com,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mst@redhat.com, kvm@vger.kernel.org, jasowang@redhat.com
In-Reply-To: <agH3WvJtAsQ_5zx4@sgarzare-redhat>
On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
> On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
> > From: "Denis V. Lunev" <den@openvz.org>
> >
> > When the host initiates an AF_VSOCK connect() to a guest that has not
> > yet loaded the virtio-vsock transport (i.e. still booting), the caller
> > blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
> > vhost_transport_do_send_pkt() silently exits when
> > vhost_vq_get_backend(vq) returns NULL.
>
> Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
It can, but it might be difficult to find a correct timeout.
And, generally, there's no way to distinguish "the guest hasn't yet initialized
the vq" from "the guest is up and running, but didn't reply to connect() in
time". That's exactly what this patch is attempting to fix.
>
> >
> > If the guest doesn't start listening within this timeout, connect()
> > returns ETIMEDOUT.
> >
> > This delay is usually pointless and it doesn't well align with our
> > behavior at other initialization stages: for example, if a connection is
> > attempted when the guest driver is already loaded, but when nothing is
> > listening yet, it returns ECONNRESET immediately without any wait.
> >
> > Fix this by checking the RX virtqueue backend in
> > vhost_transport_send_pkt() before queuing. If the backend is NULL,
> > return -ECONNREFUSED immediately.
> >
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > ---
> > drivers/vhost/vsock.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index 1d8ec6bed53e..a3f218292c3a 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> > return -ENODEV;
> > }
> >
> > + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
> > + * private_data without vq->mutex is deliberate: even if the backend becomes
> > + * NULL right after that check, do_send_pkt() checks it under the mutex.
> > + */
> > + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
>
> Why not using vhost_vq_get_backend() ?
Because it locks the mutex, which is slow and unacceptable in this hot path.
>
> Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
It's racy, but as described here in the comment and in the commit message,
any possible race outcome is covered by the subsequent checks.
> > {
> > + rcu_read_unlock();
> > + kfree_skb(skb);
> > + return -ECONNREFUSED;
>
> This is a generic send_pkt, is it okay to return ECONNREFUSED in any
> case?
EHOSTUNREACH would probably be better.
All the current send_pkt functions only return ENODEV, but it has different
semantics: they mean that the local device isn't yet ready, while there we're
dealing with the opposite end not being ready.
Best regards, Polina.
>
> Thanks,
> Stefano
>
> > + }
> > +
> > if (virtio_vsock_skb_reply(skb))
> > atomic_inc(&vsock->queued_replies);
> >
> >
> > base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
> > --
> > 2.53.0
> >
^ permalink raw reply
* Re: [PATCH net v2] vsock/vmci: fix UAF when peer resets connection during handshake
From: Bryan Tan @ 2026-05-12 13:12 UTC (permalink / raw)
To: Minh Nguyen
Cc: Vishnu Dasa, Stefano Garzarella, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman,
bcm-kernel-feedback-list, netdev, virtualization, linux-kernel,
stable
In-Reply-To: <20260512025851.189140-1-minhnguyen.080505@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5389 bytes --]
On Tue, May 12, 2026 at 3:59 AM Minh Nguyen <minhnguyen.080505@gmail.com> wrote:
>
> vmci_transport_recv_connecting_server() jumps to its destroy: label
> and performs an unconditional sock_put(pending) to release the
> explicit sock_hold() taken by vmci_transport_recv_listen() before
> schedule_delayed_work(). The existing comment claimed this was safe
> because the listen handler removes pending from the pending list on
> the way out, which would prevent vsock_pending_work() from dropping
> the same reference later.
>
> That assumption breaks for a peer RST. The default arm of the packet
> switch sets:
>
> err = pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST ? 0 : -EINVAL;
>
> and vmci_transport_recv_listen() only calls vsock_remove_pending()
> when err < 0:
>
> if (err < 0)
> vsock_remove_pending(sk, pending);
>
> For RST (err == 0) the socket stays on the pending list, so when
> vsock_pending_work() fires it takes the is_pending=true path and
> drops all three references itself: the pending-list reference via
> vsock_remove_pending(), then the two trailing sock_put(sk) calls.
> The unconditional sock_put() in destroy: had already dropped the
> explicit sock_hold() reference, so the second trailing sock_put(sk)
> in vsock_pending_work() is a write into the freed AF_VSOCK slab
> object. KASAN reports a slab-use-after-free write of 4 bytes from
> refcount_warn_saturate() on the workqueue path:
>
> BUG: KASAN: slab-use-after-free in refcount_warn_saturate
> Write of size 4 at addr ffff88800b1cac80 by task kworker
> Workqueue: events vsock_pending_work
> Call Trace:
> refcount_warn_saturate
> vsock_pending_work
> process_one_work
> worker_thread
>
> Triggering the bug requires only the ability to open a VSOCK
> connection to the target and send a RST before the listener accepts.
>
> Skip the sock_put() in destroy: when err == 0 so it only compensates
> the cases where vmci_transport_recv_listen() actually calls
> vsock_remove_pending(). RST is the only path that reaches destroy:
> with err == 0; every other path produces a negative value, so their
> behaviour is unchanged.
>
> Verified on lts-6.12.79 with KASAN enabled (CONFIG_KASAN_INLINE=y,
> kasan_multi_shot): same trigger binary, same VM, 100 iterations:
> without this patch 52 KASAN slab-use-after-free reports fire; with
> this patch applied, 0 reports.
>
> Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
> Cc: stable@vger.kernel.org
> Signed-off-by: Minh Nguyen <minhnguyen.080505@gmail.com>
> Assisted-by: Claude:claude-opus-4-7
> ---
> v2:
> - Resubmit to netdev per Stefano Garzarella's request after v1 review.
> - Retested the PoC with the patch applied on lts-6.12.79 with KASAN
> enabled: 52/100 unpatched -> 0/100 patched (same trigger binary,
> same VM, 100 iterations); test summary captured in the commit
> message.
> - Changed Cc: stable@kernel.org -> stable@vger.kernel.org now that the
> bug is no longer embargoed.
> - Rebased onto net/main (no functional change to the diff).
>
> v1 was sent to security@kernel.org on 2026-05-10 (not on lore archives;
> no public link available). v1 review summary, for reference:
> - Stefano Garzarella (vsock maintainer): "Overall LGTM, but I'd wait
> vmware guys on this that know this code better." Asked for retest
> and resubmission via the net tree workflow.
> - Bryan Tan (VMCI maintainer): "Thanks for the fix, it looks good to
> me." Also noted that no modern VMware product allows guest-to-guest
> VMCI communication, so the practical attack surface is host -> guest.
>
> net/vmw_vsock/vmci_transport.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
Acked-by: Bryan Tan <bryan-bt.tan@broadcom.com>
>
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 4296ca1..88d7128 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1269,14 +1269,16 @@ vmci_transport_recv_connecting_server(struct sock *listener,
> destroy:
> pending->sk_err = skerr;
> pending->sk_state = TCP_CLOSE;
> - /* As long as we drop our reference, all necessary cleanup will handle
> - * when the cleanup function drops its reference and our destruct
> - * implementation is called. Note that since the listen handler will
> - * remove pending from the pending list upon our failure, the cleanup
> - * function won't drop the additional reference, which is why we do it
> - * here.
> + /* Drop the reference taken by vmci_transport_recv_listen() before
> + * schedule_delayed_work() only on real errors. For a peer RST
> + * (err == 0) the listener leaves pending on the pending list, and
> + * vsock_pending_work() will drop that reference itself when it
> + * later cleans the socket up. Calling sock_put() here in that
> + * case would be a double-put and free the socket while
> + * vsock_pending_work() still holds it.
> */
> - sock_put(pending);
> + if (err < 0)
> + sock_put(pending);
>
> return err;
> }
>
> base-commit: be48e5fe51a5864566307998286a699d6b986934
> --
> 2.54.0
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5417 bytes --]
^ permalink raw reply
* Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Michael S. Tsirkin @ 2026-05-12 11:17 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Paolo Abeni, Xuan Zhuo, Eugenio Pérez, Eric Dumazet,
David S. Miller, kvm, Jason Wang, virtualization, linux-kernel,
Simon Horman, Jakub Kicinski, Stefan Hajnoczi
In-Reply-To: <agL0ewq9R-Br9mUh@sgarzare-redhat>
On Tue, May 12, 2026 at 12:03:38PM +0200, Stefano Garzarella wrote:
> On Tue, May 12, 2026 at 04:54:34AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > >
> > > After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> > > queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> > > buf_alloc when checking whether a new packet fits. This reduces the
> > > effective receive buffer below what the user configured via
> > > SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> > > silently dropped and applications that rely on the full buffer size
> > > to deadlock.
> > >
> > > Also, the reduced space is not communicated to the remote peer, so
> > > its credit calculation accounts more credit than the receiver will
> > > actually accept, causing data loss (there is no retransmission).
> > >
> > > With this approach we currently have failures in
> > > tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
> > > test 22 always fails in this way:
> > > 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
> > >
> > > 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
> > > Resource temporarily unavailable
> > >
> > > Fix this by using `buf_alloc * 2` as the total budget for payload plus
> > > skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
> > > is doubled to reserve space for sk_buff metadata. This preserves the
> > > full buf_alloc for payload under normal operation, while still bounding
> > > the skb queue growth.
> > >
> > > When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
> > > flooding where overhead dominates), the connection is reset and local
> > > socket error set to ENOBUFS, so both peers are explicitly notified of
> > > the failure rather than silently losing data.
> > >
> > > With this patch, all tests in tools/testing/vsock/vsock_test.c are
> > > now passing again.
> > >
> > > A solution to handle small-packet overhead efficiently also for
> > > SEQPACKET (we already do that for STREAM) is planned as follow-up work.
> > > This patch is needed in any case to prevent silent data loss, because
> > > even if we reduce the overhead, we can't eliminate it entirely.
> > >
> > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >
> > Thanks for the patch! I'd like to split this:
> > 1. buf alloc boost
> > 2. reset when out of credits
>
> Good point, also the reset maybe should have an other fixes tag (i.e. when
> we introduced that check)
>
> >
> > this way we can revert 2 easier later.
>
> I'm not sure if we should revert them at some point, even though we'll be
> able to handle the overhead better,
I mean we'll prevent the overflow, the condition will never be met.
> but I agree that we should split them.
>
> I'll wait for a few more comments and then send v3 with the split.
>
> Thanks,
> Stefano
^ permalink raw reply
* Re: [PATCH net 0/2] vsock/virtio: fix vsockmon tap skb construction
From: patchwork-bot+netdevbpf @ 2026-05-12 11:10 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, sunyiqixm, linux-kernel, xuanzhuo, mst, stefanha, kvm,
horms, bobbyeshleman, jasowang, kuba, davem, virtualization,
edumazet, pabeni, avkrasnov, eperezma, bobby.eshleman
In-Reply-To: <20260508164411.261440-1-sgarzare@redhat.com>
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 8 May 2026 18:44:09 +0200 you wrote:
> While reviewing the patch posted by Yiqi Sun [1] to fix an issue in
> virtio_transport_build_skb(), I discovered another issue related to
> the offset and length of the payload to be copied in the new skb.
> This was introduced when we did the skb conversion, and fixed by
> patch 1.
>
> Patch 2 fixes the issue found by Yiqi Sun in a different way: using
> iov_iter_kvec() to properly initialize all the iov_iter fields and
> removing the linear vs non-linear split like we alredy do in
> vhost-vsock.
>
> [...]
Here is the summary with links:
- [net,1/2] vsock/virtio: fix length and offset in tap skb for split packets
https://git.kernel.org/netdev/net/c/5f344d809e01
- [net,2/2] vsock/virtio: fix empty payload in tap skb for non-linear buffers
https://git.kernel.org/netdev/net/c/3a3e3d90cbc7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-12 10:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Paolo Abeni, Xuan Zhuo, Eugenio Pérez, Eric Dumazet,
David S. Miller, kvm, Jason Wang, virtualization, linux-kernel,
Simon Horman, Jakub Kicinski, Stefan Hajnoczi
In-Reply-To: <20260512044008-mutt-send-email-mst@kernel.org>
On Tue, May 12, 2026 at 04:54:34AM -0400, Michael S. Tsirkin wrote:
>On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
>> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
>> buf_alloc when checking whether a new packet fits. This reduces the
>> effective receive buffer below what the user configured via
>> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
>> silently dropped and applications that rely on the full buffer size
>> to deadlock.
>>
>> Also, the reduced space is not communicated to the remote peer, so
>> its credit calculation accounts more credit than the receiver will
>> actually accept, causing data loss (there is no retransmission).
>>
>> With this approach we currently have failures in
>> tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
>> test 22 always fails in this way:
>> 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>>
>> 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>> Resource temporarily unavailable
>>
>> Fix this by using `buf_alloc * 2` as the total budget for payload plus
>> skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
>> is doubled to reserve space for sk_buff metadata. This preserves the
>> full buf_alloc for payload under normal operation, while still bounding
>> the skb queue growth.
>>
>> When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
>> flooding where overhead dominates), the connection is reset and local
>> socket error set to ENOBUFS, so both peers are explicitly notified of
>> the failure rather than silently losing data.
>>
>> With this patch, all tests in tools/testing/vsock/vsock_test.c are
>> now passing again.
>>
>> A solution to handle small-packet overhead efficiently also for
>> SEQPACKET (we already do that for STREAM) is planned as follow-up work.
>> This patch is needed in any case to prevent silent data loss, because
>> even if we reduce the overhead, we can't eliminate it entirely.
>>
>> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Thanks for the patch! I'd like to split this:
>1. buf alloc boost
>2. reset when out of credits
Good point, also the reset maybe should have an other fixes tag (i.e.
when we introduced that check)
>
>this way we can revert 2 easier later.
I'm not sure if we should revert them at some point, even though we'll
be able to handle the overhead better, but I agree that we should split
them.
I'll wait for a few more comments and then send v3 with the split.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: syzbot @ 2026-05-12 8:59 UTC (permalink / raw)
To: ryasuoka
Cc: airlied, dmitry.osipenko, dri-devel, gurchetansingh, kraxel,
linux-kernel, maarten.lankhorst, mripard, olvaffe, ryasuoka,
simona, tzimmermann, virtualization
In-Reply-To: <20260512-virtio-gpu_wait_event-v1-1-207eb4c1a69a@redhat.com>
> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
> wait_event() without timeout when waiting for virtqueue space. If the
> host device stops processing commands, these waits block indefinitely.
> Since callers may hold DRM locks, this can make the entire system
> unresponsive.
>
> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
> consistent with the existing timeout pattern in the driver. On timeout,
> clean up and return -ENODEV, following the same error path as
> drm_dev_enter() failure.
>
> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 67865810a2e7..05e816a0ae0b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -396,7 +396,16 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
> if (vq->num_free < elemcnt) {
> spin_unlock(&vgdev->ctrlq.qlock);
> virtio_gpu_notify(vgdev);
> - wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
> + if (!wait_event_timeout(vgdev->ctrlq.ack_queue,
> + vq->num_free >= elemcnt,
> + 5 * HZ)) {
> + /* The device did not respond */
> + if (fence && vbuf->objs)
> + virtio_gpu_array_unlock_resv(vbuf->objs);
> + free_vbuf(vgdev, vbuf);
> + drm_dev_exit(idx);
> + return -ENODEV;
> + }
> goto again;
> }
>
> @@ -566,7 +575,14 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
> ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC);
> if (ret == -ENOSPC) {
> spin_unlock(&vgdev->cursorq.qlock);
> - wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt);
> + if (!wait_event_timeout(vgdev->cursorq.ack_queue,
> + vq->num_free >= outcnt,
> + 5 * HZ)) {
> + /* The device did not respond */
> + free_vbuf(vgdev, vbuf);
> + drm_dev_exit(idx);
> + return;
> + }
> spin_lock(&vgdev->cursorq.qlock);
> goto retry;
> } else {
>
> ---
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581
> change-id: 20260512-virtio-gpu_wait_event-e0cdf8675b7c
>
> Best regards,
> --
> Ryosuke Yasuoka <ryasuoka@redhat.com>
>
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
^ permalink raw reply
* [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: Ryosuke Yasuoka @ 2026-05-12 8:59 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter
Cc: dri-devel, virtualization, linux-kernel,
syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224,
syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e, Ryosuke Yasuoka
virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
wait_event() without timeout when waiting for virtqueue space. If the
host device stops processing commands, these waits block indefinitely.
Since callers may hold DRM locks, this can make the entire system
unresponsive.
Replace wait_event() with wait_event_timeout() using a 5-second timeout,
consistent with the existing timeout pattern in the driver. On timeout,
clean up and return -ENODEV, following the same error path as
drm_dev_enter() failure.
Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 67865810a2e7..05e816a0ae0b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -396,7 +396,16 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
if (vq->num_free < elemcnt) {
spin_unlock(&vgdev->ctrlq.qlock);
virtio_gpu_notify(vgdev);
- wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
+ if (!wait_event_timeout(vgdev->ctrlq.ack_queue,
+ vq->num_free >= elemcnt,
+ 5 * HZ)) {
+ /* The device did not respond */
+ if (fence && vbuf->objs)
+ virtio_gpu_array_unlock_resv(vbuf->objs);
+ free_vbuf(vgdev, vbuf);
+ drm_dev_exit(idx);
+ return -ENODEV;
+ }
goto again;
}
@@ -566,7 +575,14 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC);
if (ret == -ENOSPC) {
spin_unlock(&vgdev->cursorq.qlock);
- wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt);
+ if (!wait_event_timeout(vgdev->cursorq.ack_queue,
+ vq->num_free >= outcnt,
+ 5 * HZ)) {
+ /* The device did not respond */
+ free_vbuf(vgdev, vbuf);
+ drm_dev_exit(idx);
+ return;
+ }
spin_lock(&vgdev->cursorq.qlock);
goto retry;
} else {
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260512-virtio-gpu_wait_event-e0cdf8675b7c
Best regards,
--
Ryosuke Yasuoka <ryasuoka@redhat.com>
^ permalink raw reply related
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: syzbot @ 2026-05-12 8:59 UTC (permalink / raw)
To: ryasuoka
Cc: airlied, dmitry.osipenko, dri-devel, gurchetansingh, kraxel,
linux-kernel, maarten.lankhorst, mripard, olvaffe, ryasuoka,
simona, tzimmermann, virtualization
In-Reply-To: <20260512-virtio-gpu_wait_event-v1-1-207eb4c1a69a@redhat.com>
> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
> wait_event() without timeout when waiting for virtqueue space. If the
> host device stops processing commands, these waits block indefinitely.
> Since callers may hold DRM locks, this can make the entire system
> unresponsive.
>
> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
> consistent with the existing timeout pattern in the driver. On timeout,
> clean up and return -ENODEV, following the same error path as
> drm_dev_enter() failure.
>
> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 67865810a2e7..05e816a0ae0b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -396,7 +396,16 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
> if (vq->num_free < elemcnt) {
> spin_unlock(&vgdev->ctrlq.qlock);
> virtio_gpu_notify(vgdev);
> - wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
> + if (!wait_event_timeout(vgdev->ctrlq.ack_queue,
> + vq->num_free >= elemcnt,
> + 5 * HZ)) {
> + /* The device did not respond */
> + if (fence && vbuf->objs)
> + virtio_gpu_array_unlock_resv(vbuf->objs);
> + free_vbuf(vgdev, vbuf);
> + drm_dev_exit(idx);
> + return -ENODEV;
> + }
> goto again;
> }
>
> @@ -566,7 +575,14 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
> ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC);
> if (ret == -ENOSPC) {
> spin_unlock(&vgdev->cursorq.qlock);
> - wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt);
> + if (!wait_event_timeout(vgdev->cursorq.ack_queue,
> + vq->num_free >= outcnt,
> + 5 * HZ)) {
> + /* The device did not respond */
> + free_vbuf(vgdev, vbuf);
> + drm_dev_exit(idx);
> + return;
> + }
> spin_lock(&vgdev->cursorq.qlock);
> goto retry;
> } else {
>
> ---
> base-commit: 5d6919055dec134de3c40167a490f33c74c12581
> change-id: 20260512-virtio-gpu_wait_event-e0cdf8675b7c
>
> Best regards,
> --
> Ryosuke Yasuoka <ryasuoka@redhat.com>
>
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
^ permalink raw reply
* Re: [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Michael S. Tsirkin @ 2026-05-12 8:54 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Paolo Abeni, Xuan Zhuo, Eugenio Pérez, Eric Dumazet,
David S. Miller, kvm, Jason Wang, virtualization, linux-kernel,
Simon Horman, Jakub Kicinski, Stefan Hajnoczi
In-Reply-To: <20260512080737.36787-1-sgarzare@redhat.com>
On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> buf_alloc when checking whether a new packet fits. This reduces the
> effective receive buffer below what the user configured via
> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> silently dropped and applications that rely on the full buffer size
> to deadlock.
>
> Also, the reduced space is not communicated to the remote peer, so
> its credit calculation accounts more credit than the receiver will
> actually accept, causing data loss (there is no retransmission).
>
> With this approach we currently have failures in
> tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
> test 22 always fails in this way:
> 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>
> 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
> Resource temporarily unavailable
>
> Fix this by using `buf_alloc * 2` as the total budget for payload plus
> skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
> is doubled to reserve space for sk_buff metadata. This preserves the
> full buf_alloc for payload under normal operation, while still bounding
> the skb queue growth.
>
> When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
> flooding where overhead dominates), the connection is reset and local
> socket error set to ENOBUFS, so both peers are explicitly notified of
> the failure rather than silently losing data.
>
> With this patch, all tests in tools/testing/vsock/vsock_test.c are
> now passing again.
>
> A solution to handle small-packet overhead efficiently also for
> SEQPACKET (we already do that for STREAM) is planned as follow-up work.
> This patch is needed in any case to prevent silent data loss, because
> even if we reduce the overhead, we can't eliminate it entirely.
>
> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks for the patch! I'd like to split this:
1. buf alloc boost
2. reset when out of credits
this way we can revert 2 easier later.
> ---
> v2:
> - Close the connection when we can no longer queue new packets instead
> of losing data.
> - No longer announce the reduced buf_alloc to avoid violating the
> spec. [MST]
>
> v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
> ---
> net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 9b8014516f4f..f23bf8a11319 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -449,7 +449,10 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> {
> u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
>
> - if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
> + /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
> + * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> + */
> + if (skb_overhead + vvs->buf_used + len > (u64)vvs->buf_alloc * 2)
> return false;
>
> vvs->rx_bytes += len;
> @@ -1365,7 +1368,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> return err;
> }
>
> -static void
> +static bool
> virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> struct sk_buff *skb)
> {
> @@ -1380,10 +1383,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> spin_lock_bh(&vvs->rx_lock);
>
> can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> - if (!can_enqueue) {
> - free_pkt = true;
> + if (!can_enqueue)
> goto out;
> - }
>
> if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> vvs->msg_count++;
> @@ -1423,6 +1424,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> spin_unlock_bh(&vvs->rx_lock);
> if (free_pkt)
> kfree_skb(skb);
> +
> + return can_enqueue;
> }
>
> static int
> @@ -1435,7 +1438,16 @@ virtio_transport_recv_connected(struct sock *sk,
>
> switch (le16_to_cpu(hdr->op)) {
> case VIRTIO_VSOCK_OP_RW:
> - virtio_transport_recv_enqueue(vsk, skb);
> + if (!virtio_transport_recv_enqueue(vsk, skb)) {
> + /* There is no more space to queue the packet, so let's
> + * close the connection; otherwise, we'll lose data.
> + */
> + (void)virtio_transport_reset(vsk, skb);
> + sk->sk_state = TCP_CLOSE;
> + sk->sk_err = ENOBUFS;
> + sk_error_report(sk);
> + break;
> + }
> vsock_data_ready(sk);
> return err;
> case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> --
> 2.54.0
^ permalink raw reply
* [PATCH net v2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-12 8:07 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Paolo Abeni, Xuan Zhuo, Eugenio Pérez,
Eric Dumazet, David S. Miller, kvm, Stefano Garzarella,
Jason Wang, virtualization, linux-kernel, Simon Horman,
Jakub Kicinski, Stefan Hajnoczi
From: Stefano Garzarella <sgarzare@redhat.com>
After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
buf_alloc when checking whether a new packet fits. This reduces the
effective receive buffer below what the user configured via
SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
silently dropped and applications that rely on the full buffer size
to deadlock.
Also, the reduced space is not communicated to the remote peer, so
its credit calculation accounts more credit than the receiver will
actually accept, causing data loss (there is no retransmission).
With this approach we currently have failures in
tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
test 22 always fails in this way:
18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
Resource temporarily unavailable
Fix this by using `buf_alloc * 2` as the total budget for payload plus
skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
is doubled to reserve space for sk_buff metadata. This preserves the
full buf_alloc for payload under normal operation, while still bounding
the skb queue growth.
When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet
flooding where overhead dominates), the connection is reset and local
socket error set to ENOBUFS, so both peers are explicitly notified of
the failure rather than silently losing data.
With this patch, all tests in tools/testing/vsock/vsock_test.c are
now passing again.
A solution to handle small-packet overhead efficiently also for
SEQPACKET (we already do that for STREAM) is planned as follow-up work.
This patch is needed in any case to prevent silent data loss, because
even if we reduce the overhead, we can't eliminate it entirely.
Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- Close the connection when we can no longer queue new packets instead
of losing data.
- No longer announce the reduced buf_alloc to avoid violating the
spec. [MST]
v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
---
net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 9b8014516f4f..f23bf8a11319 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -449,7 +449,10 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
{
u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
- if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
+ /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
+ * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
+ */
+ if (skb_overhead + vvs->buf_used + len > (u64)vvs->buf_alloc * 2)
return false;
vvs->rx_bytes += len;
@@ -1365,7 +1368,7 @@ virtio_transport_recv_connecting(struct sock *sk,
return err;
}
-static void
+static bool
virtio_transport_recv_enqueue(struct vsock_sock *vsk,
struct sk_buff *skb)
{
@@ -1380,10 +1383,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_lock_bh(&vvs->rx_lock);
can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
- if (!can_enqueue) {
- free_pkt = true;
+ if (!can_enqueue)
goto out;
- }
if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
vvs->msg_count++;
@@ -1423,6 +1424,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_unlock_bh(&vvs->rx_lock);
if (free_pkt)
kfree_skb(skb);
+
+ return can_enqueue;
}
static int
@@ -1435,7 +1438,16 @@ virtio_transport_recv_connected(struct sock *sk,
switch (le16_to_cpu(hdr->op)) {
case VIRTIO_VSOCK_OP_RW:
- virtio_transport_recv_enqueue(vsk, skb);
+ if (!virtio_transport_recv_enqueue(vsk, skb)) {
+ /* There is no more space to queue the packet, so let's
+ * close the connection; otherwise, we'll lose data.
+ */
+ (void)virtio_transport_reset(vsk, skb);
+ sk->sk_state = TCP_CLOSE;
+ sk->sk_err = ENOBUFS;
+ sk_error_report(sk);
+ break;
+ }
vsock_data_ready(sk);
return err;
case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
--
2.54.0
^ permalink raw reply related
* [RESEND PATCH v3] drm/qxl: Convert qxl release idr to xarray
From: liuqiangneo @ 2026-05-12 7:34 UTC (permalink / raw)
To: kraxel, airlied
Cc: airlied, simona, maarten.lankhorst, mripard, tzimmermann,
virtualization, spice-devel, dri-devel, linux-kernel, Qiang Liu
From: Qiang Liu <liuqiang@kylinos.cn>
Replace the release_idr + release_idr_lock to an XArray.
IDR internally uses xarray so we can use it directly which simplifies our
code by removing the IDR wrapper.
Signed-off-by: Qiang Liu <liuqiang@kylinos.cn>
---
RESEND v3:
- Address Sashiko's review.
- DRM atomic_update does not run in a no-sleep atomic context;
the "atomic" means DRM state update semantics, not CPU atomicity,
so GFP_KERNEL is safe to use here.
- In current qxl_kms.c, it appears to be avoided by XA_FLAGS_ALLOC1.
- Request maintainers'further review. Thanks
v3:
- Change the type of 'handle' to u32.
'xa_limit_31b' restricts the range of 'handle',
so an implicit cast to 'int' is safe.
- Keep xa_lock in xa_lock due to concurrent access.
v2:
- Use xa_limit_31b instead of xa_limit_32b to keep IDs within INT_MAX.
- Use GFP_KERNEL instead of GFP_NOWAIT because the context is sleepable.
- Cast to u32 to avoid sign extension when atomic counter wraps above INT_MAX.
---
drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--
drivers/gpu/drm/qxl/qxl_kms.c | 4 +--
drivers/gpu/drm/qxl/qxl_release.c | 49 ++++++++++++++-----------------
3 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index cc02b5f10ad9..cf9decf39022 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -35,6 +35,7 @@
#include <linux/firmware.h>
#include <linux/platform_device.h>
#include <linux/workqueue.h>
+#include <linux/xarray.h>
#include <drm/drm_crtc.h>
#include <drm/drm_encoder.h>
@@ -208,11 +209,10 @@ struct qxl_device {
struct qxl_memslot surfaces_slot;
spinlock_t release_lock;
- struct idr release_idr;
- uint32_t release_seqno;
+ struct xarray release_xa;
+ atomic_t release_seqno;
atomic_t release_count;
wait_queue_head_t release_event;
- spinlock_t release_idr_lock;
struct mutex async_io_mutex;
unsigned int last_sent_io_cmd;
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 461b7ab9ad5c..0cebaf88f407 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -227,8 +227,8 @@ int qxl_device_init(struct qxl_device *qdev,
goto cursor_ring_free;
}
- idr_init_base(&qdev->release_idr, 1);
- spin_lock_init(&qdev->release_idr_lock);
+ xa_init_flags(&qdev->release_xa, XA_FLAGS_ALLOC1);
+ atomic_set(&qdev->release_seqno, 0);
spin_lock_init(&qdev->release_lock);
idr_init_base(&qdev->surf_id_idr, 1);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 06979d0e8a9f..afed1096cf7b 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -88,8 +88,9 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
struct qxl_release **ret)
{
struct qxl_release *release;
- int handle;
+ u32 handle;
size_t size = sizeof(*release);
+ int r;
release = kmalloc(size, GFP_KERNEL);
if (!release) {
@@ -102,19 +103,15 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
release->surface_release_id = 0;
INIT_LIST_HEAD(&release->bos);
- idr_preload(GFP_KERNEL);
- spin_lock(&qdev->release_idr_lock);
- handle = idr_alloc(&qdev->release_idr, release, 1, 0, GFP_NOWAIT);
- release->base.seqno = ++qdev->release_seqno;
- spin_unlock(&qdev->release_idr_lock);
- idr_preload_end();
- if (handle < 0) {
+ r = xa_alloc(&qdev->release_xa, &handle, release, xa_limit_31b, GFP_KERNEL);
+ release->base.seqno = (u32)atomic_inc_return(&qdev->release_seqno);
+ if (r < 0) {
kfree(release);
*ret = NULL;
- return handle;
+ return r;
}
*ret = release;
- DRM_DEBUG_DRIVER("allocated release %d\n", handle);
+ DRM_DEBUG_DRIVER("allocated release %u\n", handle);
release->id = handle;
return handle;
}
@@ -143,9 +140,7 @@ qxl_release_free(struct qxl_device *qdev,
if (release->surface_release_id)
qxl_surface_id_dealloc(qdev, release->surface_release_id);
- spin_lock(&qdev->release_idr_lock);
- idr_remove(&qdev->release_idr, release->id);
- spin_unlock(&qdev->release_idr_lock);
+ xa_erase(&qdev->release_xa, release->id);
if (dma_fence_was_initialized(&release->base)) {
WARN_ON(list_empty(&release->bos));
@@ -261,14 +256,14 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
struct qxl_release **release)
{
if (surface_cmd_type == QXL_SURFACE_CMD_DESTROY && create_rel) {
- int idr_ret;
+ int xa_ret;
struct qxl_bo *bo;
union qxl_release_info *info;
/* stash the release after the create command */
- idr_ret = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD, release);
- if (idr_ret < 0)
- return idr_ret;
+ xa_ret = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD, release);
+ if (xa_ret < 0)
+ return xa_ret;
bo = create_rel->release_bo;
(*release)->release_bo = bo;
@@ -277,7 +272,7 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
qxl_release_list_add(*release, bo);
info = qxl_release_map(qdev, *release);
- info->id = idr_ret;
+ info->id = xa_ret;
qxl_release_unmap(qdev, *release, info);
return 0;
}
@@ -291,7 +286,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size,
struct qxl_bo **rbo)
{
struct qxl_bo *bo, *free_bo = NULL;
- int idr_ret;
+ int xa_ret;
int ret = 0;
union qxl_release_info *info;
int cur_idx;
@@ -312,11 +307,11 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size,
return -EINVAL;
}
- idr_ret = qxl_release_alloc(qdev, type, release);
- if (idr_ret < 0) {
+ xa_ret = qxl_release_alloc(qdev, type, release);
+ if (xa_ret < 0) {
if (rbo)
*rbo = NULL;
- return idr_ret;
+ return xa_ret;
}
atomic_inc(&qdev->release_count);
@@ -362,7 +357,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size,
}
info = qxl_release_map(qdev, *release);
- info->id = idr_ret;
+ info->id = xa_ret;
qxl_release_unmap(qdev, *release, info);
return ret;
@@ -373,11 +368,11 @@ struct qxl_release *qxl_release_from_id_locked(struct qxl_device *qdev,
{
struct qxl_release *release;
- spin_lock(&qdev->release_idr_lock);
- release = idr_find(&qdev->release_idr, id);
- spin_unlock(&qdev->release_idr_lock);
+ xa_lock(&qdev->release_xa);
+ release = xa_load(&qdev->release_xa, id);
+ xa_unlock(&qdev->release_xa);
if (!release) {
- DRM_ERROR("failed to find id in release_idr\n");
+ DRM_ERROR("failed to find id in release_xa\n");
return NULL;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-12 7:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Michael S. Tsirkin, netdev, Eric Dumazet, Stefan Hajnoczi,
virtualization, David S. Miller, Jason Wang, Simon Horman,
linux-kernel, Paolo Abeni, Xuan Zhuo, kvm, Eugenio Pérez
In-Reply-To: <20260511164841.7f7ff557@kernel.org>
On Mon, May 11, 2026 at 04:48:41PM -0700, Jakub Kicinski wrote:
>On Mon, 11 May 2026 15:17:56 +0200 Stefano Garzarella wrote:
>> > > Okay, I thought it over over the weekend, and I agree that this patch
>> > > still doesn't solve the problem and would still result in packet loss.
>> > > So, until we resolve the issue permanently, and since 059b7dbd20a6 is
>> > > coming to stable, I'd like to rework this patch so that we only start
>> > > dropping packets when the overhead plus the queued bytes exceeds
>> > > `buf_alloc * 2`.
>> > > Removing the other changes to reduce the buf_alloc advertised, but
>> > > terminating the connection so that both peers are aware that something
>> > > went wrong.
>> > >
>> > > Any objections?
>> > >
>> > > Stefano
>> >
>> > Let's try to first fix it upstream properly please. Discuss backporting
>> > later.
>>
>> Commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
>> queue") is already in Linus tree and will land soon on stable. Which
>> issue do you see on having a patch on top of that to close the
>> connection instead of losing data and breaking our test suite?
>>
>> IMO we need that change in any case, because the previous code also
>> discarded packets without any notification, whereas breaking the
>> connection would be better in that case.
>
>Sorry if I'm speaking out of turn or misunderstanding but, like Michael
>says, let's focus on fixing this the way we want it to be fixed?
IMHO, these are two separate issues (related but still separated):
1. taking overhead into account and setting a limit (which Eric started
doing in 059b7dbd20a6)
2. reducing overhead by also merging SEQPACKET as Michael proposed (we
already do this for STREAM).
Even with option 2, the overhead won’t disappear, and we should maintain
a limit as Eric rightly pointed out, especially since the initial
problem was a flood of EOMs with 0 or 1 bytes, which option 2 doesn’t
solve, since the overhead will eventually explode.
So I agree that we should implement 2 as Michael proposed, but I think
we also need to fix 1 by improving it slightly (as I’ve tried here, and
would like to do in v2).
Since I have v2 ready and tested, I'll send it because I think we should
have it in any case, even with option 2 implemented.
>IIUC you are trying to minimize the size of the fix, please don't worry
>about the LoC in the diff at this stage.
>
Okay, I'll try to work on it, but this week is a nightmare. I hope to
come up with something by building on Michael's idea.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH net v2] vsock/vmci: fix UAF when peer resets connection during handshake
From: Stefano Garzarella @ 2026-05-12 7:06 UTC (permalink / raw)
To: Minh Nguyen
Cc: Bryan Tan, Vishnu Dasa, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman,
bcm-kernel-feedback-list, netdev, virtualization, linux-kernel,
stable
In-Reply-To: <20260512025851.189140-1-minhnguyen.080505@gmail.com>
On Tue, May 12, 2026 at 09:58:51AM +0700, Minh Nguyen wrote:
>vmci_transport_recv_connecting_server() jumps to its destroy: label
>and performs an unconditional sock_put(pending) to release the
>explicit sock_hold() taken by vmci_transport_recv_listen() before
>schedule_delayed_work(). The existing comment claimed this was safe
>because the listen handler removes pending from the pending list on
>the way out, which would prevent vsock_pending_work() from dropping
>the same reference later.
>
>That assumption breaks for a peer RST. The default arm of the packet
>switch sets:
>
> err = pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST ? 0 : -EINVAL;
>
>and vmci_transport_recv_listen() only calls vsock_remove_pending()
>when err < 0:
>
> if (err < 0)
> vsock_remove_pending(sk, pending);
>
>For RST (err == 0) the socket stays on the pending list, so when
>vsock_pending_work() fires it takes the is_pending=true path and
>drops all three references itself: the pending-list reference via
>vsock_remove_pending(), then the two trailing sock_put(sk) calls.
>The unconditional sock_put() in destroy: had already dropped the
>explicit sock_hold() reference, so the second trailing sock_put(sk)
>in vsock_pending_work() is a write into the freed AF_VSOCK slab
>object. KASAN reports a slab-use-after-free write of 4 bytes from
>refcount_warn_saturate() on the workqueue path:
>
> BUG: KASAN: slab-use-after-free in refcount_warn_saturate
> Write of size 4 at addr ffff88800b1cac80 by task kworker
> Workqueue: events vsock_pending_work
> Call Trace:
> refcount_warn_saturate
> vsock_pending_work
> process_one_work
> worker_thread
>
>Triggering the bug requires only the ability to open a VSOCK
>connection to the target and send a RST before the listener accepts.
>
>Skip the sock_put() in destroy: when err == 0 so it only compensates
>the cases where vmci_transport_recv_listen() actually calls
>vsock_remove_pending(). RST is the only path that reaches destroy:
>with err == 0; every other path produces a negative value, so their
>behaviour is unchanged.
>
>Verified on lts-6.12.79 with KASAN enabled (CONFIG_KASAN_INLINE=y,
>kasan_multi_shot): same trigger binary, same VM, 100 iterations:
>without this patch 52 KASAN slab-use-after-free reports fire; with
>this patch applied, 0 reports.
>
>Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>Cc: stable@vger.kernel.org
>Signed-off-by: Minh Nguyen <minhnguyen.080505@gmail.com>
>Assisted-by: Claude:claude-opus-4-7
>---
>v2:
> - Resubmit to netdev per Stefano Garzarella's request after v1 review.
> - Retested the PoC with the patch applied on lts-6.12.79 with KASAN
> enabled: 52/100 unpatched -> 0/100 patched (same trigger binary,
> same VM, 100 iterations); test summary captured in the commit
> message.
> - Changed Cc: stable@kernel.org -> stable@vger.kernel.org now that the
> bug is no longer embargoed.
> - Rebased onto net/main (no functional change to the diff).
>
>v1 was sent to security@kernel.org on 2026-05-10 (not on lore archives;
>no public link available). v1 review summary, for reference:
> - Stefano Garzarella (vsock maintainer): "Overall LGTM, but I'd wait
> vmware guys on this that know this code better." Asked for retest
> and resubmission via the net tree workflow.
> - Bryan Tan (VMCI maintainer): "Thanks for the fix, it looks good to
> me." Also noted that no modern VMware product allows guest-to-guest
> VMCI communication, so the practical attack surface is host -> guest.
>
> net/vmw_vsock/vmci_transport.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 4296ca1..88d7128 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -1269,14 +1269,16 @@ vmci_transport_recv_connecting_server(struct sock *listener,
> destroy:
> pending->sk_err = skerr;
> pending->sk_state = TCP_CLOSE;
>- /* As long as we drop our reference, all necessary cleanup will handle
>- * when the cleanup function drops its reference and our destruct
>- * implementation is called. Note that since the listen handler will
>- * remove pending from the pending list upon our failure, the cleanup
>- * function won't drop the additional reference, which is why we do it
>- * here.
>+ /* Drop the reference taken by vmci_transport_recv_listen() before
>+ * schedule_delayed_work() only on real errors. For a peer RST
>+ * (err == 0) the listener leaves pending on the pending list, and
>+ * vsock_pending_work() will drop that reference itself when it
>+ * later cleans the socket up. Calling sock_put() here in that
>+ * case would be a double-put and free the socket while
>+ * vsock_pending_work() still holds it.
> */
>- sock_put(pending);
>+ if (err < 0)
>+ sock_put(pending);
>
> return err;
> }
>
>base-commit: be48e5fe51a5864566307998286a699d6b986934
>--
>2.54.0
>
^ permalink raw reply
* Re: [PATCH v2] drm/virtio: move cursor resv lock acquisition to prepare_fb
From: Dmitry Osipenko @ 2026-05-12 6:34 UTC (permalink / raw)
To: Deepanshu Kartikey, airlied, kraxel, gurchetansingh, olvaffe,
maarten.lankhorst, mripard, tzimmermann, simona, sumit.semwal,
christian.koenig
Cc: dri-devel, virtualization, linux-kernel, linux-media,
linaro-mm-sig, syzbot+72bd3dd3a5d5f39a0271, stable
In-Reply-To: <20260512020718.108044-1-kartikey406@gmail.com>
On 5/12/26 05:07, Deepanshu Kartikey wrote:
> virtio_gpu_cursor_plane_update() allocates a virtio_gpu_object_array,
> locks its dma_resv, and queues a fenced transfer to the host. The
> lock acquisition can fail in two ways:
>
> - dma_resv_lock_interruptible() returns -EINTR when a signal is
> delivered while waiting for the reservation lock.
> - dma_resv_reserve_fences() returns -ENOMEM if it fails to allocate
> a fence slot; in this case lock_resv unlocks before returning.
>
> The return value was ignored, so the cursor path could proceed with
> the resv lock not held. The queue path then walks the object array
> and calls dma_resv_add_fence(), which requires the lock; with lockdep
> enabled this trips dma_resv_assert_held():
>
> WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840
> Call Trace:
> virtio_gpu_array_add_fence
> virtio_gpu_queue_ctrl_sgs
> virtio_gpu_queue_fenced_ctrl_buffer
> virtio_gpu_cursor_plane_update
> drm_atomic_helper_commit_planes
> drm_atomic_helper_commit_tail
> commit_tail
> drm_atomic_helper_commit
> drm_atomic_commit
> drm_atomic_helper_update_plane
> __setplane_atomic
> drm_mode_cursor_universal
> drm_mode_cursor_common
> drm_mode_cursor_ioctl
> drm_ioctl
> __x64_sys_ioctl
>
> Beyond the WARN, mutating the dma_resv fence list without the lock
> races with concurrent readers/writers and can corrupt the list.
>
> The DRM atomic helpers do not allow .atomic_update to fail: by the
> time it runs, the commit has been signed off to userspace and there
> is no clean rollback path. Move the fallible work -- objs allocation,
> dma_resv locking, and fence slot reservation -- into
> virtio_gpu_plane_prepare_fb, which is the designated callback for
> resource acquisition and may return errors that the framework
> handles by rolling back the commit. Stash the prepared object array
> on virtio_gpu_plane_state so the update step can consume it.
>
> Make virtio_gpu_plane_cleanup_fb release the objs if the commit was
> rolled back before update ran (i.e., objs not consumed). The queue
> path already unlocks the resv after attaching the fence (vq.c:411)
> and frees the array via put_free_delayed after host completion
> (vq.c:271), so the update step only needs to clear vgplane_st->objs
> to transfer ownership.
>
> Simplify virtio_gpu_cursor_plane_update to a no-fail queue submission
> that hands the prepared, locked objs to the queue path.
>
> The bug was reported by syzbot, triggered via fault injection
> (fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the
> -ENOMEM branch in dma_resv_reserve_fences().
>
> Reported-by: syzbot+72bd3dd3a5d5f39a0271@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271
> Fixes: 5cfd31c5b3a3 ("drm/virtio: fix virtio_gpu_cursor_plane_update().")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/all/20260510053025.100224-1-kartikey406@gmail.com/T/ [v1]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> v2: Move resv lock acquisition from .atomic_update (which must not
> fail) to .prepare_fb (which may), per maintainer review of v1.
> The previous approach of silently skipping the cursor update on
> lock failure violated the atomic-commit contract with userspace.
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
> drivers/gpu/drm/virtio/virtgpu_plane.c | 38 ++++++++++++++++++++------
> 2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index f17660a71a3e..e51f959dce46 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -198,6 +198,7 @@ struct virtio_gpu_framebuffer {
> struct virtio_gpu_plane_state {
> struct drm_plane_state base;
> struct virtio_gpu_fence *fence;
> + struct virtio_gpu_object_array *objs;
> };
> #define to_virtio_gpu_plane_state(x) \
> container_of(x, struct virtio_gpu_plane_state, base)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a126d1b25f46..b0511ace89e6 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -381,6 +381,23 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
> goto err_fence;
> }
>
> + if (plane->type == DRM_PLANE_TYPE_CURSOR && bo->dumb) {
> + struct virtio_gpu_object_array *objs;
> +
> + objs = virtio_gpu_array_alloc(1);
> + if (!objs) {
> + ret = -ENOMEM;
> + goto err_fence;
> + }
> + virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
> + ret = virtio_gpu_array_lock_resv(objs);
> + if (ret) {
> + virtio_gpu_array_put_free(objs);
> + goto err_fence;
> + }
> + vgplane_st->objs = objs;
> + }
> +
> return 0;
>
> err_fence:
> @@ -417,6 +434,12 @@ static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane,
> vgplane_st->fence = NULL;
> }
>
> + if (vgplane_st->objs) {
> + virtio_gpu_array_unlock_resv(vgplane_st->objs);
> + virtio_gpu_array_put_free(vgplane_st->objs);
> + vgplane_st->objs = NULL;
> + }
> +
> obj = state->fb->obj[0];
> if (drm_gem_is_imported(obj))
> virtio_gpu_cleanup_imported_obj(obj);
> @@ -452,21 +475,18 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,
> }
>
> if (bo && bo->dumb && (plane->state->fb != old_state->fb)) {
> - /* new cursor -- update & wait */
> - struct virtio_gpu_object_array *objs;
> -
> - objs = virtio_gpu_array_alloc(1);
> - if (!objs)
> - return;
> - virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
> - virtio_gpu_array_lock_resv(objs);
> + /* objs and fence were prepared in virtio_gpu_plane_prepare_fb;
> + * the resv is already locked. The queue path takes ownership
> + * of objs and unlocks the resv after attaching the fence.
> + */
> virtio_gpu_cmd_transfer_to_host_2d
> (vgdev, 0,
> plane->state->crtc_w,
> plane->state->crtc_h,
> - 0, 0, objs, vgplane_st->fence);
> + 0, 0, vgplane_st->objs, vgplane_st->fence);
> virtio_gpu_notify(vgdev);
> dma_fence_wait(&vgplane_st->fence->f, true);
> + vgplane_st->objs = NULL;
> }
>
> if (plane->state->fb != old_state->fb) {
I'm getting lockup with this patch applied and now see that
virtio_gpu_resource_flush() also locks BO.
Easiest option might be to add uninterruptible variant of
virtio_gpu_array_lock_resv(). Could you please try it for v3?
--
Best regards,
Dmitry
^ permalink raw reply
* [PATCH net v2] vsock/vmci: fix UAF when peer resets connection during handshake
From: Minh Nguyen @ 2026-05-12 2:58 UTC (permalink / raw)
To: Bryan Tan, Vishnu Dasa, Stefano Garzarella
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, bcm-kernel-feedback-list, netdev, virtualization,
linux-kernel, stable
vmci_transport_recv_connecting_server() jumps to its destroy: label
and performs an unconditional sock_put(pending) to release the
explicit sock_hold() taken by vmci_transport_recv_listen() before
schedule_delayed_work(). The existing comment claimed this was safe
because the listen handler removes pending from the pending list on
the way out, which would prevent vsock_pending_work() from dropping
the same reference later.
That assumption breaks for a peer RST. The default arm of the packet
switch sets:
err = pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST ? 0 : -EINVAL;
and vmci_transport_recv_listen() only calls vsock_remove_pending()
when err < 0:
if (err < 0)
vsock_remove_pending(sk, pending);
For RST (err == 0) the socket stays on the pending list, so when
vsock_pending_work() fires it takes the is_pending=true path and
drops all three references itself: the pending-list reference via
vsock_remove_pending(), then the two trailing sock_put(sk) calls.
The unconditional sock_put() in destroy: had already dropped the
explicit sock_hold() reference, so the second trailing sock_put(sk)
in vsock_pending_work() is a write into the freed AF_VSOCK slab
object. KASAN reports a slab-use-after-free write of 4 bytes from
refcount_warn_saturate() on the workqueue path:
BUG: KASAN: slab-use-after-free in refcount_warn_saturate
Write of size 4 at addr ffff88800b1cac80 by task kworker
Workqueue: events vsock_pending_work
Call Trace:
refcount_warn_saturate
vsock_pending_work
process_one_work
worker_thread
Triggering the bug requires only the ability to open a VSOCK
connection to the target and send a RST before the listener accepts.
Skip the sock_put() in destroy: when err == 0 so it only compensates
the cases where vmci_transport_recv_listen() actually calls
vsock_remove_pending(). RST is the only path that reaches destroy:
with err == 0; every other path produces a negative value, so their
behaviour is unchanged.
Verified on lts-6.12.79 with KASAN enabled (CONFIG_KASAN_INLINE=y,
kasan_multi_shot): same trigger binary, same VM, 100 iterations:
without this patch 52 KASAN slab-use-after-free reports fire; with
this patch applied, 0 reports.
Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Cc: stable@vger.kernel.org
Signed-off-by: Minh Nguyen <minhnguyen.080505@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
v2:
- Resubmit to netdev per Stefano Garzarella's request after v1 review.
- Retested the PoC with the patch applied on lts-6.12.79 with KASAN
enabled: 52/100 unpatched -> 0/100 patched (same trigger binary,
same VM, 100 iterations); test summary captured in the commit
message.
- Changed Cc: stable@kernel.org -> stable@vger.kernel.org now that the
bug is no longer embargoed.
- Rebased onto net/main (no functional change to the diff).
v1 was sent to security@kernel.org on 2026-05-10 (not on lore archives;
no public link available). v1 review summary, for reference:
- Stefano Garzarella (vsock maintainer): "Overall LGTM, but I'd wait
vmware guys on this that know this code better." Asked for retest
and resubmission via the net tree workflow.
- Bryan Tan (VMCI maintainer): "Thanks for the fix, it looks good to
me." Also noted that no modern VMware product allows guest-to-guest
VMCI communication, so the practical attack surface is host -> guest.
net/vmw_vsock/vmci_transport.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 4296ca1..88d7128 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -1269,14 +1269,16 @@ vmci_transport_recv_connecting_server(struct sock *listener,
destroy:
pending->sk_err = skerr;
pending->sk_state = TCP_CLOSE;
- /* As long as we drop our reference, all necessary cleanup will handle
- * when the cleanup function drops its reference and our destruct
- * implementation is called. Note that since the listen handler will
- * remove pending from the pending list upon our failure, the cleanup
- * function won't drop the additional reference, which is why we do it
- * here.
+ /* Drop the reference taken by vmci_transport_recv_listen() before
+ * schedule_delayed_work() only on real errors. For a peer RST
+ * (err == 0) the listener leaves pending on the pending list, and
+ * vsock_pending_work() will drop that reference itself when it
+ * later cleans the socket up. Calling sock_put() here in that
+ * case would be a double-put and free the socket while
+ * vsock_pending_work() still holds it.
*/
- sock_put(pending);
+ if (err < 0)
+ sock_put(pending);
return err;
}
base-commit: be48e5fe51a5864566307998286a699d6b986934
--
2.54.0
^ permalink raw reply related
* Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Gregory Price @ 2026-05-12 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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, Liam R. Howlett,
Harry Yoo, Hao Li
In-Reply-To: <20260511175032-mutt-send-email-mst@kernel.org>
On Mon, May 11, 2026 at 05:59:23PM -0400, Michael S. Tsirkin wrote:
> On Mon, May 11, 2026 at 12:52:10PM -0400, Gregory Price wrote:
> > On Mon, May 11, 2026 at 11:55:40AM -0400, Michael S. Tsirkin wrote:
> > > On Mon, May 11, 2026 at 11:37:37AM -0400, Gregory Price wrote:
> > > > On Mon, May 11, 2026 at 05:01:55AM -0400, Michael S. Tsirkin wrote:
> > > >
> > > > > +/*
> > > > > + * Sentinel for user_addr: indicates a non-user allocation.
> > > > > + * Cannot use 0 because address 0 is a valid userspace mapping.
> > > > > + */
> > > > > +#define USER_ADDR_NONE ((unsigned long)-1)
> > > >
> > > > Ehm, hm. Does -1 hold as a non-user address across all architectures?
> > > >
> > > > What about in linear addressing / no VM mode?
> > >
> > > this is used on a fault. I don't think there are any faults then?
> > > But maybe FAULT_ADDR_NONE would be clearer.
> > >
> >
> > Meh, naming here is less relevant than the sentinel correctness.
> >
> > My only concern is really whether -1 could end up being a valid address
> > in some horrid future timeline and this all going belly up.
> >
> > Is why I asked about whether this is correct on all architectures.
>
> I think the answer is yes: on all architectures Linux supports, the last
> page of the address space is never a valid user mapping. The kernel
> enforces this -- mmap will not create a mapping whose end wraps past -1.
>
> if (addr > TASK_SIZE - len)
> return -ENOMEM;
>
>
> So the maximum vm_end = addr + len <= TASK_SIZE.
> On every architecture TASK_SIZE <= (unsigned long)-1, so vm_end can
> never wrap around to 0 and address -1 can never be within any
> VMA.
>
That is worth documenting at least in the changelog if not the comment
with the define there's a clue if anything ever changes.
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 14/30] mm: hugetlb: use __GFP_ZERO and skip zeroing for zeroed pages
From: Gregory Price @ 2026-05-12 2:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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
In-Reply-To: <20260511174337-mutt-send-email-mst@kernel.org>
On Mon, May 11, 2026 at 05:47:48PM -0400, Michael S. Tsirkin wrote:
> On Mon, May 11, 2026 at 12:36:59PM -0400, Gregory Price wrote:
> >
> > This feels like a very odd pattern:
> >
> > 1) ask for __GFP_ZERO
> > 2) Have to check whether it was actually zeroed
> >
> > Seems like the zeroing piece should just be sunk in if you're going to
> > ask for __GFP_ZERO anyway. And in that case, maybe just `bool zero` as
> > an argument, rather than GFP (to avoid future overloading of flags).
> >
> > ~Gregory
>
> Heh. The reason is that it either allocates from buddy - using gfp flags
> or from the pool, in which case it zeroes.
>
> We could even avoid the bool - just test __GFP_ZERO inside
> alloc_hugetlb_folio. Would that be better?
>
Hard to know until we see the full shape of things, but it seems
reasonable if we can eliminate one or both new arguments that this would
be a good thing and the logic should just be sunk into hugetlb.
~Gregory
^ permalink raw reply
* Re: [PATCH] drm/virtio: check virtio_gpu_array_lock_resv() return in cursor update
From: Deepanshu Kartikey @ 2026-05-12 2:07 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: airlied, kraxel, gurchetansingh, olvaffe, maarten.lankhorst,
mripard, tzimmermann, simona, sumit.semwal, christian.koenig,
dri-devel, virtualization, linux-kernel, linux-media,
linaro-mm-sig, syzbot+72bd3dd3a5d5f39a0271, stable
In-Reply-To: <e1741cf2-3416-4464-bcae-741f0c87448b@collabora.com>
On Mon, May 11, 2026 at 2:41 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
> plane->state->crtc_w,
>
> Thanks for the patch. Atomic update shouldn't fail due to non-critical
> errors like on a signal interrupt. Could you please move this code that
> may fail in update() to .prepare/cleanup_fb() callbacks?
>
> --
> Best regards,
> Dmitry
Thanks for the feedback . I have sent patch v2.
Thanks
Deepanshu kartikey
^ permalink raw reply
* [PATCH v2] drm/virtio: move cursor resv lock acquisition to prepare_fb
From: Deepanshu Kartikey @ 2026-05-12 2:07 UTC (permalink / raw)
To: airlied, kraxel, dmitry.osipenko, gurchetansingh, olvaffe,
maarten.lankhorst, mripard, tzimmermann, simona, sumit.semwal,
christian.koenig
Cc: dri-devel, virtualization, linux-kernel, linux-media,
linaro-mm-sig, Deepanshu Kartikey, syzbot+72bd3dd3a5d5f39a0271,
stable
virtio_gpu_cursor_plane_update() allocates a virtio_gpu_object_array,
locks its dma_resv, and queues a fenced transfer to the host. The
lock acquisition can fail in two ways:
- dma_resv_lock_interruptible() returns -EINTR when a signal is
delivered while waiting for the reservation lock.
- dma_resv_reserve_fences() returns -ENOMEM if it fails to allocate
a fence slot; in this case lock_resv unlocks before returning.
The return value was ignored, so the cursor path could proceed with
the resv lock not held. The queue path then walks the object array
and calls dma_resv_add_fence(), which requires the lock; with lockdep
enabled this trips dma_resv_assert_held():
WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840
Call Trace:
virtio_gpu_array_add_fence
virtio_gpu_queue_ctrl_sgs
virtio_gpu_queue_fenced_ctrl_buffer
virtio_gpu_cursor_plane_update
drm_atomic_helper_commit_planes
drm_atomic_helper_commit_tail
commit_tail
drm_atomic_helper_commit
drm_atomic_commit
drm_atomic_helper_update_plane
__setplane_atomic
drm_mode_cursor_universal
drm_mode_cursor_common
drm_mode_cursor_ioctl
drm_ioctl
__x64_sys_ioctl
Beyond the WARN, mutating the dma_resv fence list without the lock
races with concurrent readers/writers and can corrupt the list.
The DRM atomic helpers do not allow .atomic_update to fail: by the
time it runs, the commit has been signed off to userspace and there
is no clean rollback path. Move the fallible work -- objs allocation,
dma_resv locking, and fence slot reservation -- into
virtio_gpu_plane_prepare_fb, which is the designated callback for
resource acquisition and may return errors that the framework
handles by rolling back the commit. Stash the prepared object array
on virtio_gpu_plane_state so the update step can consume it.
Make virtio_gpu_plane_cleanup_fb release the objs if the commit was
rolled back before update ran (i.e., objs not consumed). The queue
path already unlocks the resv after attaching the fence (vq.c:411)
and frees the array via put_free_delayed after host completion
(vq.c:271), so the update step only needs to clear vgplane_st->objs
to transfer ownership.
Simplify virtio_gpu_cursor_plane_update to a no-fail queue submission
that hands the prepared, locked objs to the queue path.
The bug was reported by syzbot, triggered via fault injection
(fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the
-ENOMEM branch in dma_resv_reserve_fences().
Reported-by: syzbot+72bd3dd3a5d5f39a0271@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271
Fixes: 5cfd31c5b3a3 ("drm/virtio: fix virtio_gpu_cursor_plane_update().")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260510053025.100224-1-kartikey406@gmail.com/T/ [v1]
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v2: Move resv lock acquisition from .atomic_update (which must not
fail) to .prepare_fb (which may), per maintainer review of v1.
The previous approach of silently skipping the cursor update on
lock failure violated the atomic-commit contract with userspace.
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_plane.c | 38 ++++++++++++++++++++------
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..e51f959dce46 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -198,6 +198,7 @@ struct virtio_gpu_framebuffer {
struct virtio_gpu_plane_state {
struct drm_plane_state base;
struct virtio_gpu_fence *fence;
+ struct virtio_gpu_object_array *objs;
};
#define to_virtio_gpu_plane_state(x) \
container_of(x, struct virtio_gpu_plane_state, base)
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a126d1b25f46..b0511ace89e6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -381,6 +381,23 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
goto err_fence;
}
+ if (plane->type == DRM_PLANE_TYPE_CURSOR && bo->dumb) {
+ struct virtio_gpu_object_array *objs;
+
+ objs = virtio_gpu_array_alloc(1);
+ if (!objs) {
+ ret = -ENOMEM;
+ goto err_fence;
+ }
+ virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
+ ret = virtio_gpu_array_lock_resv(objs);
+ if (ret) {
+ virtio_gpu_array_put_free(objs);
+ goto err_fence;
+ }
+ vgplane_st->objs = objs;
+ }
+
return 0;
err_fence:
@@ -417,6 +434,12 @@ static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane,
vgplane_st->fence = NULL;
}
+ if (vgplane_st->objs) {
+ virtio_gpu_array_unlock_resv(vgplane_st->objs);
+ virtio_gpu_array_put_free(vgplane_st->objs);
+ vgplane_st->objs = NULL;
+ }
+
obj = state->fb->obj[0];
if (drm_gem_is_imported(obj))
virtio_gpu_cleanup_imported_obj(obj);
@@ -452,21 +475,18 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,
}
if (bo && bo->dumb && (plane->state->fb != old_state->fb)) {
- /* new cursor -- update & wait */
- struct virtio_gpu_object_array *objs;
-
- objs = virtio_gpu_array_alloc(1);
- if (!objs)
- return;
- virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
- virtio_gpu_array_lock_resv(objs);
+ /* objs and fence were prepared in virtio_gpu_plane_prepare_fb;
+ * the resv is already locked. The queue path takes ownership
+ * of objs and unlocks the resv after attaching the fence.
+ */
virtio_gpu_cmd_transfer_to_host_2d
(vgdev, 0,
plane->state->crtc_w,
plane->state->crtc_h,
- 0, 0, objs, vgplane_st->fence);
+ 0, 0, vgplane_st->objs, vgplane_st->fence);
virtio_gpu_notify(vgdev);
dma_fence_wait(&vgplane_st->fence->f, true);
+ vgplane_st->objs = NULL;
}
if (plane->state->fb != old_state->fb) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Jakub Kicinski @ 2026-05-11 23:48 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Michael S. Tsirkin, netdev, Eric Dumazet, Stefan Hajnoczi,
virtualization, David S. Miller, Jason Wang, Simon Horman,
linux-kernel, Paolo Abeni, Xuan Zhuo, kvm, Eugenio Pérez
In-Reply-To: <CAGxU2F6TPfRxZ7SX20vXiqDM4_sY3yUQBUSfEM=Nr+TKXwT9mA@mail.gmail.com>
On Mon, 11 May 2026 15:17:56 +0200 Stefano Garzarella wrote:
> > > Okay, I thought it over over the weekend, and I agree that this patch
> > > still doesn't solve the problem and would still result in packet loss.
> > > So, until we resolve the issue permanently, and since 059b7dbd20a6 is
> > > coming to stable, I'd like to rework this patch so that we only start
> > > dropping packets when the overhead plus the queued bytes exceeds
> > > `buf_alloc * 2`.
> > > Removing the other changes to reduce the buf_alloc advertised, but
> > > terminating the connection so that both peers are aware that something
> > > went wrong.
> > >
> > > Any objections?
> > >
> > > Stefano
> >
> > Let's try to first fix it upstream properly please. Discuss backporting
> > later.
>
> Commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> queue") is already in Linus tree and will land soon on stable. Which
> issue do you see on having a patch on top of that to close the
> connection instead of losing data and breaking our test suite?
>
> IMO we need that change in any case, because the previous code also
> discarded packets without any notification, whereas breaking the
> connection would be better in that case.
Sorry if I'm speaking out of turn or misunderstanding but, like Michael
says, let's focus on fixing this the way we want it to be fixed?
IIUC you are trying to minimize the size of the fix, please don't worry
about the LoC in the diff at this stage.
^ permalink raw reply
* Re: [PATCH resend v6 10/30] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Michael S. Tsirkin @ 2026-05-11 22:02 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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, Liam R. Howlett
In-Reply-To: <agJPpxyu4gKqq8rj@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 05:52:39PM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:41:16PM -0400, Michael S. Tsirkin wrote:
> > On Mon, May 11, 2026 at 12:05:18PM -0400, Gregory Price wrote:
> > > On Mon, May 11, 2026 at 05:02:39AM -0400, Michael S. Tsirkin wrote:
> > > > Same change as the previous patch but for alloc_swap_folio:
> > > > pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> > > >
> > >
> > > Starting to seem like this should all just get squashed into once patch.
> > >
> > > ~Gregory
> >
> > I mean I was told the patches are too big and too hard to review.
> > It's just same, independent changes made in several places.
> > Does it matter if they are squashed or not?
> >
>
> Mostly i think the ordering of the patches thrashes from one set
> (alignment) to another set (USER_ADDR_NONE). If all of one set was
> pulled ahead of the other then it would be easier to follow.
>
> This particular set seemed trivial enough to just be one patch, but i
> don't think it matters all that much.
>
> ~Gregory
I can reorder and put alignment before threading, sure, that's trivial:
these patches are all independent and cause no conflicts when
reordering.
So far the main work will be the hugetlb thing - it really parallels
what I did with post_alloc_hook, to think of it.
Will take me a couple of days, I'm behind on other tasks.
--
MST
^ 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