* Re: [PATCH net] virtio-net: fix len check in receive_big()
From: David Laight @ 2026-06-11 9:48 UTC (permalink / raw)
To: Xiang Mei
Cc: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260610221606.1091465-1-xmei5@asu.edu>
On Wed, 10 Jun 2026 15:16:06 -0700
Xiang Mei <xmei5@asu.edu> wrote:
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
>
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..afe73eda1491 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> + vi->big_packets_num_skbfrags * PAGE_SIZE;
That looks like a constant (for the vi).
Probably worth saving rather than recalculating all the time.
-- David
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] vsock: fold sk_acceptq_removed() into vsock_remove_pending()
From: Stefano Garzarella @ 2026-06-11 9:43 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <20260611085603.626387-1-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 08:56:03AM +0000, Raf Dickson wrote:
>On Thu, Jun 11, 2026 at 10:35:23AM +0200, Stefano Garzarella wrote:
>> Maybe we can keep these patches separated (which I like) if we
>> introduce the vsock_pending_to_accept() I suggested with an initial
>> patch of the series.
>
>Agreed. Will restructure as a v3 series:
> 1/3: introduce vsock_pending_to_accept() for vmci
> 2/3: fold sk_acceptq_added() into vsock_add_pending()
> 3/3: fold sk_acceptq_removed() into vsock_remove_pending()
IMO would be nice to have also a patch to fold sk_acceptq_added() in
vsock_add_pending().
The rest LGTM.
Stefano
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
From: Stefano Garzarella @ 2026-06-11 9:41 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <20260611085537.622665-1-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 08:55:37AM +0000, Raf Dickson wrote:
>On Thu, Jun 11, 2026 at 10:27:42AM +0200, Stefano Garzarella wrote:
>> Maybe adding a new function that moves a socket from the pending queue
>> to the accept queue avoiding also sock_put/sock_hold dance and
>> sk_acceptq_removed()/sk_acceptq_added().
>>
>> To be called in vmci_transport_recv_connecting_server().
>>
>> So can we move sk_acceptq_added() also in vsock_add_pending()?
>
>I like the vsock_pending_to_accept() approach, it makes the vmci
>path clean without the double-accounting issue. Will send a v3 series
>with that as patch 1, followed by folding sk_acceptq_added() into
>vsock_add_pending() and sk_acceptq_removed() into vsock_remove_pending()
>as separate patches.
Okay, but VMCI still need to call vsock_add_pending() and
sk_acceptq_added() in vmci_transport_recv_listen(), no?
>
>Any preference on the name xd? vsock_pending_to_accept() works for me.
Also for me :-)
Stefano
^ permalink raw reply
* Re: [PATCH v3] vduse: Add suspend
From: Eugenio Perez Martin @ 2026-06-11 9:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Michael S. Tsirkin, oe-kbuild, lkp, oe-kbuild-all, virtualization,
Jason Wang, Cindy Lu, Xuan Zhuo, Stefano Garzarella, linux-kernel,
Laurent Vivier, Yongji Xie, Maxime Coquelin
In-Reply-To: <aip9tNOx6WHIq22p@stanley.mountain>
On Thu, Jun 11, 2026 at 11:20 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Thu, Jun 11, 2026 at 05:03:24AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jun 11, 2026 at 10:18:51AM +0300, Dan Carpenter wrote:
> > > Hi Eugenio,
> > >
> > > kernel test robot noticed the following build warnings:
> > >
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > >
> > > url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
> > > base: next-20260609
> > > patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
> > > patch subject: [PATCH v3] vduse: Add suspend
> > > config: arm64-randconfig-r072-20260610 (https://download.01.org/0day-ci/archive/20260611/202606111115.tKKe1qCE-lkp@intel.com/config)
> > > compiler: aarch64-linux-gcc (GCC) 8.5.0
> > > smatch: v0.5.0-9185-gbcc58b9c
> > >
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Reported-by: Dan Carpenter <error27@gmail.com>
> > > | Closes: https://lore.kernel.org/r/202606111115.tKKe1qCE-lkp@intel.com/
> > >
> > > smatch warnings:
> > > drivers/vdpa/vdpa_user/vduse_dev.c:577 vduse_vq_kick() warn: inconsistent returns '&vq->kick_lock'.
> > > drivers/vdpa/vdpa_user/vduse_dev.c:1302 vduse_dev_queue_irq_work() warn: inconsistent returns '&dev->rwsem'.
> > >
> > > vim +577 drivers/vdpa/vdpa_user/vduse_dev.c
> > >
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 566 goto unlock;
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 567
> > > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
> > > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
> > > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
> > >
> > > unlock before returning?
> > >
> > > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
> > > 3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 @577 spin_unlock(&vq->kick_lock);
> > > c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
> >
> >
> > I think this is fixed by:
> >
> > commit e4a249d15eb2d4b28213bebb1eefaf2e6d99de0b (HEAD -> vhost, linux-next-vhost/linux-next, kernel.org/vhost, kernel.org/test)
> > Author: Nathan Chancellor <nathan@kernel.org>
> > Date: Wed Jun 10 12:16:49 2026 -0700
> >
> > vduse: Fix error around jumping over a __cleanup() variable
> >
> > right?
>
> These things haven't hit linux-next yet. I found the email.
> https://lore.kernel.org/all/20260610-vduse_vq_kick-fix-guard-usage-v1-1-0ce02c08006e@kernel.org/
>
> That only fixes the bug in vduse_vq_kick(), not the bug in
> vduse_dev_queue_irq_work(). I don't see a fix for that
> yet on lore but I may have missed it.
>
No, I can test & send a fast patch for that.
Thanks!
^ permalink raw reply
* Re: [PATCH v3] vduse: Add suspend
From: Eugenio Perez Martin @ 2026-06-11 9:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Dan Carpenter, oe-kbuild, lkp, oe-kbuild-all, virtualization,
Jason Wang, Cindy Lu, Xuan Zhuo, Stefano Garzarella, linux-kernel,
Laurent Vivier, Yongji Xie, Maxime Coquelin
In-Reply-To: <20260611050140-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 11:03 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jun 11, 2026 at 10:18:51AM +0300, Dan Carpenter wrote:
> > Hi Eugenio,
> >
> > kernel test robot noticed the following build warnings:
> >
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
> > base: next-20260609
> > patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
> > patch subject: [PATCH v3] vduse: Add suspend
> > config: arm64-randconfig-r072-20260610 (https://download.01.org/0day-ci/archive/20260611/202606111115.tKKe1qCE-lkp@intel.com/config)
> > compiler: aarch64-linux-gcc (GCC) 8.5.0
> > smatch: v0.5.0-9185-gbcc58b9c
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Closes: https://lore.kernel.org/r/202606111115.tKKe1qCE-lkp@intel.com/
> >
> > smatch warnings:
> > drivers/vdpa/vdpa_user/vduse_dev.c:577 vduse_vq_kick() warn: inconsistent returns '&vq->kick_lock'.
> > drivers/vdpa/vdpa_user/vduse_dev.c:1302 vduse_dev_queue_irq_work() warn: inconsistent returns '&dev->rwsem'.
> >
> > vim +577 drivers/vdpa/vdpa_user/vduse_dev.c
> >
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 566 goto unlock;
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 567
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
> >
> > unlock before returning?
> >
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
> > 3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 @577 spin_unlock(&vq->kick_lock);
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
>
>
> I think this is fixed by:
>
> commit e4a249d15eb2d4b28213bebb1eefaf2e6d99de0b (HEAD -> vhost, linux-next-vhost/linux-next, kernel.org/vhost, kernel.org/test)
> Author: Nathan Chancellor <nathan@kernel.org>
> Date: Wed Jun 10 12:16:49 2026 -0700
>
> vduse: Fix error around jumping over a __cleanup() variable
>
> right?
>
Right, that was fast indeed.
^ permalink raw reply
* Re: [PATCH v2 0/3] virtiofs: hiprio FORGET robustness and no-reply request completion
From: Miklos Szeredi @ 2026-06-11 9:29 UTC (permalink / raw)
To: Li Wang
Cc: German Maglione, Vivek Goyal, Stefan Hajnoczi, Eugenio Pérez,
virtualization, linux-fsdevel, linux-kernel
In-Reply-To: <20260402104407.11495-1-liwang@kylinos.cn>
On Thu, 2 Apr 2026 at 12:45, Li Wang <liwang@kylinos.cn> wrote:
>
> This series fixes virtiofs completion for FUSE requests that do not use a
> reply descriptor, tightens hiprio FORGET handling when virtqueue submission
> fails transiently, and prevents the hiprio worker from stalling when one
> FORGET must be dropped after an unrecoverable error.
Can someone from the virtiofs team please review this?
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v3] vduse: Add suspend
From: Dan Carpenter @ 2026-06-11 9:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: oe-kbuild, Eugenio Pérez, lkp, oe-kbuild-all, virtualization,
Jason Wang, Cindy Lu, Xuan Zhuo, Stefano Garzarella, linux-kernel,
Laurent Vivier, Yongji Xie, Maxime Coquelin
In-Reply-To: <20260611050140-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 05:03:24AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2026 at 10:18:51AM +0300, Dan Carpenter wrote:
> > Hi Eugenio,
> >
> > kernel test robot noticed the following build warnings:
> >
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
> > base: next-20260609
> > patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
> > patch subject: [PATCH v3] vduse: Add suspend
> > config: arm64-randconfig-r072-20260610 (https://download.01.org/0day-ci/archive/20260611/202606111115.tKKe1qCE-lkp@intel.com/config)
> > compiler: aarch64-linux-gcc (GCC) 8.5.0
> > smatch: v0.5.0-9185-gbcc58b9c
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Closes: https://lore.kernel.org/r/202606111115.tKKe1qCE-lkp@intel.com/
> >
> > smatch warnings:
> > drivers/vdpa/vdpa_user/vduse_dev.c:577 vduse_vq_kick() warn: inconsistent returns '&vq->kick_lock'.
> > drivers/vdpa/vdpa_user/vduse_dev.c:1302 vduse_dev_queue_irq_work() warn: inconsistent returns '&dev->rwsem'.
> >
> > vim +577 drivers/vdpa/vdpa_user/vduse_dev.c
> >
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 566 goto unlock;
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 567
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
> >
> > unlock before returning?
> >
> > 9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
> > 3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 @577 spin_unlock(&vq->kick_lock);
> > c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
>
>
> I think this is fixed by:
>
> commit e4a249d15eb2d4b28213bebb1eefaf2e6d99de0b (HEAD -> vhost, linux-next-vhost/linux-next, kernel.org/vhost, kernel.org/test)
> Author: Nathan Chancellor <nathan@kernel.org>
> Date: Wed Jun 10 12:16:49 2026 -0700
>
> vduse: Fix error around jumping over a __cleanup() variable
>
> right?
These things haven't hit linux-next yet. I found the email.
https://lore.kernel.org/all/20260610-vduse_vq_kick-fix-guard-usage-v1-1-0ce02c08006e@kernel.org/
That only fixes the bug in vduse_vq_kick(), not the bug in
vduse_dev_queue_irq_work(). I don't see a fix for that
yet on lore but I may have missed it.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Herbert Xu @ 2026-06-11 9:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michael Bommarito, Olivia Mackall, linux-crypto, Jason Wang,
Kees Cook, Christian Borntraeger, virtualization, linux-kernel,
Dan Williams, Ingo Molnar, H. Peter Anvin, torvalds, alan, tglx
In-Reply-To: <20260611050731-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 05:10:32AM -0400, Michael S. Tsirkin wrote:
>
> data_avail is under hypervisor control
>
> avail = min_t(unsigned int, vi->data_avail, sizeof(vi->data));
> if (vi->data_idx >= avail) {
> vi->data_idx = 0;
>
> and maybe this can speculate past the if?
>
> I agree, this is all speculation )
Either it is vulnerable to Spectre, or it isn't. Adding nospec
markers when you're not sure is cargo cult programming.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Michael S. Tsirkin @ 2026-06-11 9:10 UTC (permalink / raw)
To: Herbert Xu
Cc: Michael Bommarito, Olivia Mackall, linux-crypto, Jason Wang,
Kees Cook, Christian Borntraeger, virtualization, linux-kernel,
Dan Williams, Ingo Molnar, H. Peter Anvin, torvalds, alan, tglx
In-Reply-To: <aipvZhfvdtRxOQm0@gondor.apana.org.au>
On Thu, Jun 11, 2026 at 04:18:46PM +0800, Herbert Xu wrote:
> On Thu, Jun 11, 2026 at 03:58:17AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jun 11, 2026 at 03:46:58PM +0800, Herbert Xu wrote:
> > > On Thu, Jun 11, 2026 at 03:30:14AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, Jun 11, 2026 at 12:43:09PM +0800, Herbert Xu wrote:
> > > > > On Sun, May 31, 2026 at 10:22:51AM -0400, Michael Bommarito wrote:
> > > > > >
> > > > > > + size = min_t(unsigned int, size, avail - vi->data_idx);
> > > > > > + idx = array_index_nospec(vi->data_idx, sizeof(vi->data));
> > > > > > + memcpy(buf, vi->data + idx, size);
> > > >
> > > > All the "malicious device" things are confusing. Spectre things -
> > > > doubly so.
> > > >
> > > > So if an access is speculated then CPU might speculate feeding a kernel
> > > > secret into RNG. And then the speculated RNG value maybe can be also
> > > > speculatively be used by some kernel code as an index
> > > > to trigger a cache access, finally leaking the secret?
> > > >
> > > > Maybe?
> > >
> > > The way Spectre works is if you have an actual instruction using
> > > idx directly. I don't see how that translates to memcpy.
> >
> > I am not sure it has to be direct:
> >
> > if (malicious_idx > SIZE)
> > return;
> > src += malicious_idx;
>
> Wait but vi->data_idx isn't even under the hypervisor's control.
>
> It's an index maintained by our own driver. So how can it be
> malicious?
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
data_avail is under hypervisor control
avail = min_t(unsigned int, vi->data_avail, sizeof(vi->data));
if (vi->data_idx >= avail) {
vi->data_idx = 0;
and maybe this can speculate past the if?
I agree, this is all speculation )
--
MST
^ permalink raw reply
* Re: [PATCH RESEND] virtio_console: read size from config space during device init
From: Filip Hejsek @ 2026-06-11 9:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Amit Shah, Arnd Bergmann, Greg Kroah-Hartman, Rusty Russell,
virtualization, linux-kernel
In-Reply-To: <20260611044443-mutt-send-email-mst@kernel.org>
On Thu, 2026-06-11 at 05:01 -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2026 at 10:29:50AM +0200, Filip Hejsek wrote:
> > On Thu, 2026-06-11 at 03:38 -0400, Michael S. Tsirkin wrote:
> > > [...]
> > > > >
> > > > > Wait a second. Why is there this rproc test here?
> > > > > Was not in the original code and commit log says nothing about it.
> > > > >
> > > >
> > > > Previously, this code was in config_work_handler(), which was never
> > > > called for rproc_serial (it's scheduled from config_intr(), which is
> > > > the config_changed handler only for virtio_console).
> > > >
> > > > Now update_size_from_config() is called unconditionally from
> > > > virtcons_probe(), so it will be called for rproc_serial too, which
> > > > doesn't have the F_SIZE feature.
> > >
> > > So why not test it?
> >
> > The virtio_console driver implements two similar but distinct virtio
> > devices: VIRTIO_ID_CONSOLE and VIRTIO_ID_RPROC_SERIAL. Although some of
> > the implementation code is shared, the devices are different. In
> > particular, rproc_serial doesn't support multiport nor any of the tty
> > specific features. This means that the relevant feature bits are not
> > valid for this device and must not be tested.
>
>
>
> > I have to admit though that I don't quite understand what the
> > RPROC_SERIAL device is supposed to be used for. It was added by commit
> > 1b6370463e88b0c1c317de16d7b962acc1dab4f2, which describes it as "a
> > simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for
> > communicating with a remote processor in an asymmetric multi-processing
> > configuration". It seems that it was never standardized, as the virtio
> > spec only says that its ID is reserved.
> >
> > > What does "not a valid feature" mean?
> >
> > I copied the "not a valid feature" comment form other instances in the
> > same file where a feature is tested, e.g. in resize_console():
> >
> > /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
> > if (!is_rproc_serial(vdev) &&
> > virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> > hvc_resize(port->cons.hvc, port->cons.ws);
> >
> >
> > Best regards,
> > Filip Hejsek
>
> I get it, it's existing code. It still makes no sense.
>
> rproc has:
>
> static const unsigned int rproc_serial_features[] = {
> };
>
> No features.
> So testing any feature bit at all always returns 0.
virtio_has_feature() will BUG() if called with a feature that hasn't
been offered by the driver (see virtio_check_driver_offered_feature).
(Maybe that was why __virtio_test_bit was used? But that seems pretty
hacky to me.)
>
> there's no reason to special case anything.
>
> So I'm testing this, but I'm only compiling rproc, so pls holler if it seems wrong:
>
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 198b97314168..2261862d4b4c 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -340,7 +340,7 @@ static inline bool use_multiport(struct ports_device *portdev)
> */
> if (!portdev->vdev)
> return false;
> - return __virtio_test_bit(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
> + return virtio_has_feature(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
> }
>
> static DEFINE_SPINLOCK(dma_bufs_lock);
> @@ -1156,9 +1156,7 @@ static void resize_console(struct port *port)
>
> vdev = port->portdev->vdev;
>
> - /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
> - if (!is_rproc_serial(vdev) &&
> - virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> + if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> hvc_resize(port->cons.hvc, port->cons.ws);
> }
>
> @@ -1783,11 +1781,8 @@ static void update_size_from_config(struct ports_device *portdev)
> * We'll use this way of resizing only for legacy support.
> * For multiport devices, use control messages to indicate
> * console size changes so that it can be done per-port.
> - *
> - * Don't test F_SIZE at all if we're rproc: not a valid feature.
> */
> - if (is_rproc_serial(vdev) ||
> - use_multiport(portdev) ||
> + if (use_multiport(portdev) ||
> !virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> return;
>
> @@ -1994,9 +1989,7 @@ static int virtcons_probe(struct virtio_device *vdev)
> multiport = false;
> portdev->max_nr_ports = 1;
>
> - /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
> - if (!is_rproc_serial(vdev) &&
> - virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
> + if (virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
> struct virtio_console_config, max_nr_ports,
> &portdev->max_nr_ports) == 0) {
> if (portdev->max_nr_ports == 0 ||
>
>
>
>
^ permalink raw reply
* Re: [PATCH v3] vduse: Add suspend
From: Michael S. Tsirkin @ 2026-06-11 9:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: oe-kbuild, Eugenio Pérez, lkp, oe-kbuild-all, virtualization,
Jason Wang, Cindy Lu, Xuan Zhuo, Stefano Garzarella, linux-kernel,
Laurent Vivier, Yongji Xie, Maxime Coquelin
In-Reply-To: <202606111115.tKKe1qCE-lkp@intel.com>
On Thu, Jun 11, 2026 at 10:18:51AM +0300, Dan Carpenter wrote:
> Hi Eugenio,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
> base: next-20260609
> patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
> patch subject: [PATCH v3] vduse: Add suspend
> config: arm64-randconfig-r072-20260610 (https://download.01.org/0day-ci/archive/20260611/202606111115.tKKe1qCE-lkp@intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 8.5.0
> smatch: v0.5.0-9185-gbcc58b9c
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Closes: https://lore.kernel.org/r/202606111115.tKKe1qCE-lkp@intel.com/
>
> smatch warnings:
> drivers/vdpa/vdpa_user/vduse_dev.c:577 vduse_vq_kick() warn: inconsistent returns '&vq->kick_lock'.
> drivers/vdpa/vdpa_user/vduse_dev.c:1302 vduse_dev_queue_irq_work() warn: inconsistent returns '&dev->rwsem'.
>
> vim +577 drivers/vdpa/vdpa_user/vduse_dev.c
>
> c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
> c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
> c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
> c8a6153b6c59d9 Xie Yongji 2021-08-31 566 goto unlock;
> c8a6153b6c59d9 Xie Yongji 2021-08-31 567
> 9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
> 9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
> 9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
>
> unlock before returning?
>
> 9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
> c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
> 3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
> c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
> c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
> c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
> c8a6153b6c59d9 Xie Yongji 2021-08-31 @577 spin_unlock(&vq->kick_lock);
> c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
I think this is fixed by:
commit e4a249d15eb2d4b28213bebb1eefaf2e6d99de0b (HEAD -> vhost, linux-next-vhost/linux-next, kernel.org/vhost, kernel.org/test)
Author: Nathan Chancellor <nathan@kernel.org>
Date: Wed Jun 10 12:16:49 2026 -0700
vduse: Fix error around jumping over a __cleanup() variable
right?
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH RESEND] virtio_console: read size from config space during device init
From: Michael S. Tsirkin @ 2026-06-11 9:01 UTC (permalink / raw)
To: Filip Hejsek
Cc: Amit Shah, Arnd Bergmann, Greg Kroah-Hartman, Rusty Russell,
virtualization, linux-kernel
In-Reply-To: <831dca185e55f56a14c0c580ab33ce84361eb67b.camel@gmail.com>
On Thu, Jun 11, 2026 at 10:29:50AM +0200, Filip Hejsek wrote:
> On Thu, 2026-06-11 at 03:38 -0400, Michael S. Tsirkin wrote:
> > [...]
> > > >
> > > > Wait a second. Why is there this rproc test here?
> > > > Was not in the original code and commit log says nothing about it.
> > > >
> > >
> > > Previously, this code was in config_work_handler(), which was never
> > > called for rproc_serial (it's scheduled from config_intr(), which is
> > > the config_changed handler only for virtio_console).
> > >
> > > Now update_size_from_config() is called unconditionally from
> > > virtcons_probe(), so it will be called for rproc_serial too, which
> > > doesn't have the F_SIZE feature.
> >
> > So why not test it?
>
> The virtio_console driver implements two similar but distinct virtio
> devices: VIRTIO_ID_CONSOLE and VIRTIO_ID_RPROC_SERIAL. Although some of
> the implementation code is shared, the devices are different. In
> particular, rproc_serial doesn't support multiport nor any of the tty
> specific features. This means that the relevant feature bits are not
> valid for this device and must not be tested.
> I have to admit though that I don't quite understand what the
> RPROC_SERIAL device is supposed to be used for. It was added by commit
> 1b6370463e88b0c1c317de16d7b962acc1dab4f2, which describes it as "a
> simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for
> communicating with a remote processor in an asymmetric multi-processing
> configuration". It seems that it was never standardized, as the virtio
> spec only says that its ID is reserved.
>
> > What does "not a valid feature" mean?
>
> I copied the "not a valid feature" comment form other instances in the
> same file where a feature is tested, e.g. in resize_console():
>
> /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
> if (!is_rproc_serial(vdev) &&
> virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> hvc_resize(port->cons.hvc, port->cons.ws);
>
>
> Best regards,
> Filip Hejsek
I get it, it's existing code. It still makes no sense.
rproc has:
static const unsigned int rproc_serial_features[] = {
};
No features.
So testing any feature bit at all always returns 0.
there's no reason to special case anything.
So I'm testing this, but I'm only compiling rproc, so pls holler if it seems wrong:
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 198b97314168..2261862d4b4c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -340,7 +340,7 @@ static inline bool use_multiport(struct ports_device *portdev)
*/
if (!portdev->vdev)
return false;
- return __virtio_test_bit(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
+ return virtio_has_feature(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
}
static DEFINE_SPINLOCK(dma_bufs_lock);
@@ -1156,9 +1156,7 @@ static void resize_console(struct port *port)
vdev = port->portdev->vdev;
- /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
- if (!is_rproc_serial(vdev) &&
- virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
+ if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
hvc_resize(port->cons.hvc, port->cons.ws);
}
@@ -1783,11 +1781,8 @@ static void update_size_from_config(struct ports_device *portdev)
* We'll use this way of resizing only for legacy support.
* For multiport devices, use control messages to indicate
* console size changes so that it can be done per-port.
- *
- * Don't test F_SIZE at all if we're rproc: not a valid feature.
*/
- if (is_rproc_serial(vdev) ||
- use_multiport(portdev) ||
+ if (use_multiport(portdev) ||
!virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
return;
@@ -1994,9 +1989,7 @@ static int virtcons_probe(struct virtio_device *vdev)
multiport = false;
portdev->max_nr_ports = 1;
- /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
- if (!is_rproc_serial(vdev) &&
- virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+ if (virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
struct virtio_console_config, max_nr_ports,
&portdev->max_nr_ports) == 0) {
if (portdev->max_nr_ports == 0 ||
--
MST
^ permalink raw reply related
* Re: [PATCH v3] vduse: Add suspend
From: Dan Carpenter @ 2026-06-11 7:18 UTC (permalink / raw)
To: oe-kbuild, Eugenio Pérez, Michael S . Tsirkin
Cc: lkp, oe-kbuild-all, virtualization, Jason Wang, Cindy Lu,
Xuan Zhuo, Stefano Garzarella, linux-kernel, Laurent Vivier,
Yongji Xie, Eugenio Pérez, Maxime Coquelin
In-Reply-To: <20260610083452.477759-1-eperezma@redhat.com>
Hi Eugenio,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-Add-suspend/20260610-164534
base: next-20260609
patch link: https://lore.kernel.org/r/20260610083452.477759-1-eperezma%40redhat.com
patch subject: [PATCH v3] vduse: Add suspend
config: arm64-randconfig-r072-20260610 (https://download.01.org/0day-ci/archive/20260611/202606111115.tKKe1qCE-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 8.5.0
smatch: v0.5.0-9185-gbcc58b9c
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202606111115.tKKe1qCE-lkp@intel.com/
smatch warnings:
drivers/vdpa/vdpa_user/vduse_dev.c:577 vduse_vq_kick() warn: inconsistent returns '&vq->kick_lock'.
drivers/vdpa/vdpa_user/vduse_dev.c:1302 vduse_dev_queue_irq_work() warn: inconsistent returns '&dev->rwsem'.
vim +577 drivers/vdpa/vdpa_user/vduse_dev.c
c8a6153b6c59d9 Xie Yongji 2021-08-31 562 static void vduse_vq_kick(struct vduse_virtqueue *vq)
c8a6153b6c59d9 Xie Yongji 2021-08-31 563 {
c8a6153b6c59d9 Xie Yongji 2021-08-31 564 spin_lock(&vq->kick_lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^
c8a6153b6c59d9 Xie Yongji 2021-08-31 565 if (!vq->ready)
c8a6153b6c59d9 Xie Yongji 2021-08-31 566 goto unlock;
c8a6153b6c59d9 Xie Yongji 2021-08-31 567
9c4307e82fa1dc Eugenio Pérez 2026-06-10 568 guard(rwsem_read)(&vq->dev->rwsem);
9c4307e82fa1dc Eugenio Pérez 2026-06-10 569 if (vq->dev->suspended)
9c4307e82fa1dc Eugenio Pérez 2026-06-10 570 return;
unlock before returning?
9c4307e82fa1dc Eugenio Pérez 2026-06-10 571
c8a6153b6c59d9 Xie Yongji 2021-08-31 572 if (vq->kickfd)
3652117f854819 Christian Brauner 2023-11-22 573 eventfd_signal(vq->kickfd);
c8a6153b6c59d9 Xie Yongji 2021-08-31 574 else
c8a6153b6c59d9 Xie Yongji 2021-08-31 575 vq->kicked = true;
c8a6153b6c59d9 Xie Yongji 2021-08-31 576 unlock:
c8a6153b6c59d9 Xie Yongji 2021-08-31 @577 spin_unlock(&vq->kick_lock);
c8a6153b6c59d9 Xie Yongji 2021-08-31 578 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH net-next] vsock/vmci: use sk_acceptq_is_full() helper
From: Raf Dickson @ 2026-06-11 8:56 UTC (permalink / raw)
To: sgarzare
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <aipzlgTKa1OuKrwg@sgarzare-redhat>
On Thu, Jun 11, 2026 at 10:37:30AM +0200, Stefano Garzarella wrote:
> Thanks for this, what about fixing also hyperv_transport ?
nice spotting, hyperv_transport.c:326 has the same open-coded check.
Will include it in v2 of this patch.
Raf
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] vsock: fold sk_acceptq_removed() into vsock_remove_pending()
From: Raf Dickson @ 2026-06-11 8:56 UTC (permalink / raw)
To: sgarzare
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <aipyNh_qseEUxj6Y@sgarzare-redhat>
On Thu, Jun 11, 2026 at 10:35:23AM +0200, Stefano Garzarella wrote:
> Maybe we can keep these patches separated (which I like) if we
> introduce the vsock_pending_to_accept() I suggested with an initial
> patch of the series.
Agreed. Will restructure as a v3 series:
1/3: introduce vsock_pending_to_accept() for vmci
2/3: fold sk_acceptq_added() into vsock_add_pending()
3/3: fold sk_acceptq_removed() into vsock_remove_pending()
Raf
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
From: Raf Dickson @ 2026-06-11 8:55 UTC (permalink / raw)
To: sgarzare
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <aiptrtQIC5ivKjem@sgarzare-redhat>
On Thu, Jun 11, 2026 at 10:27:42AM +0200, Stefano Garzarella wrote:
> Maybe adding a new function that moves a socket from the pending queue
> to the accept queue avoiding also sock_put/sock_hold dance and
> sk_acceptq_removed()/sk_acceptq_added().
>
> To be called in vmci_transport_recv_connecting_server().
>
> So can we move sk_acceptq_added() also in vsock_add_pending()?
I like the vsock_pending_to_accept() approach, it makes the vmci
path clean without the double-accounting issue. Will send a v3 series
with that as patch 1, followed by folding sk_acceptq_added() into
vsock_add_pending() and sk_acceptq_removed() into vsock_remove_pending()
as separate patches.
Any preference on the name xd? vsock_pending_to_accept() works for me.
Raf
^ permalink raw reply
* Re: [PATCH net-next] vsock/vmci: use sk_acceptq_is_full() helper
From: Stefano Garzarella @ 2026-06-11 8:37 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <20260611023830.106259-1-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 02:38:30AM +0000, Raf Dickson wrote:
>Replace the open-coded backlog check with sk_acceptq_is_full().
>The helper uses > instead of >=, which is the correct comparison
>per commit 64a146513f8f ("[NET]: Revert incorrect accept queue
>backlog changes."), and adds READ_ONCE() for proper memory ordering.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Signed-off-by: Raf Dickson <rafdog35@gmail.com>
>---
> net/vmw_vsock/vmci_transport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks for this, what about fixing also hyperv_transport ?
Stefano
>
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 91516488a7..56503bee31 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -1010,7 +1010,7 @@ static int vmci_transport_recv_listen(struct sock *sk,
> * reset. Otherwise we create and initialize a child socket and reply
> * with a connection negotiation.
> */
>- if (sk->sk_ack_backlog >= sk->sk_max_ack_backlog) {
>+ if (sk_acceptq_is_full(sk)) {
> vmci_transport_reply_reset(pkt);
> return -ECONNREFUSED;
> }
>--
>2.54.0
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] vsock: fold sk_acceptq_removed() into vsock_remove_pending()
From: Stefano Garzarella @ 2026-06-11 8:35 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <20260611021317.69362-3-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 02:13:17AM +0000, Raf Dickson wrote:
>Callers of vsock_remove_pending() must also call sk_acceptq_removed()
>to keep sk_ack_backlog consistent. Move the call into
>vsock_remove_pending() itself to make it automatic and prevent future
>callers from forgetting it.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Raf Dickson <rafdog35@gmail.com>
>---
> net/vmw_vsock/af_vsock.c | 3 +--
> net/vmw_vsock/vmci_transport.c | 5 +----
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 73e6416ee9..b9772a0205 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -493,6 +493,7 @@ void vsock_remove_pending(struct sock *listener, struct sock *pending)
> list_del_init(&vpending->pending_links);
> sock_put(listener);
> sock_put(pending);
>+ sk_acceptq_removed(listener);
> }
I'm unsure about the ordering of patches. Now that I'm looking at this,
it's clear that 2 patches are really related and can't be split without
breaking the bisectability.
Maybe we can keep these patches separated (which I like) if we introduce
the vsock_pending_to_accept() I suggested with an initial patch of the
series (feel free to chose another name, I'm not great at naming xD).
Thanks,
Stefano
> EXPORT_SYMBOL_GPL(vsock_remove_pending);
>
>@@ -761,8 +762,6 @@ static void vsock_pending_work(struct work_struct *work)
>
> if (vsock_is_pending(sk)) {
> vsock_remove_pending(listener, sk);
>-
>- sk_acceptq_removed(listener);
> } else if (!vsk->rejected) {
> /* We are not on the pending list and accept() did not reject
> * us, so we must have been accepted by our user process. We
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 91516488a7..a417403c8d 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -980,11 +980,8 @@ static int vmci_transport_recv_listen(struct sock *sk,
> err = -EINVAL;
> }
>
>- if (err < 0) {
>+ if (err < 0)
> vsock_remove_pending(sk, pending);
>- sk_acceptq_removed(sk);
>- }
>-
> release_sock(pending);
> vmci_transport_release_pending(pending);
>
>--
>2.54.0
>
^ permalink raw reply
* Re: [PATCH RESEND] virtio_console: read size from config space during device init
From: Filip Hejsek @ 2026-06-11 8:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Amit Shah, Arnd Bergmann, Greg Kroah-Hartman, Rusty Russell,
virtualization, linux-kernel
In-Reply-To: <20260611033747-mutt-send-email-mst@kernel.org>
On Thu, 2026-06-11 at 03:38 -0400, Michael S. Tsirkin wrote:
> [...]
> > >
> > > Wait a second. Why is there this rproc test here?
> > > Was not in the original code and commit log says nothing about it.
> > >
> >
> > Previously, this code was in config_work_handler(), which was never
> > called for rproc_serial (it's scheduled from config_intr(), which is
> > the config_changed handler only for virtio_console).
> >
> > Now update_size_from_config() is called unconditionally from
> > virtcons_probe(), so it will be called for rproc_serial too, which
> > doesn't have the F_SIZE feature.
>
> So why not test it?
The virtio_console driver implements two similar but distinct virtio
devices: VIRTIO_ID_CONSOLE and VIRTIO_ID_RPROC_SERIAL. Although some of
the implementation code is shared, the devices are different. In
particular, rproc_serial doesn't support multiport nor any of the tty
specific features. This means that the relevant feature bits are not
valid for this device and must not be tested.
I have to admit though that I don't quite understand what the
RPROC_SERIAL device is supposed to be used for. It was added by commit
1b6370463e88b0c1c317de16d7b962acc1dab4f2, which describes it as "a
simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for
communicating with a remote processor in an asymmetric multi-processing
configuration". It seems that it was never standardized, as the virtio
spec only says that its ID is reserved.
> What does "not a valid feature" mean?
I copied the "not a valid feature" comment form other instances in the
same file where a feature is tested, e.g. in resize_console():
/* Don't test F_SIZE at all if we're rproc: not a valid feature! */
if (!is_rproc_serial(vdev) &&
virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
hvc_resize(port->cons.hvc, port->cons.ws);
Best regards,
Filip Hejsek
> >
^ permalink raw reply
* Re: [PATCH net-next] vsock/vmci: use sk_acceptq_is_full() helper
From: Raf Dickson @ 2026-06-11 8:27 UTC (permalink / raw)
To: leonardi
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <aippcmlliXpO0BXM@leonardi-redhat>
On Thu, Jun 11, 2026 at 07:58AM, Luigi Leonardi wrote:
> this blank line should be dropped
>
> note: according to patchwork [1] you forgot to CC some maintainers,
> please be more careful next time :)
>
> Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Thanks for the review! Fixed the blank line and will add the missing
maintainers (horms, edumazet, kuba) in v2. Will send after the 24h
window. Also tried get_maintainer.pl but it kept refusing to recognize
the sparse checkout as a kernel tree -- lesson learned to check
patchwork next time.
Raf
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] vsock: fold sk_acceptq_added() into vsock_enqueue_accept()
From: Stefano Garzarella @ 2026-06-11 8:27 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list, bobbyeshleman
In-Reply-To: <20260611021317.69362-2-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 02:13:16AM +0000, Raf Dickson wrote:
>virtio and hyperv call sk_acceptq_added() immediately before
>vsock_enqueue_accept(). Move the call into vsock_enqueue_accept()
>itself so callers cannot forget it and the accounting is consistent.
>
>vmci is left unchanged as sk_acceptq_added() there pairs with
I'm confused, vmci is also calling vsock_enqueue_accept(), are we
calling sk_acceptq_added() twice?
Aaaah, vmci is calling vsock_enqueue_accept() after
vsock_remove_pending(), so IIUC with the next patch we fix this, so
should we reverse the order of the patches?
Also, should we avoid this?
Maybe adding a new function that moves a socket from the pending queue
to the accept queue avoiding also sock_put/sock_hold dance and
sk_acceptq_removed()/sk_acceptq_added().
I mean something like this (untested):
void vsock_pending_to_accept(struct sock *listener, struct sock *pending)
{
struct vsock_sock *vpending = vsock_sk(pending);
struct vsock_sock *vlistener = vsock_sk(listener);
list_del_init(&vpending->pending_links);
list_add_tail(&vpending->accept_queue, &vlistener->accept_queue);
}
To be called in vmci_transport_recv_connecting_server().
WDYT?
>vsock_add_pending(), not vsock_enqueue_accept(), since connections
So can we move sk_acceptq_added() also in vsock_add_pending()?
(with another patch I guess)
Thanks,
Stefano
>can be cleaned up by the pending work timer before ever reaching
>vsock_enqueue_accept().
>
>Suggested-by: Paolo Abeni <pabeni@redhat.com>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Signed-off-by: Raf Dickson <rafdog35@gmail.com>
>---
> net/vmw_vsock/af_vsock.c | 1 +
> net/vmw_vsock/hyperv_transport.c | 1 -
> net/vmw_vsock/virtio_transport_common.c | 1 -
> 3 files changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 2ce1063d4a..73e6416ee9 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -507,6 +507,7 @@ void vsock_enqueue_accept(struct sock *listener, struct sock *connected)
> sock_hold(connected);
> sock_hold(listener);
> list_add_tail(&vconnected->accept_queue, &vlistener->accept_queue);
>+ sk_acceptq_added(listener);
> }
> EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index b3394946b2..0de8148877 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -410,7 +410,6 @@ static void hvs_open_connection(struct vmbus_channel *chan)
>
> if (conn_from_host) {
> new->sk_state = TCP_ESTABLISHED;
>- sk_acceptq_added(sk);
>
> hvs_new->vm_srv_id = *if_type;
> hvs_new->host_srv_id = *if_instance;
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index b10666937c..4a39d48db9 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1582,7 +1582,6 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> return ret;
> }
>
>- sk_acceptq_added(sk);
> if (virtio_transport_space_update(child, skb))
> child->sk_write_space(child);
>
>--
>2.54.0
>
^ permalink raw reply
* Re: [PATCH v5] i2c: virtio: retain xfer with kref to fix UAF on interrupted wait
From: Viresh Kumar @ 2026-06-11 8:24 UTC (permalink / raw)
To: Gavin Li
Cc: Michael S. Tsirkin, linux-i2c, Chen, Jian Jun, andi.shyti,
virtualization
In-Reply-To: <CAKvMnUXk0GrYNNYOHvME6FYRdKik-7fV5B-NgvRM+i5yYKTmvw@mail.gmail.com>
On 10-06-26, 13:34, Gavin Li wrote:
> Another option I am considering:
>
> - Uninterruptible wait with timeout, timeout based on adap->timeout
> - Upon timeout, reset the device with
> virtio_i2c_del_vqs() + virtio_i2c_setup_vqs() + virtio_device_ready()
>
> This behavior more closely mirrors what other I2C bus drivers do.
> The device should be completely quiesced when virtio_i2c_del_vqs()
> returns, avoiding the UAF.
Looks more reasonable with lesser drawbacks.
--
viresh
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Herbert Xu @ 2026-06-11 7:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michael Bommarito, Olivia Mackall, linux-crypto, Jason Wang,
Kees Cook, Christian Borntraeger, virtualization, linux-kernel,
Dan Williams, Ingo Molnar, H. Peter Anvin, torvalds, alan, tglx
In-Reply-To: <20260611025916-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 03:30:14AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2026 at 12:43:09PM +0800, Herbert Xu wrote:
> > On Sun, May 31, 2026 at 10:22:51AM -0400, Michael Bommarito wrote:
> > >
> > > + size = min_t(unsigned int, size, avail - vi->data_idx);
> > > + idx = array_index_nospec(vi->data_idx, sizeof(vi->data));
> > > + memcpy(buf, vi->data + idx, size);
>
> All the "malicious device" things are confusing. Spectre things -
> doubly so.
>
> So if an access is speculated then CPU might speculate feeding a kernel
> secret into RNG. And then the speculated RNG value maybe can be also
> speculatively be used by some kernel code as an index
> to trigger a cache access, finally leaking the secret?
>
> Maybe?
The way Spectre works is if you have an actual instruction using
idx directly. I don't see how that translates to memcpy.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Herbert Xu @ 2026-06-11 8:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Michael Bommarito, Olivia Mackall, linux-crypto, Jason Wang,
Kees Cook, Christian Borntraeger, virtualization, linux-kernel,
Dan Williams, Ingo Molnar, H. Peter Anvin, torvalds, alan, tglx
In-Reply-To: <20260611035035-mutt-send-email-mst@kernel.org>
On Thu, Jun 11, 2026 at 03:58:17AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2026 at 03:46:58PM +0800, Herbert Xu wrote:
> > On Thu, Jun 11, 2026 at 03:30:14AM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Jun 11, 2026 at 12:43:09PM +0800, Herbert Xu wrote:
> > > > On Sun, May 31, 2026 at 10:22:51AM -0400, Michael Bommarito wrote:
> > > > >
> > > > > + size = min_t(unsigned int, size, avail - vi->data_idx);
> > > > > + idx = array_index_nospec(vi->data_idx, sizeof(vi->data));
> > > > > + memcpy(buf, vi->data + idx, size);
> > >
> > > All the "malicious device" things are confusing. Spectre things -
> > > doubly so.
> > >
> > > So if an access is speculated then CPU might speculate feeding a kernel
> > > secret into RNG. And then the speculated RNG value maybe can be also
> > > speculatively be used by some kernel code as an index
> > > to trigger a cache access, finally leaking the secret?
> > >
> > > Maybe?
> >
> > The way Spectre works is if you have an actual instruction using
> > idx directly. I don't see how that translates to memcpy.
>
> I am not sure it has to be direct:
>
> if (malicious_idx > SIZE)
> return;
> src += malicious_idx;
Wait but vi->data_idx isn't even under the hypervisor's control.
It's an index maintained by our own driver. So how can it be
malicious?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next] vsock/vmci: use sk_acceptq_is_full() helper
From: Luigi Leonardi @ 2026-06-11 7:58 UTC (permalink / raw)
To: Raf Dickson
Cc: netdev, virtualization, pabeni, sgarzare, stefanha, bryan-bt.tan,
vishnu.dasa, bcm-kernel-feedback-list
In-Reply-To: <20260611023830.106259-1-rafdog35@gmail.com>
On Thu, Jun 11, 2026 at 02:38:30AM +0000, Raf Dickson wrote:
>Replace the open-coded backlog check with sk_acceptq_is_full().
>The helper uses > instead of >=, which is the correct comparison
>per commit 64a146513f8f ("[NET]: Revert incorrect accept queue
>backlog changes."), and adds READ_ONCE() for proper memory ordering.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>
this blank line should be dropped
>Signed-off-by: Raf Dickson <rafdog35@gmail.com>
>---
> net/vmw_vsock/vmci_transport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 91516488a7..56503bee31 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -1010,7 +1010,7 @@ static int vmci_transport_recv_listen(struct sock *sk,
> * reset. Otherwise we create and initialize a child socket and reply
> * with a connection negotiation.
> */
>- if (sk->sk_ack_backlog >= sk->sk_max_ack_backlog) {
>+ if (sk_acceptq_is_full(sk)) {
> vmci_transport_reply_reset(pkt);
> return -ECONNREFUSED;
> }
>--
>2.54.0
>
Thanks for the patch!
note: according to patchwork [1] you forgot to CC some maintainers,
please be more careful next time :)
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
[1] https://patchwork.kernel.org/project/netdevbpf/patch/20260611023830.106259-1-rafdog35@gmail.com/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox