Linux virtualization list
 help / color / mirror / Atom feed
* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-14 14:57 UTC (permalink / raw)
  To: netdev
  Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
	Simon Horman, Paolo Abeni, Jakub Kicinski, Jason Wang, kvm,
	Stefan Hajnoczi, virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260513105417.56761-2-sgarzare@redhat.com>

On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
>From: Stefano Garzarella <sgarzare@redhat.com>
>
>When there is no more space to queue an incoming packet, the packet is
>silently dropped. This causes data loss without any notification to
>either peer, since there is no retransmission.
>
>Under normal circumstances, this should never happen. However, it could
>happen if the other peer doesn't respect the credit, or if the skb
>overhead, which we recently began to take into account with commit
>059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
>is too high.
>
>Fix this by resetting the connection and setting the local socket error
>to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
>packet, so both peers are explicitly notified of the failure rather than
>silently losing data.
>
>Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 989cc252d3d3..4a4ac69d1ad1 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1350,7 +1350,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)
> {
>@@ -1365,10 +1365,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++;
>@@ -1408,6 +1406,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
>@@ -1420,7 +1420,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);

sashiko reported some issues related to setting TCP_CLOSE state and not 
removing the socket from the connect table:
https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com

I'll change this by calling virtio_transport_do_close() and 
vsock_remove_sock() in the next version.

Stefano

>+			break;
>+		}
> 		vsock_data_ready(sk);
> 		return err;
> 	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
>-- 
>2.54.0
>


^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Michael S. Tsirkin @ 2026-05-14 14:48 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: <agW3JyLAwVsCZ2qR@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 07:51:03AM -0400, Gregory Price wrote:
> On Tue, May 12, 2026 at 05:05:16PM -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.
> > 
> > While I was unable to quantify the performance difference, it is
> > an obvious waste, even if small.
> > 
> > Propagate the PageReported flag to sub-pages during expand(),
> > both in page_del_and_expand() and try_to_claim_block(), so
> > that they are recognized as already-reported.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: Claude:claude-opus-4-6
> ... snip ...
> > @@ -1731,9 +1740,10 @@ static __always_inline void page_del_and_expand(struct zone *zone,
> >  						int high, int migratetype)
> >  {
> >  	int nr_pages = 1 << high;
> > +	bool was_reported = page_reported(page);
> >  
> >  	__del_page_from_free_list(page, zone, high, migratetype);
> > -	nr_pages -= expand(zone, page, low, high, migratetype);
> > +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
> >  	account_freepages(zone, -nr_pages, migratetype);
> >  }
> >  
> 
> Maybe mildly out of scope but worth asking:  Are there other flags that
> should be retained/propogated on a split?  If so, rather than pass
> was_reported, should we just take a temporary copy of the page flags and
> pass them all in?
> 
> ~Gregory


Not that I can see, no.

-- 
MST


^ permalink raw reply

* Re: [PATCH net v3 2/2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-14 14:44 UTC (permalink / raw)
  To: netdev
  Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
	Simon Horman, Paolo Abeni, Jakub Kicinski, Jason Wang, kvm,
	Stefan Hajnoczi, virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260513105417.56761-3-sgarzare@redhat.com>

On Wed, May 13, 2026 at 12:54:17PM +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.
>
>With this patch, all tests in tools/testing/vsock/vsock_test.c are
>now passing again.
>
>Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 4a4ac69d1ad1..e22117bf5dcd 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -434,7 +434,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;

sashiko reported a potential overflow here:
https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com

I'll check the credit and overflow separately to ensure both are 
correct.

The portion relating to the user setting a buffer that is too small is a 
pre-existing issue and IMO an edge case that we can ignore.

Thanks,
Stefano


^ permalink raw reply

* Re: [PATCH v7 25/31] mm: page_alloc: propagate PG_zeroed in split_large_buddy
From: Gregory Price @ 2026-05-14 14:18 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: <2db0d124c647e711c0bf95b1c7f1316a0d1408ae.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:07:20PM -0400, Michael S. Tsirkin wrote:
> When splitting a large buddy page, propagate the PG_zeroed flag
> to each sub-page before freeing it.  __free_pages_prepare clears
> all flags (including PG_zeroed), so the flag must be re-set on
> each fragment after the split.  This ensures that the buddy merge
> logic can see PG_zeroed on pages that were part of a larger
> zeroed block.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh

Reviewed-by: Gregory Price <gourry@gourry.net>

> ---
>  mm/page_alloc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 468e8bde7d34..ce43f5a3dbaa 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1523,6 +1523,7 @@ static void split_large_buddy(struct zone *zone, struct page *page,
>  			      unsigned long pfn, int order, fpi_t fpi)
>  {
>  	unsigned long end = pfn + (1 << order);
> +	bool zeroed = PageZeroed(page);
>  
>  	VM_WARN_ON_ONCE(!IS_ALIGNED(pfn, 1 << order));
>  	/* Caller removed page from freelist, buddy info cleared! */
> @@ -1534,6 +1535,8 @@ static void split_large_buddy(struct zone *zone, struct page *page,
>  	do {
>  		int mt = get_pfnblock_migratetype(page, pfn);
>  
> +		if (zeroed)
> +			__SetPageZeroed(page);
>  		__free_one_page(page, pfn, zone, order, mt, fpi);
>  		pfn += 1 << order;
>  		if (pfn == end)
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH v7 22/31] mm: page_alloc: preserve PG_zeroed in page_del_and_expand
From: Gregory Price @ 2026-05-14 14:15 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: <9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:07:05PM -0400, Michael S. Tsirkin wrote:
> Propagate PG_zeroed through buddy splits in page_del_and_expand()
> and try_to_claim_block().  When a zeroed high-order page is split
> to satisfy a smaller allocation, the sub-pages placed back on the
> free lists keep PG_zeroed.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh

Already reviewed this, but noticing the same pattern - wondering if we
should just snapshot all the flags and plumb it through instead of just
extending functions like this to have infinite bools.

> ---
>  mm/page_alloc.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d70c9ba6b329..468e8bde7d34 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1712,7 +1712,8 @@ struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
>   * -- nyc
>   */
>  static inline unsigned int expand(struct zone *zone, struct page *page, int low,
> -				  int high, int migratetype, bool reported)
> +				  int high, int migratetype, bool reported,
> +				  bool zeroed)
>  {
>  	unsigned int size = 1 << high;
>  	unsigned int nr_added = 0;
> @@ -1743,6 +1744,8 @@ static inline unsigned int expand(struct zone *zone, struct page *page, int low,
>  		 */
>  		if (reported)
>  			__SetPageReported(&page[size]);
> +		if (zeroed)
> +			__SetPageZeroed(&page[size]);
>  	}
>  
>  	return nr_added;
> @@ -1754,10 +1757,12 @@ static __always_inline void page_del_and_expand(struct zone *zone,
>  {
>  	int nr_pages = 1 << high;
>  	bool was_reported = page_reported(page);
> +	bool was_zeroed = PageZeroed(page);
>  
>  	__del_page_from_free_list(page, zone, high, migratetype);
>  
> -	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
> +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported,
> +			   was_zeroed);
>  	account_freepages(zone, -nr_pages, migratetype);
>  }
>  
> @@ -2335,11 +2340,12 @@ try_to_claim_block(struct zone *zone, struct page *page,
>  	if (current_order >= pageblock_order) {
>  		unsigned int nr_added;
>  		bool was_reported = page_reported(page);
> +		bool was_zeroed = PageZeroed(page);
>  
>  		del_page_from_free_list(page, zone, current_order, block_type);
>  		change_pageblock_range(page, current_order, start_type);
>  		nr_added = expand(zone, page, order, current_order, start_type,
> -				  was_reported);
> +				  was_reported, was_zeroed);
>  		account_freepages(zone, nr_added, start_type);
>  		return page;
>  	}
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Dmitry Ilvokhin @ 2026-05-14 14:13 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
	Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
	Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
	linux-arch, linux-mm, linux-trace-kernel, kernel-team,
	Paul E. McKenney
In-Reply-To: <20260513114102.50f4ca68@gandalf.local.home>

On Wed, May 13, 2026 at 11:41:02AM -0400, Steven Rostedt wrote:
> On Tue,  5 May 2026 17:09:34 +0000
> Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> 
> > Use the arch-overridable queued_spin_release(), introduced in the
> > previous commit, to ensure the tracepoint works correctly across all
> 
> Remove the ", introduced in the previous commit," That's useless in git
> change logs.

Thanks for the suggestion, will do here and in other places.

[...]

> >  /**
> >   * queued_spin_unlock - unlock a queued spinlock
> >   * @lock : Pointer to queued spinlock structure
> > + *
> > + * Generic tracing wrapper around the arch-overridable
> > + * queued_spin_release().
> >   */
> >  static __always_inline void queued_spin_unlock(struct qspinlock *lock)
> >  {
> > +	/*
> > +	 * Trace and release are combined in queued_spin_release_traced() so
> > +	 * the compiler does not need to preserve the lock pointer across the
> > +	 * function call, avoiding callee-saved register save/restore on the
> > +	 * hot path.
> > +	 */
> > +	if (tracepoint_enabled(contended_release)) {
> > +		queued_spin_release_traced(lock);
> > +		return;
> 
> Get rid of the "return;". What does it save you? It just makes it that you
> need to duplicate the code. Even though it's a one liner, it can cause bugs
> in the future if this changes. You could call the function:
> 
>   do_trace_queued_spin_release_traced(lock);
> 
> 
> > +	}
> >  	queued_spin_release(lock);
> >  }
> >  
> > diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> > index af8d122bb649..649fdca69288 100644
> > --- a/kernel/locking/qspinlock.c
> > +++ b/kernel/locking/qspinlock.c
> > @@ -104,6 +104,14 @@ static __always_inline u32  __pv_wait_head_or_lock(struct qspinlock *lock,
> >  #define queued_spin_lock_slowpath	native_queued_spin_lock_slowpath
> >  #endif
> >  
> > +void __lockfunc queued_spin_release_traced(struct qspinlock *lock)
> > +{
> > +	if (queued_spin_is_contended(lock))
> > +		trace_call__contended_release(lock);
> > +	queued_spin_release(lock);
> 
> And then remove the duplicate call of "queued_spin_release()" here.

This is the scenario the comment above the static branch describes.
Here's what it looks like in practice on x86_64 (defconfig, compiled
with GCC 11).

Current design (trace + unlock combined, with return):
  
    endbr64
    xchg %ax,%ax                     ; NOP (static branch)
    movb $0x0,(%rdi)                 ; unlock
    decl %gs:__preempt_count
    je   preempt
    jmp  __x86_return_thunk
    call queued_spin_release_traced  ; cold
    jmp  preempt_handling            ; cold
    call __SCT__preempt_schedule
    jmp  __x86_return_thunk

With the trace-only function (no return, unlock after the call):
  
    endbr64
    push %rbx                        ; saves callee-saved rbx (!)
    mov  %rdi,%rbx                   ; preserve lock across call (!)
    xchg %ax,%ax                     ; NOP (static branch)
    movb $0x0,(%rbx)                 ; unlock
    decl %gs:__preempt_count
    je   preempt
    pop  %rbx                        ; callee-saved restore (!)
    jmp  __x86_return_thunk
    call queued_spin_release_traced  ; cold
    jmp  unlock                      ; cold
    call __SCT__preempt_schedule
    pop  %rbx
    jmp  __x86_return_thunk

Three extra instructions marked by "!" on the hot path (push, mov, pop),
all wasted when the tracepoint is off. That's the main reason for
combining trace and unlock in the same out-of-line function.

^ permalink raw reply

* Re: [PATCH v7 19/31] mm: page_reporting: skip redundant zeroing of host-zeroed reported pages
From: Gregory Price @ 2026-05-14 14:13 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: <b6966bac2a4bb8027ec0383ed84b497e05d5f835.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:06:48PM -0400, Michael S. Tsirkin wrote:
> When a guest reports free pages to the hypervisor via the page reporting
> framework (used by virtio-balloon and hv_balloon), the host typically
> zeros those pages when reclaiming their backing memory.  However, when
> those pages are later allocated in the guest, post_alloc_hook()
> unconditionally zeros them again if __GFP_ZERO is set.  This
> double-zeroing is wasteful, especially for large pages.
> 
> Avoid redundant zeroing:
> 
> - Add a host_zeroes_pages flag to page_reporting_dev_info, allowing
>   drivers to declare that their host zeros reported pages on reclaim.
>   A static key (page_reporting_host_zeroes) gates the fast path.
> 
> - Add PG_zeroed page flag (sharing PG_private bit) to mark pages
>   that have been zeroed by the host.  Set it in
>   page_reporting_drain() after the host reports them.
> 
> - Thread the zeroed bool through rmqueue -> prep_new_page ->
>   post_alloc_hook, where it skips redundant zeroing for __GFP_ZERO
>   allocations.
> 
> No driver sets host_zeroes_pages yet; a follow-up patch to
> virtio_balloon is needed to opt in.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
>  include/linux/page-flags.h     |  9 +++++
>  include/linux/page_reporting.h |  3 ++
>  mm/compaction.c                |  6 ++--
>  mm/internal.h                  |  2 +-
>  mm/page_alloc.c                | 66 +++++++++++++++++++++++-----------
>  mm/page_reporting.c            | 14 +++++++-
>  mm/page_reporting.h            | 12 +++++++
>  7 files changed, 87 insertions(+), 25 deletions(-)
> 

Similar question to prior comment - we're adding plumbing in this patch
specifically to handle the zeroed page flag.

Should we instead just take a snapshot of entire page flags state
and plumb that all the way through?

I imagine we might want future post-alloc ops that want to know about
buddy-internal state of the page, so it seems useful/extensible.

David and others may have a differing opinion on whether that should
wait for another user.

~Gregory

> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 0e03d816e8b9..4ee64134acc3 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -135,6 +135,8 @@ enum pageflags {
>  	PG_swapcache = PG_owner_priv_1, /* Swap page: swp_entry_t in private */
>  	/* Some filesystems */
>  	PG_checked = PG_owner_priv_1,
> +	/* Page contents are known to be zero */
> +	PG_zeroed = PG_private,
>  
>  	/*
>  	 * Depending on the way an anonymous folio can be mapped into a page
> @@ -673,6 +675,13 @@ FOLIO_TEST_CLEAR_FLAG_FALSE(young)
>  FOLIO_FLAG_FALSE(idle)
>  #endif
>  
> +/*
> + * PageZeroed() tracks pages known to be zero.  The allocator
> + * uses this to skip redundant zeroing in post_alloc_hook().
> + */
> +__PAGEFLAG(Zeroed, zeroed, PF_NO_COMPOUND)
> +#define __PG_ZEROED (1UL << PG_zeroed)
> +
>  /*
>   * PageReported() is used to track reported free pages within the Buddy
>   * allocator. We can use the non-atomic version of the test and set
> diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> index 5ab5be02fa15..c331c6b36687 100644
> --- a/include/linux/page_reporting.h
> +++ b/include/linux/page_reporting.h
> @@ -14,6 +14,9 @@ struct page_reporting_dev_info {
>  	int (*report)(struct page_reporting_dev_info *prdev,
>  		      struct scatterlist *sg, unsigned int nents);
>  
> +	/* If true, host zeros reported pages on reclaim */
> +	bool host_zeroes_pages;
> +
>  	/* work struct for processing reports */
>  	struct delayed_work work;
>  
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 4336e433c99b..8000fc5e0a2e 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -82,7 +82,8 @@ static inline bool is_via_compact_memory(int order) { return false; }
>  
>  static struct page *mark_allocated_noprof(struct page *page, unsigned int order, gfp_t gfp_flags)
>  {
> -	post_alloc_hook(page, order, __GFP_MOVABLE, USER_ADDR_NONE);
> +	__ClearPageZeroed(page);
> +	post_alloc_hook(page, order, __GFP_MOVABLE, false, USER_ADDR_NONE);
>  	set_page_refcounted(page);
>  	return page;
>  }
> @@ -1849,9 +1850,10 @@ static struct folio *compaction_alloc_noprof(struct folio *src, unsigned long da
>  		set_page_private(&freepage[size], start_order);
>  	}
>  	dst = (struct folio *)freepage;
> +	__ClearPageZeroed(&dst->page);
>  	if (order)
>  		prep_compound_page(&dst->page, order);
> -	post_alloc_hook(&dst->page, order, __GFP_MOVABLE, USER_ADDR_NONE);
> +	post_alloc_hook(&dst->page, order, __GFP_MOVABLE, false, USER_ADDR_NONE);
>  	set_page_refcounted(&dst->page);
>  	cc->nr_freepages -= 1 << order;
>  	cc->nr_migratepages -= 1 << order;
> diff --git a/mm/internal.h b/mm/internal.h
> index 389098200aa6..fd910743ddc3 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -928,7 +928,7 @@ static inline void init_compound_tail(struct page *tail,
>  }
>  
>  void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags,
> -		     unsigned long user_addr);
> +		     bool zeroed, unsigned long user_addr);
>  extern bool free_pages_prepare(struct page *page, unsigned int order);
>  
>  extern int user_min_free_kbytes;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 76f39dd026ff..bd3b909cacdf 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1743,6 +1743,7 @@ static __always_inline void page_del_and_expand(struct zone *zone,
>  	bool was_reported = page_reported(page);
>  
>  	__del_page_from_free_list(page, zone, high, migratetype);
> +
>  	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
>  	account_freepages(zone, -nr_pages, migratetype);
>  }
> @@ -1815,8 +1816,10 @@ static inline bool should_skip_init(gfp_t flags)
>  	return (flags & __GFP_SKIP_ZERO);
>  }
>  
> +
>  inline void post_alloc_hook(struct page *page, unsigned int order,
> -				gfp_t gfp_flags, unsigned long user_addr)
> +				gfp_t gfp_flags, bool zeroed,
> +				unsigned long user_addr)
>  {
>  	bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags) &&
>  			!should_skip_init(gfp_flags);
> @@ -1825,6 +1828,14 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  
>  	set_page_private(page, 0);
>  
> +	/*
> +	 * If the page is zeroed, skip memory initialization.
> +	 * We still need to handle tag zeroing separately since the host
> +	 * does not know about memory tags.
> +	 */
> +	if (zeroed && init && !zero_tags)
> +		init = false;
> +
>  	arch_alloc_page(page, order);
>  	debug_pagealloc_map_pages(page, 1 << order);
>  
> @@ -1882,13 +1893,13 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  }
>  
>  static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
> -							unsigned int alloc_flags,
> -							unsigned long user_addr)
> +			  unsigned int alloc_flags, bool zeroed,
> +			  unsigned long user_addr)
>  {
>  	if (order && (gfp_flags & __GFP_COMP))
>  		prep_compound_page(page, order);
>  
> -	post_alloc_hook(page, order, gfp_flags, user_addr);
> +	post_alloc_hook(page, order, gfp_flags, zeroed, user_addr);
>  
>  	/*
>  	 * page is set pfmemalloc when ALLOC_NO_WATERMARKS was necessary to
> @@ -3154,6 +3165,7 @@ int __isolate_free_page(struct page *page, unsigned int order)
>  	}
>  
>  	del_page_from_free_list(page, zone, order, mt);
> +	__ClearPageZeroed(page);
>  
>  	/*
>  	 * Set the pageblock if the isolated page is at least half of a
> @@ -3226,7 +3238,7 @@ static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
>  static __always_inline
>  struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zone,
>  			   unsigned int order, unsigned int alloc_flags,
> -			   int migratetype)
> +			   int migratetype, bool *zeroed)
>  {
>  	struct page *page;
>  	unsigned long flags;
> @@ -3261,6 +3273,8 @@ struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zone,
>  			}
>  		}
>  		spin_unlock_irqrestore(&zone->lock, flags);
> +		*zeroed = PageZeroed(page);
> +		__ClearPageZeroed(page);
>  	} while (check_new_pages(page, order));
>  
>  	/*
> @@ -3329,10 +3343,9 @@ static int nr_pcp_alloc(struct per_cpu_pages *pcp, struct zone *zone, int order)
>  /* Remove page from the per-cpu list, caller must protect the list */
>  static inline
>  struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
> -			int migratetype,
> -			unsigned int alloc_flags,
> +			int migratetype, unsigned int alloc_flags,
>  			struct per_cpu_pages *pcp,
> -			struct list_head *list)
> +			struct list_head *list, bool *zeroed)
>  {
>  	struct page *page;
>  
> @@ -3367,6 +3380,8 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
>  		page = list_first_entry(list, struct page, pcp_list);
>  		list_del(&page->pcp_list);
>  		pcp->count -= 1 << order;
> +		*zeroed = PageZeroed(page);
> +		__ClearPageZeroed(page);
>  	} while (check_new_pages(page, order));
>  
>  	return page;
> @@ -3375,7 +3390,8 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
>  /* Lock and remove page from the per-cpu list */
>  static struct page *rmqueue_pcplist(struct zone *preferred_zone,
>  			struct zone *zone, unsigned int order,
> -			int migratetype, unsigned int alloc_flags)
> +			int migratetype, unsigned int alloc_flags,
> +			bool *zeroed)
>  {
>  	struct per_cpu_pages *pcp;
>  	struct list_head *list;
> @@ -3393,7 +3409,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone,
>  	 */
>  	pcp->free_count >>= 1;
>  	list = &pcp->lists[order_to_pindex(migratetype, order)];
> -	page = __rmqueue_pcplist(zone, order, migratetype, alloc_flags, pcp, list);
> +	page = __rmqueue_pcplist(zone, order, migratetype, alloc_flags,
> +				 pcp, list, zeroed);
>  	pcp_spin_unlock(pcp);
>  	if (page) {
>  		__count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order);
> @@ -3418,19 +3435,19 @@ static inline
>  struct page *rmqueue(struct zone *preferred_zone,
>  			struct zone *zone, unsigned int order,
>  			gfp_t gfp_flags, unsigned int alloc_flags,
> -			int migratetype)
> +			int migratetype, bool *zeroed)
>  {
>  	struct page *page;
>  
>  	if (likely(pcp_allowed_order(order))) {
>  		page = rmqueue_pcplist(preferred_zone, zone, order,
> -				       migratetype, alloc_flags);
> +				       migratetype, alloc_flags, zeroed);
>  		if (likely(page))
>  			goto out;
>  	}
>  
>  	page = rmqueue_buddy(preferred_zone, zone, order, alloc_flags,
> -							migratetype);
> +			     migratetype, zeroed);
>  
>  out:
>  	/* Separate test+clear to avoid unnecessary atomics */
> @@ -3821,6 +3838,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  	struct pglist_data *last_pgdat = NULL;
>  	bool last_pgdat_dirty_ok = false;
>  	bool no_fallback;
> +	bool zeroed;
>  	bool skip_kswapd_nodes = nr_online_nodes > 1;
>  	bool skipped_kswapd_nodes = false;
>  
> @@ -3965,10 +3983,11 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  
>  try_this_zone:
>  		page = rmqueue(zonelist_zone(ac->preferred_zoneref), zone, order,
> -				gfp_mask, alloc_flags, ac->migratetype);
> +					gfp_mask, alloc_flags, ac->migratetype,
> +					&zeroed);
>  		if (page) {
>  			prep_new_page(page, order, gfp_mask, alloc_flags,
> -				      ac->user_addr);
> +				      zeroed, ac->user_addr);
>  
>  			return page;
>  		} else {
> @@ -4195,9 +4214,11 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>  	count_vm_event(COMPACTSTALL);
>  
>  	/* Prep a captured page if available */
> -	if (page)
> -		prep_new_page(page, order, gfp_mask, alloc_flags,
> +	if (page) {
> +		__ClearPageZeroed(page);
> +		prep_new_page(page, order, gfp_mask, alloc_flags, false,
>  			      ac->user_addr);
> +	}
>  
>  	/* Try get a page from the freelist if available */
>  	if (!page)
> @@ -5170,6 +5191,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  	/* Attempt the batch allocation */
>  	pcp_list = &pcp->lists[order_to_pindex(ac.migratetype, 0)];
>  	while (nr_populated < nr_pages) {
> +		bool zeroed = false;
>  
>  		/* Skip existing pages */
>  		if (page_array[nr_populated]) {
> @@ -5178,7 +5200,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  		}
>  
>  		page = __rmqueue_pcplist(zone, 0, ac.migratetype, alloc_flags,
> -								pcp, pcp_list);
> +					 pcp, pcp_list, &zeroed);
>  		if (unlikely(!page)) {
>  			/* Try and allocate at least one page */
>  			if (!nr_account) {
> @@ -5189,7 +5211,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  		}
>  		nr_account++;
>  
> -		prep_new_page(page, 0, gfp, 0, USER_ADDR_NONE);
> +		prep_new_page(page, 0, gfp, 0, zeroed, USER_ADDR_NONE);
>  		set_page_refcounted(page);
>  		page_array[nr_populated++] = page;
>  	}
> @@ -6929,7 +6951,8 @@ static void split_free_frozen_pages(struct list_head *list, gfp_t gfp_mask)
>  		list_for_each_entry_safe(page, next, &list[order], lru) {
>  			int i;
>  
> -			post_alloc_hook(page, order, gfp_mask, USER_ADDR_NONE);
> +			__ClearPageZeroed(page);
> +			post_alloc_hook(page, order, gfp_mask, false, USER_ADDR_NONE);
>  			if (!order)
>  				continue;
>  
> @@ -7134,8 +7157,9 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  	} else if (start == outer_start && end == outer_end && is_power_of_2(end - start)) {
>  		struct page *head = pfn_to_page(start);
>  
> +		__ClearPageZeroed(head);
>  		check_new_pages(head, order);
> -		prep_new_page(head, order, gfp_mask, 0, USER_ADDR_NONE);
> +		prep_new_page(head, order, gfp_mask, 0, false, USER_ADDR_NONE);
>  	} else {
>  		ret = -EINVAL;
>  		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index 006f7cdddc18..37e4fce9eb38 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -50,6 +50,8 @@ EXPORT_SYMBOL_GPL(page_reporting_order);
>  #define PAGE_REPORTING_DELAY	(2 * HZ)
>  static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly;
>  
> +DEFINE_STATIC_KEY_FALSE(page_reporting_host_zeroes);
> +
>  enum {
>  	PAGE_REPORTING_IDLE = 0,
>  	PAGE_REPORTING_REQUESTED,
> @@ -129,8 +131,11 @@ page_reporting_drain(struct page_reporting_dev_info *prdev,
>  		 * report on the new larger page when we make our way
>  		 * up to that higher order.
>  		 */
> -		if (PageBuddy(page) && buddy_order(page) == order)
> +		if (PageBuddy(page) && buddy_order(page) == order) {
>  			__SetPageReported(page);
> +			if (page_reporting_host_zeroes_pages())
> +				__SetPageZeroed(page);
> +		}
>  	} while ((sg = sg_next(sg)));
>  
>  	/* reinitialize scatterlist now that it is empty */
> @@ -391,6 +396,10 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
>  	/* Assign device to allow notifications */
>  	rcu_assign_pointer(pr_dev_info, prdev);
>  
> +	/* enable zeroed page optimization if host zeroes reported pages */
> +	if (prdev->host_zeroes_pages)
> +		static_branch_enable(&page_reporting_host_zeroes);
> +
>  	/* enable page reporting notification */
>  	if (!static_key_enabled(&page_reporting_enabled)) {
>  		static_branch_enable(&page_reporting_enabled);
> @@ -415,6 +424,9 @@ void page_reporting_unregister(struct page_reporting_dev_info *prdev)
>  
>  		/* Flush any existing work, and lock it out */
>  		cancel_delayed_work_sync(&prdev->work);
> +
> +		if (prdev->host_zeroes_pages)
> +			static_branch_disable(&page_reporting_host_zeroes);
>  	}
>  
>  	mutex_unlock(&page_reporting_mutex);
> diff --git a/mm/page_reporting.h b/mm/page_reporting.h
> index c51dbc228b94..736ea7b37e9e 100644
> --- a/mm/page_reporting.h
> +++ b/mm/page_reporting.h
> @@ -15,6 +15,13 @@ DECLARE_STATIC_KEY_FALSE(page_reporting_enabled);
>  extern unsigned int page_reporting_order;
>  void __page_reporting_notify(void);
>  
> +DECLARE_STATIC_KEY_FALSE(page_reporting_host_zeroes);
> +
> +static inline bool page_reporting_host_zeroes_pages(void)
> +{
> +	return static_branch_unlikely(&page_reporting_host_zeroes);
> +}
> +
>  static inline bool page_reported(struct page *page)
>  {
>  	return static_branch_unlikely(&page_reporting_enabled) &&
> @@ -46,6 +53,11 @@ static inline void page_reporting_notify_free(unsigned int order)
>  #else /* CONFIG_PAGE_REPORTING */
>  #define page_reported(_page)	false
>  
> +static inline bool page_reporting_host_zeroes_pages(void)
> +{
> +	return false;
> +}
> +
>  static inline void page_reporting_notify_free(unsigned int order)
>  {
>  }
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Michael S. Tsirkin @ 2026-05-14 14:07 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Arseniy Krasnov, Stefan Hajnoczi, kvm, Eric Dumazet,
	Eugenio Pérez, Xuan Zhuo, virtualization, David S. Miller,
	Jason Wang, linux-kernel, Maher Azzouzi
In-Reply-To: <20260514092948.268720-1-sgarzare@redhat.com>

On Thu, May 14, 2026 at 11:29:48AM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> When a large message is fragmented into multiple skbs, the zerocopy
> uarg is only allocated and attached to the last skb in the loop.
> Non-final skbs carry pinned user pages with no completion tracking,
> so the kernel has no way to notify userspace when those pages are safe
> to reuse. If the loop breaks early the uarg is never allocated at all,
> leaking pinned pages with no completion notification.
> 
> Fix this by following the approach used by TCP: allocate the zerocopy
> uarg (if not provided by the caller) before the send loop and attach
> it to every skb via skb_zcopy_set(), which takes a reference per skb.
> Each skb's completion properly decrements the refcount, and the
> notification only fires after the last skb is freed.
> On failure, if no data was sent, the uarg is cleanly aborted via
> net_zcopy_put_abort().
> 
> This issue was initially discovered by sashiko while reviewing commit
> 1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
> but was pre-existing.
> 
> Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
> Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
> Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
> Reported-by: Maher Azzouzi <maherazz04@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
>  1 file changed, 34 insertions(+), 49 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 989cc252d3d3..1e3409d28164 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
>  	return true;
>  }
>  
> -static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
> -					   struct sk_buff *skb,
> -					   struct msghdr *msg,
> -					   size_t pkt_len,
> -					   bool zerocopy)
> -{
> -	struct ubuf_info *uarg;
> -
> -	if (msg->msg_ubuf) {
> -		uarg = msg->msg_ubuf;
> -		net_zcopy_get(uarg);
> -	} else {
> -		struct ubuf_info_msgzc *uarg_zc;
> -
> -		uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> -					    pkt_len, NULL, false);
> -		if (!uarg)
> -			return -1;
> -
> -		uarg_zc = uarg_to_msgzc(uarg);
> -		uarg_zc->zerocopy = zerocopy ? 1 : 0;
> -	}
> -
> -	skb_zcopy_init(skb, uarg);
> -
> -	return 0;
> -}
> -
>  static int virtio_transport_fill_skb(struct sk_buff *skb,
>  				     struct virtio_vsock_pkt_info *info,
>  				     size_t len,
> @@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	u32 src_cid, src_port, dst_cid, dst_port;
>  	const struct virtio_transport *t_ops;
>  	struct virtio_vsock_sock *vvs;
> +	struct ubuf_info *uarg = NULL;
>  	u32 pkt_len = info->pkt_len;
>  	bool can_zcopy = false;
> +	bool have_uref = false;
>  	u32 rest_len;
>  	int ret;
>  
> @@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  		if (can_zcopy)
>  			max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
>  					    (MAX_SKB_FRAGS * PAGE_SIZE));
> +
> +		if (info->msg->msg_flags & MSG_ZEROCOPY &&
> +		    info->op == VIRTIO_VSOCK_OP_RW) {
> +			uarg = info->msg->msg_ubuf;
> +
> +			if (!uarg) {
> +				uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> +							    pkt_len, NULL, false);
> +				if (!uarg) {
> +					virtio_transport_put_credit(vvs, pkt_len);
> +					return -ENOMEM;
> +				}
> +
> +				if (!can_zcopy)
> +					uarg_to_msgzc(uarg)->zerocopy = 0;
> +
> +				have_uref = true;
> +			}
> +		}
>  	}
>  
>  	rest_len = pkt_len;
> @@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  			break;
>  		}
>  
> -		/* We process buffer part by part, allocating skb on
> -		 * each iteration. If this is last skb for this buffer
> -		 * and MSG_ZEROCOPY mode is in use - we must allocate
> -		 * completion for the current syscall.
> -		 *
> -		 * Pass pkt_len because msg iter is already consumed
> -		 * by virtio_transport_fill_skb(), so iter->count
> -		 * can not be used for RLIMIT_MEMLOCK pinned-pages
> -		 * accounting done by msg_zerocopy_realloc().
> -		 */
> -		if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
> -		    skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
> -			if (virtio_transport_init_zcopy_skb(vsk, skb,
> -							    info->msg,
> -							    pkt_len,
> -							    can_zcopy)) {
> -				kfree_skb(skb);
> -				ret = -ENOMEM;
> -				break;
> -			}
> -		}
> +		skb_zcopy_set(skb, uarg, NULL);
>  
>  		virtio_transport_inc_tx_pkt(vvs, skb);
>  
> @@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  
>  	virtio_transport_put_credit(vvs, rest_len);
>  
> +	/* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
> +	 * skb_zcopy_set() increases it for each skb, so we can drop that
> +	 * initial reference to keep it balanced.
> +	 */
> +	if (have_uref) {
> +		if (rest_len == pkt_len)
> +			/* No data sent, abort the notification. */
> +			net_zcopy_put_abort(uarg, true);
> +		else
> +			net_zcopy_put(uarg);
> +	}
> +
>  	/* Return number of bytes, if any data has been sent. */
>  	if (rest_len != pkt_len)
>  		ret = pkt_len - rest_len;
> -- 
> 2.54.0


^ permalink raw reply

* Re: [PATCH v7 18/31] mm: memfd: skip zeroing for zeroed hugetlb pool pages
From: Gregory Price @ 2026-05-14 14:07 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: <0ec51a9091b5ed3f61a49935953dd537d6c87428.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:06:43PM -0400, Michael S. Tsirkin wrote:
> gather_surplus_pages() pre-allocates hugetlb pages into the pool
> during mmap.  Pass __GFP_ZERO so these pages are zeroed by the
> buddy allocator, and HPG_zeroed is set by alloc_surplus_hugetlb_folio.
> 
> Add bool *zeroed output to alloc_hugetlb_folio_reserve() so
> callers can check whether the pool page is known-zero.  memfd's
> memfd_alloc_folio() uses this to skip the explicit folio_zero_user()
> when the page is already zero.
> 
> This avoids redundant zeroing for memfd hugetlb pages that were
> pre-allocated into the pool and never mapped to userspace.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  include/linux/hugetlb.h |  6 ++++--
>  mm/hugetlb.c            | 11 +++++++++--
>  mm/memfd.c              | 14 ++++++++------
>  3 files changed, 21 insertions(+), 10 deletions(-)
> 
... snip ...
> @@ -2221,6 +2221,12 @@ struct folio *alloc_hugetlb_folio_reserve(struct hstate *h, int preferred_nid,
>  		h->resv_huge_pages--;
>  
>  	spin_unlock_irq(&hugetlb_lock);
> +
> +	if (zeroed && folio) {
> +		*zeroed = folio_test_hugetlb_zeroed(folio);
> +		folio_clear_hugetlb_zeroed(folio);
> +	}
> +
>  	return folio;
>  }

Hmmm...

if the intent is to simply communicate to the next function up whether
the folio has been zeroed just so that function can zero it - does it
make sense instead to just zero it in this function instead?

Also the only user is memfd, and that appears to always want the folio
zeroed, so i think it simplifies this entire patch to:

struct folio* alloc_hugetlb_folio_reserve() {
... 
    if (folio && !folio_test_hugetlb_zeroed(folio))
        folio_zero_user(folio, 0);
    else if (folio)
        folio_clear_hugetlb_zeroed(folio);
    return folio;
...
}

with no API changes

~Gregory

^ permalink raw reply

* Re: [PATCH v7 15/31] mm: vma_alloc_anon_folio_pmd: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-14 13:55 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: <411a7b6a750c1904b99be92fad0e678ad791ca82.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:06:27PM -0400, Michael S. Tsirkin wrote:
> Now that vma_alloc_folio aligns the address internally, drop the
> redundant HPAGE_PMD_MASK alignment at the callsite.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Gregory Price <gourry@gourry.net>

> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 970e077019b7..d689e6491ddb 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1337,7 +1337,7 @@ static struct folio *vma_alloc_anon_folio_pmd(struct vm_area_struct *vma,
>  	const int order = HPAGE_PMD_ORDER;
>  	struct folio *folio;
>  
> -	folio = vma_alloc_folio(gfp, order, vma, addr & HPAGE_PMD_MASK);
> +	folio = vma_alloc_folio(gfp, order, vma, addr);
>  
>  	if (unlikely(!folio)) {
>  		count_vm_event(THP_FAULT_FALLBACK);
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH v7 13/31] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-14 13:54 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: <66119d3bab318d74c1586b99dd70fb74e2da1ec0.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:06:16PM -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, ...).
> 

Rather than reference other patches, just explain the contents of this
patch.  otherwise...

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh

Reviewed-by: Gregory Price <gourry@gourry.net>

> ---
>  mm/memory.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 0824441a6ba1..74523bc00d8a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4734,8 +4734,7 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
>  	/* Try allocating the highest of the remaining orders. */
>  	gfp = vma_thp_gfp_mask(vma);
>  	while (orders) {
> -		addr = ALIGN_DOWN(vmf->address, PAGE_SIZE << order);
> -		folio = vma_alloc_folio(gfp, order, vma, addr);
> +		folio = vma_alloc_folio(gfp, order, vma, vmf->address);
>  		if (folio) {
>  			if (!mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
>  							    gfp, entry))
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH v7 11/31] mm: remove arch vma_alloc_zeroed_movable_folio overrides
From: Gregory Price @ 2026-05-14 13:53 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, Magnus Lindholm,
	Greg Ungerer, Geert Uytterhoeven, Richard Henderson, Matt Turner,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	linux-alpha, linux-m68k, linux-s390
In-Reply-To: <7d637bcac4bb5705388136a75f0fdfe2881e5f20.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:06:04PM -0400, Michael S. Tsirkin wrote:
> Now that the generic vma_alloc_zeroed_movable_folio() uses
> __GFP_ZERO, the arch-specific macros on alpha, m68k, s390, and
> x86 that did the same thing are redundant.  Remove them.
> 
> arm64 is not affected: it has a real function override that
> handles MTE tag zeroing, not just __GFP_ZERO.
> 
> Suggested-by: David Hildenbrand <david@kernel.org>
> Acked-by: Magnus Lindholm <linmag7@gmail.com>
> Acked-by: Greg Ungerer <gerg@linux-m68k.org>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Gregory Price <gourry@gourry.net>


^ permalink raw reply

* Re: [PATCH v7 09/31] mm: use folio_zero_user for user pages in post_alloc_hook
From: Gregory Price @ 2026-05-14 13: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: <45d1ea85b574399459a64fdba28fcf04abfa3e7e.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:05:54PM -0400, Michael S. Tsirkin wrote:
> When post_alloc_hook() needs to zero a page for an explicit
> __GFP_ZERO allocation for a user page (user_addr is set), use folio_zero_user()
> instead of kernel_init_pages().  This zeros near the faulting
> address last, keeping those cachelines hot for the impending
> user access.
> 
> folio_zero_user() is only used for explicit __GFP_ZERO, not for
> init_on_alloc.  On architectures with virtually-indexed caches
> (e.g., ARM), clear_user_highpage() performs per-line cache
> operations; using it for init_on_alloc would add overhead that
> kernel_init_pages() avoids (the page fault path flushes the
> cache at PTE installation time regardless).
> 
> No functional change yet: current callers do not pass __GFP_ZERO
> for user pages (they zero at the callsite instead).  Subsequent
> patches will convert them.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>  mm/page_alloc.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index db387dd6b813..76f39dd026ff 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1861,9 +1861,20 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
>  		for (i = 0; i != 1 << order; ++i)
>  			page_kasan_tag_reset(page + i);
>  	}
> -	/* If memory is still not initialized, initialize it now. */
> -	if (init)
> -		kernel_init_pages(page, 1 << order);
> +	/*
> +	 * If memory is still not initialized, initialize it now.
> +	 * When __GFP_ZERO was explicitly requested and user_addr is set,
> +	 * use folio_zero_user() which zeros near the faulting address
> +	 * last, keeping those cachelines hot.  For init_on_alloc, use
> +	 * kernel_init_pages() to avoid unnecessary cache flush overhead
> +	 * on architectures with virtually-indexed caches.
> +	 */
> +	if (init) {
> +		if ((gfp_flags & __GFP_ZERO) && user_addr != USER_ADDR_NONE)
> +			folio_zero_user(page_folio(page), user_addr);
> +		else
> +			kernel_init_pages(page, 1 << order);
> +	}

Open question but not necessarily in-scope:

Should __GFP_ZERO just be implied if (user_addr != USER_ADDR_NONE)?

Putting aside how that's done without introducing another gfp flag
(maybe something explicit like `alloc_pages_nozero(...)` ), it seems
like a very short jump to just adding __GFP_ZERO to any user-alloc by
default.

I'd be curious to know how many callers across the system omit
__GFP_ZERO when allocating a user-page, and whether there might be
scenarios where we subtly miss it (seems unlikely and narrow, but very
possibly something a driver could do unintentionally).

~Gregory

^ permalink raw reply

* Re: [PATCH net v2] vsock/vmci: fix UAF when peer resets connection during handshake
From: Paolo Abeni @ 2026-05-14 13:26 UTC (permalink / raw)
  To: Minh Nguyen, Bryan Tan, Vishnu Dasa, Stefano Garzarella
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Simon Horman,
	bcm-kernel-feedback-list, netdev, virtualization, linux-kernel,
	stable
In-Reply-To: <20260512025851.189140-1-minhnguyen.080505@gmail.com>

On 5/12/26 4:58 AM, 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(-)
> 
> 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);

Sashiko says:

---
Could this change lead to a socket memory leak if another packet arrives
before vsock_pending_work() executes?
If a peer RST is received (err == 0), the socket stays on the
pending_links list with its state set to TCP_CLOSE, and the base
reference is kept.
If the peer then sends another packet (such as another RST) within the
delay window before vsock_pending_work() runs,
vmci_transport_get_pending() might find this same socket.
Since its state is TCP_CLOSE, vmci_transport_recv_listen() would hit the
default switch case, set err = -EINVAL, and call vsock_remove_pending().
This removes the socket from the list and drops the list reference, but
it bypasses vmci_transport_recv_connecting_server(), meaning the base
reference is never dropped.
When vsock_pending_work() runs later, vsock_is_pending() evaluates to false.
This sets cleanup = false and bypasses the sock_put(sk) call, leaking
the pending socket.
While not introduced by this patch, does this error path leak
sk_ack_backlog slots on failed handshakes?
If a handshake fails due to an error, vmci_transport_recv_listen()
handles it by calling vsock_remove_pending(). This removes the socket
from the pending_links list but does not call sk_acceptq_removed(sk).
When vsock_pending_work() runs later, vsock_is_pending() evaluates to
false because the socket is no longer in the list. This causes the work
function to skip its own sk_acceptq_removed(listener) call, meaning the
listener's sk_ack_backlog is never decremented.
---

it looks like the above is trading an UaF for a leak ?!?


^ permalink raw reply

* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Dmitry Ilvokhin @ 2026-05-14 12:34 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
	Dennis Zhou, Tejun Heo, Christoph Lameter, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, linux-kernel, linux-mips,
	virtualization, linux-arch, linux-mm, linux-trace-kernel,
	kernel-team, Paul E. McKenney
In-Reply-To: <20260513193342.GB2545104@noisy.programming.kicks-ass.net>

On Wed, May 13, 2026 at 09:33:42PM +0200, Peter Zijlstra wrote:
> On Tue, May 05, 2026 at 05:09:34PM +0000, Dmitry Ilvokhin wrote:
> > Use the arch-overridable queued_spin_release(), introduced in the
> > previous commit, to ensure the tracepoint works correctly across all
> > architectures, including those with custom unlock implementations (e.g.
> > x86 paravirt).
> > 
> > When the tracepoint is disabled, the only addition to the hot path is a
> > single NOP instruction (the static branch). When enabled, the contention
> > check, trace call, and unlock are combined in an out-of-line function to
> > minimize hot path impact, avoiding the compiler needing to preserve the
> > lock pointer in a callee-saved register across the trace call.
> > 
> > Binary size impact (x86_64, defconfig):
> >   uninlined unlock (common case): +680 bytes  (+0.00%)
> >   inlined unlock (worst case):    +83659 bytes (+0.21%)
> > 
> > The inlined unlock case could not be achieved through Kconfig options on
> > x86_64 as PREEMPT_BUILD unconditionally selects UNINLINE_SPIN_UNLOCK on
> > x86_64. The UNINLINE_SPIN_UNLOCK guards were manually inverted to force
> > inline the unlock path and estimate the worst case binary size increase.
> > 
> > In practice, configurations with UNINLINE_SPIN_UNLOCK=n have already
> > opted against binary size optimization, so the inlined worst case is
> > unlikely to be a concern.
> 
> This is not quite accurate. You add the (5byte) NOP for the static
> branch, but then you also add another 5 bytes for the CALL and at least
> another 2 bytes (possibly 5) for a JMP back into the previous stream.
> That is 12-15 bytes added to what was a single MOV instruction.
> 
> That is quite ludicrous.

Thanks for the feedback, Peter. This is exactly the kind of feedback I
was looking for.

I understand your concerns and initially I had exactly the same
thoughts, and after I looked into the generated code more carefully the
impact on the executed path is smaller than the total size increase
suggests.

Generated code of _raw_spin_unlock() for baseline (before the patch) is
31 bytes in total (x86_64, defconfig, GCC 11).

    3e0:  endbr64                          ; 4 bytes
    3e4:  movb $0x0,(%rdi)                 ; 3 bytes (unlock)
    3e7:  decl %gs:__preempt_count         ; 7 bytes
    3ee:  je   3f5                         ; 2 bytes
    3f0:  jmp  __x86_return_thunk          ; 5 bytes
    3f5:  call __SCT__preempt_schedule     ; 5 bytes
    3fa:  jmp  __x86_return_thunk          ; 5 bytes

Generated code of _raw_spin_unlock() with tracepoint (after the patch
applied) is 40 bytes in total.

    bc0:  endbr64                          ; 4 bytes
    bc4:  xchg %ax,%ax                     ; 2 bytes (NOP, static branch)
    bc6:  movb $0x0,(%rdi)                 ; 3 bytes (unlock)
    bc9:  decl %gs:__preempt_count         ; 7 bytes
    bd0:  je   bde                         ; 2 bytes
    bd2:  jmp  __x86_return_thunk          ; 5 bytes
    bd7:  call queued_spin_release_traced  ; 5 bytes
    bdc:  jmp  bc9                         ; 2 bytes
    bde:  call __SCT__preempt_schedule     ; 5 bytes
    be3:  jmp  __x86_return_thunk          ; 5 bytes

It is 40 bytes (+9 bytes compared to baseline, 2 bytes for NOP and 7
bytes for CALL and JMP).

But if we look at the executed path the picture is a bit different.

Baseline, in best case scenario of least number of executed
instructions.

    3e0:  endbr64                          ; 4 bytes (always executed)
    3e4:  movb $0x0,(%rdi)                 ; 3 bytes (unlock,
                                           ; always executed)
    3e7:  decl %gs:__preempt_count         ; 7 bytes (always executed)
    3ee:  je   3f5                         ; 2 bytes (always executed)
    3f0:  jmp  __x86_return_thunk          ; 5 bytes (executed if above
                                           ; je is not taken)
                                           ; rest is not executed
    3f5:  call __SCT__preempt_schedule     ; 5 bytes
    3fa:  jmp  __x86_return_thunk          ; 5 bytes

Tracepoint (again same case of least number of executed instructions).

    bc0:  endbr64                          ; 4 bytes (always executed)
    bc4:  xchg %ax,%ax                     ; 2 bytes (always executed, this is an
                                           ; only addition on the execution path).
    bc6:  movb $0x0,(%rdi)                 ; 3 bytes (unlock, always executed)
    bc9:  decl %gs:__preempt_count         ; 7 bytes (always executed)
    bd0:  je   bde                         ; 2 bytes (always executed)
    bd2:  jmp  __x86_return_thunk          ; 5 bytes (executed if above
                                           ; je is not taken)
                                           ; rest is not executed
    bd7:  call queued_spin_release_traced  ; 5 bytes
    bdc:  jmp  bc9                         ; 2 bytes
    bde:  call __SCT__preempt_schedule     ; 5 bytes
    be3:  jmp  __x86_return_thunk          ; 5 bytes

On the execution path we are getting 21 byte worth of instructions on
baseline against 23 bytes. The only addition on any executed path is the
2-byte NOP, that has a special treatment in CPU, cheap, but not entirely
free.

From a total size perspective it's 9 bytes, but on the executed path it's
a single 2-byte NOP.

Does this change the picture for you, or is the NOP still a concern for
this path?

> 
> I disagree that UNINLINE_SPIN_UNLOCK=n opts against binary size. For x86
> the unlock is smaller than a function call.
> 

Fair point on the UNINLINE_SPIN_UNLOCK characterization, but
UNINLINE_SPIN_UNLOCKis always "y" on x86_64. The inlined case only
applies to s390 (unconditionally), csky and loongarch (when
!PREEMPTION). I'll remove this, thanks.

> 
> I really don't see how this is worth it.

^ permalink raw reply

* Re: [PATCH v7 04/31] mm: hugetlb: remove dead alloc_hugetlb_folio stub
From: Gregory Price @ 2026-05-14 11:53 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: <0143c5e2086fccb05703500c9269a899d4784768.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:05:26PM -0400, Michael S. Tsirkin wrote:
> Remove the !CONFIG_HUGETLB_PAGE stub for alloc_hugetlb_folio().
> 
> The stub is dead code: all callers are in mm/hugetlb.c
> (CONFIG_HUGETLB_PAGE) or fs/hugetlbfs/inode.c (CONFIG_HUGETLBFS),
> and CONFIG_HUGETLB_PAGE is def_bool HUGETLBFS with nothing
> selecting it independently.
> 
> The stub is also broken: it returns NULL, but all callers check
> IS_ERR(folio), so a NULL return would not be caught and would
> crash on the subsequent folio dereference.
> 
> Remove it now since follow-up patches change the signature of
> alloc_hugetlb_folio and would otherwise need to update the
> broken stub too.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6

Reviewed-by: Gregory Price <gourry@gourry.net>


^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Gregory Price @ 2026-05-14 11:51 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: <feab82b370b3e66b58b9cfbc1efaf26fe6061d2f.1778616612.git.mst@redhat.com>

On Tue, May 12, 2026 at 05:05:16PM -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.
> 
> While I was unable to quantify the performance difference, it is
> an obvious waste, even if small.
> 
> Propagate the PageReported flag to sub-pages during expand(),
> both in page_del_and_expand() and try_to_claim_block(), so
> that they are recognized as already-reported.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
... snip ...
> @@ -1731,9 +1740,10 @@ static __always_inline void page_del_and_expand(struct zone *zone,
>  						int high, int migratetype)
>  {
>  	int nr_pages = 1 << high;
> +	bool was_reported = page_reported(page);
>  
>  	__del_page_from_free_list(page, zone, high, migratetype);
> -	nr_pages -= expand(zone, page, low, high, migratetype);
> +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
>  	account_freepages(zone, -nr_pages, migratetype);
>  }
>  

Maybe mildly out of scope but worth asking:  Are there other flags that
should be retained/propogated on a split?  If so, rather than pass
was_reported, should we just take a temporary copy of the page flags and
pass them all in?

~Gregory

^ permalink raw reply

* [PATCH] drm/qxl: validate dst_offset in apply_reloc against BO size
From: Berkant Koc @ 2026-05-14 10:07 UTC (permalink / raw)
  To: Dave Airlie, Gerd Hoffmann
  Cc: virtualization, spice-devel, dri-devel, linux-kernel

The QXL DRM relocation handlers apply_reloc() and apply_surf_reloc() in
drivers/gpu/drm/qxl/qxl_ioctl.c perform user-controlled writes at
reloc_page + (info->dst_offset & ~PAGE_MASK), where dst_offset comes
verbatim from the drm_qxl_reloc ioctl argument. An aspirational comment
above apply_reloc explicitly states "dst must be validated, i.e. whole
bo on vram/surfacesram", but the validation has never been implemented
since the driver was merged in v3.10 (2013).

qxl_process_single_command() copies the user drm_qxl_reloc array into
qxl_reloc_info, narrowing dst_offset from __u64 to uint32_t. This still
gives the caller (any DRM_AUTH client) the full 32-bit / 4 GiB range and
no bounds check against dst_bo->tbo.base.size happens before the writes.

The kmap target is io_mapping_map_atomic_wc() over the QXL device's full
VRAM aperture (vram_mapping). The BUG_ON inside io-mapping.h only fires
when the offset exceeds the entire aperture, not the specific BO, so
writes within the aperture but outside the resolved BO succeed and land
on whatever data occupies those pages -- including other guest processes'
QXL BOs, queued QXL release-command BOs, and ring buffers that QEMU
consumes.

The kernel-side primitive is a guest-local OOB write (CWE-787) against
guest VRAM, not a host escape. Corrupting QXL command structures that
QEMU/SPICE then parses moves the attack surface to QEMU's QXL parser
(separate codebase, historically vulnerable), so this is a viable
priming primitive for guest-to-host chains whose terminal step lives in
userspace QEMU. The kernel bug stands as a defense-in-depth concern
independent of any QEMU chain.

Public discussion of this issue is at:
https://lore.kernel.org/virtualization/36acd33982bfdce04090e17294596ff8@berkoc.com/T/#u

Add an explicit bound check in the per-reloc fill loop in
qxl_process_single_command() so that both apply_reloc (8-byte write) and
apply_surf_reloc (4-byte write) refuse out-of-BO offsets and the
arithmetic overflow case (dst_offset close to U32_MAX) with -EINVAL.

Signed-off-by: Berkant Koc <me@berkoc.com>
---
 drivers/gpu/drm/qxl/qxl_ioctl.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026..c65d468 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -231,6 +231,26 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
 		}
 
+		/*
+		 * dst must be validated, i.e. whole bo on vram/surfacesram.
+		 * Refuse offsets that would write past the end of the bo, or
+		 * where dst_offset + write_size would overflow uint32_t.
+		 */
+		if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
+		    (reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t) ||
+		     reloc_info[i].dst_offset + sizeof(uint64_t) >
+		     reloc_info[i].dst_bo->tbo.base.size)) {
+			ret = -EINVAL;
+			goto out_free_bos;
+		}
+		if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
+		    (reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t) ||
+		     reloc_info[i].dst_offset + sizeof(uint32_t) >
+		     reloc_info[i].dst_bo->tbo.base.size)) {
+			ret = -EINVAL;
+			goto out_free_bos;
+		}
+
 		/* reserve and validate the reloc dst bo */
 		if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
 			ret = qxlhw_handle_to_bo(file_priv, reloc.src_handle, release,
-- 
2.43.0


^ permalink raw reply related

* [PATCH net] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Stefano Garzarella @ 2026-05-14  9:29 UTC (permalink / raw)
  To: netdev
  Cc: Jakub Kicinski, Paolo Abeni, Simon Horman, Arseniy Krasnov,
	Stefan Hajnoczi, Stefano Garzarella, kvm, Eric Dumazet,
	Eugenio Pérez, Michael S. Tsirkin, Xuan Zhuo, virtualization,
	David S. Miller, Jason Wang, linux-kernel, Maher Azzouzi

From: Stefano Garzarella <sgarzare@redhat.com>

When a large message is fragmented into multiple skbs, the zerocopy
uarg is only allocated and attached to the last skb in the loop.
Non-final skbs carry pinned user pages with no completion tracking,
so the kernel has no way to notify userspace when those pages are safe
to reuse. If the loop breaks early the uarg is never allocated at all,
leaking pinned pages with no completion notification.

Fix this by following the approach used by TCP: allocate the zerocopy
uarg (if not provided by the caller) before the send loop and attach
it to every skb via skb_zcopy_set(), which takes a reference per skb.
Each skb's completion properly decrements the refcount, and the
notification only fires after the last skb is freed.
On failure, if no data was sent, the uarg is cleanly aborted via
net_zcopy_put_abort().

This issue was initially discovered by sashiko while reviewing commit
1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
but was pre-existing.

Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
Cc: Arseniy Krasnov <avkrasnov@salutedevices.com>
Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
Reported-by: Maher Azzouzi <maherazz04@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 83 ++++++++++---------------
 1 file changed, 34 insertions(+), 49 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 989cc252d3d3..1e3409d28164 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -70,34 +70,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
 	return true;
 }
 
-static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
-					   struct sk_buff *skb,
-					   struct msghdr *msg,
-					   size_t pkt_len,
-					   bool zerocopy)
-{
-	struct ubuf_info *uarg;
-
-	if (msg->msg_ubuf) {
-		uarg = msg->msg_ubuf;
-		net_zcopy_get(uarg);
-	} else {
-		struct ubuf_info_msgzc *uarg_zc;
-
-		uarg = msg_zerocopy_realloc(sk_vsock(vsk),
-					    pkt_len, NULL, false);
-		if (!uarg)
-			return -1;
-
-		uarg_zc = uarg_to_msgzc(uarg);
-		uarg_zc->zerocopy = zerocopy ? 1 : 0;
-	}
-
-	skb_zcopy_init(skb, uarg);
-
-	return 0;
-}
-
 static int virtio_transport_fill_skb(struct sk_buff *skb,
 				     struct virtio_vsock_pkt_info *info,
 				     size_t len,
@@ -317,8 +289,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 	u32 src_cid, src_port, dst_cid, dst_port;
 	const struct virtio_transport *t_ops;
 	struct virtio_vsock_sock *vvs;
+	struct ubuf_info *uarg = NULL;
 	u32 pkt_len = info->pkt_len;
 	bool can_zcopy = false;
+	bool have_uref = false;
 	u32 rest_len;
 	int ret;
 
@@ -360,6 +334,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 		if (can_zcopy)
 			max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
 					    (MAX_SKB_FRAGS * PAGE_SIZE));
+
+		if (info->msg->msg_flags & MSG_ZEROCOPY &&
+		    info->op == VIRTIO_VSOCK_OP_RW) {
+			uarg = info->msg->msg_ubuf;
+
+			if (!uarg) {
+				uarg = msg_zerocopy_realloc(sk_vsock(vsk),
+							    pkt_len, NULL, false);
+				if (!uarg) {
+					virtio_transport_put_credit(vvs, pkt_len);
+					return -ENOMEM;
+				}
+
+				if (!can_zcopy)
+					uarg_to_msgzc(uarg)->zerocopy = 0;
+
+				have_uref = true;
+			}
+		}
 	}
 
 	rest_len = pkt_len;
@@ -378,27 +371,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 			break;
 		}
 
-		/* We process buffer part by part, allocating skb on
-		 * each iteration. If this is last skb for this buffer
-		 * and MSG_ZEROCOPY mode is in use - we must allocate
-		 * completion for the current syscall.
-		 *
-		 * Pass pkt_len because msg iter is already consumed
-		 * by virtio_transport_fill_skb(), so iter->count
-		 * can not be used for RLIMIT_MEMLOCK pinned-pages
-		 * accounting done by msg_zerocopy_realloc().
-		 */
-		if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
-		    skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
-			if (virtio_transport_init_zcopy_skb(vsk, skb,
-							    info->msg,
-							    pkt_len,
-							    can_zcopy)) {
-				kfree_skb(skb);
-				ret = -ENOMEM;
-				break;
-			}
-		}
+		skb_zcopy_set(skb, uarg, NULL);
 
 		virtio_transport_inc_tx_pkt(vvs, skb);
 
@@ -422,6 +395,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 
 	virtio_transport_put_credit(vvs, rest_len);
 
+	/* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
+	 * skb_zcopy_set() increases it for each skb, so we can drop that
+	 * initial reference to keep it balanced.
+	 */
+	if (have_uref) {
+		if (rest_len == pkt_len)
+			/* No data sent, abort the notification. */
+			net_zcopy_put_abort(uarg, true);
+		else
+			net_zcopy_put(uarg);
+	}
+
 	/* Return number of bytes, if any data has been sent. */
 	if (rest_len != pkt_len)
 		ret = pkt_len - rest_len;
-- 
2.54.0


^ permalink raw reply related

* Re: [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size
From: Willy Tarreau @ 2026-05-14  4:57 UTC (permalink / raw)
  To: me; +Cc: security, virtualization, berkantkoc
In-Reply-To: <36acd33982bfdce04090e17294596ff8@berkoc.com>

Hello,

On Thu, May 14, 2026 at 02:07:47AM +0200, me@berkoc.com wrote:
> # QXL DRM: `apply_reloc` and `apply_surf_reloc` write to `info->dst_offset` without validating against BO size
> 
> ## Summary
> 
> The QXL DRM driver's relocation processing in `drivers/gpu/drm/qxl/qxl_ioctl.c` writes 64-bit and 32-bit values to `reloc_page + (info->dst_offset & ~PAGE_MASK)` after mapping `info->dst_offset & PAGE_MASK` into kernel space via `qxl_bo_kmap_atomic_page`. The `dst_offset` is taken verbatim from the user-supplied `drm_qxl_reloc` ioctl argument and is never validated against the destination buffer object's size before the write.
> 
> An aspirational comment at the top of `apply_reloc` explicitly notes that "`dst` must be validated, i.e. whole bo on vram/surfacesram", but the validation has not been implemented since the driver was introduced in 2013.
> 
> ## Impact
> 
> This is **not** a host-VM-escape primitive (the writes hit guest-controlled VRAM emulated by QEMU, not host kernel memory). It is a **guest-side memory corruption** primitive:
> 
> - A guest userspace process with `DRM_AUTH` (any logged-in user) can write attacker-chosen 64-bit/32-bit values to arbitrary offsets within the QXL VRAM aperture.
> - Targets reachable within the aperture include other guest processes' QXL buffer objects, QXL release-command BOs queued for the host, and ring buffers that QEMU consumes.
> - Corrupting QXL command structures that QEMU/SPICE then parses **moves the attack surface** to QEMU's QXL parser (separate codebase; historically vulnerable). This kernel bug is therefore a viable priming primitive for a guest-to-host escape chain whose terminal step lives in QEMU.
> 
> CWE-787 (Out-of-bounds Write). Realistic CVSS 3.1: `AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H` ≈ **5.5 Medium** for the kernel-side bug in isolation. If chained with a QEMU QXL parser bug: higher.
> 
> This is submitted as **hardening / defense-in-depth**, not as a self-contained RCE/VM-escape claim.
> 
> ## Affected versions
> 
> Linux mainline HEAD (audited 2026-05-13). The pattern is unchanged since the QXL DRM driver merged in v3.10 (2013). Commit history is sparse on this file; the issue is structural.
> 
> ## Vulnerable code
> 
> `drivers/gpu/drm/qxl/qxl_ioctl.c:73-110`:
> 
> ```c
> struct qxl_reloc_info {
>     int type;
>     struct qxl_bo *dst_bo;
>     uint32_t dst_offset;          /* <-- user-controlled, copied from drm_qxl_reloc.dst_offset (__u64) */
>     struct qxl_bo *src_bo;
>     int src_offset;
> };
> 
> /*
>  * dst must be validated, i.e. whole bo on vram/surfacesram (right now all bo's
>  * are on vram).
>  * *(dst + dst_off) = qxl_bo_physical_address(src, src_off)
>  */
> static void
> apply_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
> {
>     void *reloc_page;
> 
>     reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo,
>                                          info->dst_offset & PAGE_MASK);
>     *(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) =
>         qxl_bo_physical_address(qdev, info->src_bo, info->src_offset);
>     qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page);
> }
> ```
> 
> `apply_surf_reloc` (Lines 98-110) has the identical pattern for a `uint32_t` write.
> 
> ### How `dst_offset` reaches `apply_reloc`
> 
> The user-space struct is defined in `include/uapi/drm/qxl_drm.h`:
> 
> ```c
> struct drm_qxl_reloc {
>     __u64 src_offset;
>     __u64 dst_offset;        /* user-controlled */
>     __u32 src_handle;
>     __u32 dst_handle;
>     __u32 reloc_type;
>     __u32 pad;
> };
> ```
> 
> `qxl_process_single_command` (`qxl_ioctl.c:141-269`) copies the user `drm_qxl_reloc` array into `reloc_info`, narrows `dst_offset` from `__u64` to `uint32_t` (silent truncation -- still gives the attacker the full 32-bit / 4 GB range), and then calls `apply_reloc`/`apply_surf_reloc`. No bounds check against `dst_bo->tbo.base.size` exists anywhere in the call chain. `qxl_release_reserve_list` only handles TTM placement validation.
> 
> ### Mapping behaviour
> 
> `qxl_bo_kmap_atomic_page` (`drivers/gpu/drm/qxl/qxl_object.c`) computes `offset = (bo->tbo.resource->start << PAGE_SHIFT) + page_offset` and maps via `io_mapping_map_atomic_wc` on the QXL device's full VRAM aperture (`vram_mapping`). The `BUG_ON(offset >= mapping->size)` inside `io_mapping_map_atomic_wc` (see `include/linux/io-mapping.h`) only fires if the offset exceeds the *entire* aperture, not the specific BO. Writes within the aperture but outside the BO succeed and land on whatever data occupies those pages.
> 
> ## Proposed fix
> 
> Add the validation the comment promised, in the per-reloc-info-fill loop in `qxl_process_single_command` (around line 228-244):
> 
> ```c
> +   if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
> +       (reloc_info[i].dst_offset + sizeof(uint64_t) > reloc_info[i].dst_bo->tbo.base.size ||
> +        reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t))) {
> +       ret = -EINVAL;
> +       goto out_free_bos;
> +   }
> +   if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
> +       (reloc_info[i].dst_offset + sizeof(uint32_t) > reloc_info[i].dst_bo->tbo.base.size ||
> +        reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t))) {
> +       ret = -EINVAL;
> +       goto out_free_bos;
> +   }
> ```

Please turn this fix into a regular patch that can be applied. If it's
accepted by maintainers, it will save them some time and will have you
credited for finding and fixing this bug.

> Equivalent to checking `dst_offset` plus the write size fits inside the resolved BO. Also enforce a reasonable upper bound on `cmd->relocs_num` to prevent allocation DoS.
> 
> ## Discovery
> 
> Static analysis of QXL DRM ioctl handlers.
> 
> ## Disclosure
> 
> Per Linux kernel security policy, report to `security@kernel.org` and the QXL maintainers (Gerd Hoffmann, virtualization@lists.linux.dev).

Thanks for your report. However, this report is already public via the
virtualization list, so no need to involve security@, and it can be
dropped from future responses:

   https://lore.kernel.org/virtualization/36acd33982bfdce04090e17294596ff8@berkoc.com/T/#u

Willy

^ permalink raw reply

* Re: [PATCH net-next v12 0/4] tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops
From: patchwork-bot+netdevbpf @ 2026-05-14  1:10 UTC (permalink / raw)
  To: Simon Schippers
  Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
	kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
	netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260510151529.43895-1-simon.schippers@tu-dortmund.de>

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 10 May 2026 17:15:25 +0200 you wrote:
> This patch series deals with tun/tap & vhost-net which drop incoming
> SKBs whenever their internal ptr_ring buffer is full. Instead, with this
> patch series, the associated netdev queue is stopped - but only when a
> qdisc is attached. If no qdisc is present the existing behavior is
> preserved. The XDP transmit path is not affected. This patch series
> touches tun/tap and vhost-net, as they share common logic and must be
> updated together. Modifying only one of them would break the other.
> 
> [...]

Here is the summary with links:
  - [net-next,v12,1/4] tun/tap: add ptr_ring consume helper with netdev queue wakeup
    https://git.kernel.org/netdev/net-next/c/d4c22d70d725
  - [net-next,v12,2/4] vhost-net: wake queue of tun/tap after ptr_ring consume
    https://git.kernel.org/netdev/net-next/c/baf808fe4fcd
  - [net-next,v12,3/4] ptr_ring: move free-space check into separate helper
    https://git.kernel.org/netdev/net-next/c/fba362c17d9d
  - [net-next,v12,4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
    https://git.kernel.org/netdev/net-next/c/1d6e569b7d0c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size
From: me @ 2026-05-14  0:07 UTC (permalink / raw)
  To: security; +Cc: virtualization, berkantkoc

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5425 bytes --]

# QXL DRM: `apply_reloc` and `apply_surf_reloc` write to `info->dst_offset` without validating against BO size

## Summary

The QXL DRM driver's relocation processing in `drivers/gpu/drm/qxl/qxl_ioctl.c` writes 64-bit and 32-bit values to `reloc_page + (info->dst_offset & ~PAGE_MASK)` after mapping `info->dst_offset & PAGE_MASK` into kernel space via `qxl_bo_kmap_atomic_page`. The `dst_offset` is taken verbatim from the user-supplied `drm_qxl_reloc` ioctl argument and is never validated against the destination buffer object's size before the write.

An aspirational comment at the top of `apply_reloc` explicitly notes that "`dst` must be validated, i.e. whole bo on vram/surfacesram", but the validation has not been implemented since the driver was introduced in 2013.

## Impact

This is **not** a host-VM-escape primitive (the writes hit guest-controlled VRAM emulated by QEMU, not host kernel memory). It is a **guest-side memory corruption** primitive:

- A guest userspace process with `DRM_AUTH` (any logged-in user) can write attacker-chosen 64-bit/32-bit values to arbitrary offsets within the QXL VRAM aperture.
- Targets reachable within the aperture include other guest processes' QXL buffer objects, QXL release-command BOs queued for the host, and ring buffers that QEMU consumes.
- Corrupting QXL command structures that QEMU/SPICE then parses **moves the attack surface** to QEMU's QXL parser (separate codebase; historically vulnerable). This kernel bug is therefore a viable priming primitive for a guest-to-host escape chain whose terminal step lives in QEMU.

CWE-787 (Out-of-bounds Write). Realistic CVSS 3.1: `AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H` ≈ **5.5 Medium** for the kernel-side bug in isolation. If chained with a QEMU QXL parser bug: higher.

This is submitted as **hardening / defense-in-depth**, not as a self-contained RCE/VM-escape claim.

## Affected versions

Linux mainline HEAD (audited 2026-05-13). The pattern is unchanged since the QXL DRM driver merged in v3.10 (2013). Commit history is sparse on this file; the issue is structural.

## Vulnerable code

`drivers/gpu/drm/qxl/qxl_ioctl.c:73-110`:

```c
struct qxl_reloc_info {
    int type;
    struct qxl_bo *dst_bo;
    uint32_t dst_offset;          /* <-- user-controlled, copied from drm_qxl_reloc.dst_offset (__u64) */
    struct qxl_bo *src_bo;
    int src_offset;
};

/*
 * dst must be validated, i.e. whole bo on vram/surfacesram (right now all bo's
 * are on vram).
 * *(dst + dst_off) = qxl_bo_physical_address(src, src_off)
 */
static void
apply_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
{
    void *reloc_page;

    reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo,
                                         info->dst_offset & PAGE_MASK);
    *(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) =
        qxl_bo_physical_address(qdev, info->src_bo, info->src_offset);
    qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page);
}
```

`apply_surf_reloc` (Lines 98-110) has the identical pattern for a `uint32_t` write.

### How `dst_offset` reaches `apply_reloc`

The user-space struct is defined in `include/uapi/drm/qxl_drm.h`:

```c
struct drm_qxl_reloc {
    __u64 src_offset;
    __u64 dst_offset;        /* user-controlled */
    __u32 src_handle;
    __u32 dst_handle;
    __u32 reloc_type;
    __u32 pad;
};
```

`qxl_process_single_command` (`qxl_ioctl.c:141-269`) copies the user `drm_qxl_reloc` array into `reloc_info`, narrows `dst_offset` from `__u64` to `uint32_t` (silent truncation -- still gives the attacker the full 32-bit / 4 GB range), and then calls `apply_reloc`/`apply_surf_reloc`. No bounds check against `dst_bo->tbo.base.size` exists anywhere in the call chain. `qxl_release_reserve_list` only handles TTM placement validation.

### Mapping behaviour

`qxl_bo_kmap_atomic_page` (`drivers/gpu/drm/qxl/qxl_object.c`) computes `offset = (bo->tbo.resource->start << PAGE_SHIFT) + page_offset` and maps via `io_mapping_map_atomic_wc` on the QXL device's full VRAM aperture (`vram_mapping`). The `BUG_ON(offset >= mapping->size)` inside `io_mapping_map_atomic_wc` (see `include/linux/io-mapping.h`) only fires if the offset exceeds the *entire* aperture, not the specific BO. Writes within the aperture but outside the BO succeed and land on whatever data occupies those pages.

## Proposed fix

Add the validation the comment promised, in the per-reloc-info-fill loop in `qxl_process_single_command` (around line 228-244):

```c
+   if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
+       (reloc_info[i].dst_offset + sizeof(uint64_t) > reloc_info[i].dst_bo->tbo.base.size ||
+        reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t))) {
+       ret = -EINVAL;
+       goto out_free_bos;
+   }
+   if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
+       (reloc_info[i].dst_offset + sizeof(uint32_t) > reloc_info[i].dst_bo->tbo.base.size ||
+        reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t))) {
+       ret = -EINVAL;
+       goto out_free_bos;
+   }
```

Equivalent to checking `dst_offset` plus the write size fits inside the resolved BO. Also enforce a reasonable upper bound on `cmd->relocs_num` to prevent allocation DoS.

## Discovery

Static analysis of QXL DRM ioctl handlers.

## Disclosure

Per Linux kernel security policy, report to `security@kernel.org` and the QXL maintainers (Gerd Hoffmann, virtualization@lists.linux.dev).

^ permalink raw reply

* Re: [PATCH v7 00/31] mm/virtio: skip redundant zeroing of host-zeroed pages
From: Michael S. Tsirkin @ 2026-05-13 23:29 UTC (permalink / raw)
  To: Gregory Price
  Cc: David Hildenbrand (Arm), linux-kernel, 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: <agSoDnbk5rjx9yM1@gourry-fedora-PF4VCD3F>

On Wed, May 13, 2026 at 12:34:22PM -0400, Gregory Price wrote:
> On Wed, May 13, 2026 at 09:36:06AM +0200, David Hildenbrand (Arm) wrote:
> > 
> > Please wait at least a week before sending out new versions of this size.
> > 
> > It's just a lot of noise in people's inbox, and as raised, a lot of people won't
> > manage to review this before next month.
> > 
> 
> Also I wasn't even done reviewing what you had previously sent lol.

Sorry thought you were.

> I can only spare so much time each week on upstream patch review :]


You certainly invested more than a fair share on this one, thanks a lot!

> (This is also the pot calling the kettle black, I am guilty of doing
>  this same thing so uh... lesson learned, sorry David and Andrew!)
> 
> ~Gregory

Ya I clearly overdid it. I'll wait at least a week. Thanks!

-- 
MST


^ permalink raw reply

* Re: [PATCH v2] drm/virtio: Extend blob UAPI with deferred-mapping hinting
From: Dmitry Osipenko @ 2026-05-13 22:10 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu,
	Pierre-Eric Pelloux-Prayer, Alyssa Rosenzweig
  Cc: Rob Clark, dri-devel, virtualization, linux-kernel, kernel,
	Nagapuri Ramesh
In-Reply-To: <20260501000043.2483678-1-dmitry.osipenko@collabora.com>

On 5/1/26 03:00, Dmitry Osipenko wrote:
> If userspace never maps GEM object, then BO wastes hostmem space
> because VirtIO-GPU driver maps VRAM BO at the BO's creating time.
> 
> Make mappings on-demand by adding new RESOURCE_CREATE_BLOB IOCTL/UAPI
> hinting flag telling that host mapping should be deferred until first
> mapping is made when the flag is set by userspace.
> 
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Reviewed-by: Rob Clark <robdclark@gmail.com>
> ---
> 
> Changelog:
> 
> v2: Rebased on recent kernel. Intel i915 native ctx landed to Mesa, lets to
>     proceed with merging the mapping hint as a first user of the feature.
> 
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  2 ++
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c |  1 +
>  drivers/gpu/drm/virtio/virtgpu_vram.c  | 30 +++++++++++++++++++++-----
>  include/uapi/drm/virtgpu_drm.h         |  4 ++++
>  4 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index f17660a71a3e..6f49213e23f8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -84,6 +84,7 @@ struct virtio_gpu_object_params {
>  	uint32_t blob_mem;
>  	uint32_t blob_flags;
>  	uint64_t blob_id;
> +	uint32_t blob_hints;
>  };
>  
>  struct virtio_gpu_object {
> @@ -507,6 +508,7 @@ struct sg_table *virtio_gpu_vram_map_dma_buf(struct virtio_gpu_object *bo,
>  void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
>  				   struct sg_table *sgt,
>  				   enum dma_data_direction dir);
> +void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram);
>  
>  /* virtgpu_submit.c */
>  int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index c33c057365f8..01daa72b1310 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -489,6 +489,7 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
>  	params->size = rc_blob->size;
>  	params->blob = true;
>  	params->blob_flags = rc_blob->blob_flags;
> +	params->blob_hints = rc_blob->blob_hints;
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
> index 084e80227433..4ae3cbc35dd3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vram.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
> @@ -3,6 +3,8 @@
>  
>  #include <linux/dma-mapping.h>
>  
> +static DEFINE_MUTEX(map_lock);
> +
>  static void virtio_gpu_vram_free(struct drm_gem_object *obj)
>  {
>  	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> @@ -42,6 +44,11 @@ static int virtio_gpu_vram_mmap(struct drm_gem_object *obj,
>  	if (!(bo->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
>  		return -EINVAL;
>  
> +	virtio_gpu_vram_map_deferred(vram);
> +
> +	if (vram->map_state == STATE_INITIALIZING)
> +		virtio_gpu_notify(vgdev);
> +
>  	wait_event(vgdev->resp_wq, vram->map_state != STATE_INITIALIZING);
>  	if (vram->map_state != STATE_OK)
>  		return -EINVAL;
> @@ -218,14 +225,27 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
>  
>  	virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
>  					    0);
> -	if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
> -		ret = virtio_gpu_vram_map(&vram->base);
> -		if (ret) {
> -			virtio_gpu_vram_free(obj);
> -			return ret;
> +	if (!(params->blob_hints & DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING)) {
> +		if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
> +			ret = virtio_gpu_vram_map(&vram->base);
> +			if (ret) {
> +				virtio_gpu_vram_free(obj);
> +				return ret;
> +			}
>  		}
>  	}
>  
>  	*bo_ptr = &vram->base;
>  	return 0;
>  }
> +
> +void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram)
> +{
> +	if (!(vram->base.blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
> +		return;
> +
> +	mutex_lock(&map_lock);
> +	if (!drm_mm_node_allocated(&vram->vram_node))
> +		virtio_gpu_vram_map(&vram->base);
> +	mutex_unlock(&map_lock);
> +}
> diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
> index 9debb320c34b..ba09a4ee3e77 100644
> --- a/include/uapi/drm/virtgpu_drm.h
> +++ b/include/uapi/drm/virtgpu_drm.h
> @@ -200,6 +200,10 @@ struct drm_virtgpu_resource_create_blob {
>  	__u32 cmd_size;
>  	__u64 cmd;
>  	__u64 blob_id;
> +
> +#define DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING        0x0001
> +	__u32 blob_hints;
> +	__u32 pad2;
>  };
>  
>  #define VIRTGPU_CONTEXT_PARAM_CAPSET_ID       0x0001

Applied patch to misc-next

-- 
Best regards,
Dmitry

^ permalink raw reply

* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: Dmitry Osipenko @ 2026-05-13 21:54 UTC (permalink / raw)
  To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter
  Cc: dri-devel, virtualization, linux-kernel
In-Reply-To: <82aeba42-7872-4e92-ade8-2b7b6a8a5e43@collabora.com>

On 5/14/26 00:40, Dmitry Osipenko wrote:
> Hi,
> 
> On 5/12/26 11:59, Ryosuke Yasuoka wrote:
>> 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(-)
> 
> If host stops processing commands, this is a problem on host side. Isn't it?

It may be acceptable to have wait_event_timeout() in a loop, printing
warnings about unresponsive host.

Don't think we can assume that 5 seconds is enough to say that host is
busted, unless spec says so.

There could be a driver module parameter, specifying the timeoout. This
will be acceptable as user takes responsibility for the special timeout
behaviour.

-- 
Best regards,
Dmitry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox