* [PATCH net-next 3/3] xsk: replace sk_busy_loop with sk_tx_busy_loop in __xsk_sendmsg()
From: menglong8.dong @ 2026-06-11 7:12 UTC (permalink / raw)
To: jasowang
Cc: mst, xuanzhuo, eperezma, andrew+netdev, davem, edumazet, kuba,
pabeni, magnus.karlsson, maciej.fijalkowski, sdf, horms, ast,
daniel, hawk, john.fastabend, bjorn, kerneljasonxing, netdev,
virtualization, linux-kernel, bpf
In-Reply-To: <20260611071242.2485058-1-dongml2@chinatelecom.cn>
From: Menglong Dong <dongml2@chinatelecom.cn>
Replace sk_busy_loop with sk_tx_busy_loop to support tx napi in
__xsk_sendmsg().
Fixes: a0731952d9cd ("xsk: Add busy-poll support for {recv,send}msg()")
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
net/xdp/xsk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 5e5786cd9af5..2bf9a7313ac4 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1158,7 +1158,7 @@ static int __xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len
return -ENOBUFS;
if (sk_can_busy_loop(sk))
- sk_busy_loop(sk, 1); /* only support non-blocking sockets */
+ sk_tx_busy_loop(sk, 1); /* only support non-blocking sockets */
if (xs->zc && xsk_no_wakeup(sk))
return 0;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Dmitry Ilvokhin @ 2026-06-11 7:17 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: <20260603120811.GW3493090@noisy.programming.kicks-ass.net>
On Wed, Jun 03, 2026 at 02:08:11PM +0200, Peter Zijlstra wrote:
> Also, I think someone should go do some performance runs with
> ARCH_INLINE_SPIN_* set for x86 just like for s390.
As promised, I set ARCH_INLINE_SPIN_UNLOCK{,_BH,_IRQ,_IRQRESTORE} for
x86 and measured the effect on a few real workloads.
Short version: inlining of _raw_spin_unlock() adds measurable kernel
i-cache pressure on every workload I tried, and on a
kernel-i-cache-bound one (nginx connection churn) it costs ~1.27%
throughput. I did not find a workload where it helps.
HOW BENCHMARKS WERE CHOSEN
The cost of inlining unlock is text footprint increase. Every unlock
site grows, and the extra bytes compete for the shared L1i. The bill is
paid by unrelated code, in both kernel and userspace.
Locktorture and similar microbenchmarks can't see this, because they
usually hammer a tiny loop that stays L1i-resident, so they measure
fast-path cycles, where inlining (fewer instructions per unlock) looks
neutral-to-good.
To make the cost visible, the workload has to have real instruction
cache pressure. To achieve that, it has to touch a lot of code.
A good way to screen benchmarks: look for high tma_frontend_bound
fraction from 'perf stat -M TopdownL1' and simultaneously require it to
spend non-trivial time in the kernel (be syscall-heavy).
SETUP
Hardware: 2x Intel Xeon Gold 6138 (Skylake-SP), 20 cores/socket, 40C/80T
with kernel built from locking/core branch. Baseline _raw_spin_unlock()
is out-of-line via UNINLINE_SPIN_UNLOCK=y. Experiment adds the four
selects above (exact patch is at the end of this message). Cache
geometry (lscpu -C):
NAME ONE-SIZE ALL-SIZE WAYS TYPE LEVEL SETS PHY-LINE COHERENCY-SIZE
L1d 32K 1.3M 8 Data 1 64 1 64
L1i 32K 1.3M 8 Instruction 1 64 1 64
L2 1M 40M 16 Unified 2 1024 1 64
L3 27.5M 55M 11 Unified 3 40960 1 64
Per run I collected cycles, instructions and L1i-misses. To stay within
the available PMU counters, each run used only 3 events: cycles,
instructions and one L1i filter (:u or :k). The NMI watchdog was off and
every run reported 100% counter enablement (no multiplexing). Userspace
and kernel misses therefore come from separate runs. Each benchmark was
run 20x per side: 10 with the :u counter, 10 with :k. Cycles,
instructions and throughput are pooled across all 20, each L1i split
comes from its 10.
KERNEL IMAGE SIZE
To give a sense of the code-footprint increase, scripts/bloat-o-meter on
vmlinux, GCC 11, x86_64, defconfig + CONFIG_PARAVIRT_SPINLOCKS=y:
Total: Before=23838694, After=23977159, chg +0.58%
ROCKSDB (DELETESEQ)
db_bench -benchmarks=deleteseq
Metric Baseline Experiment Delta Sig
----------------------------------------------------------------------
Instructions (total) 9,574,476,543 9,573,602,441 -0.01% flat
L1i-miss :k (kernel) 198,588,165 216,672,536 +9.11% **
L1i-miss :u (userspace) 593,276,235 616,433,813 +3.90% **
Throughput ops/s 431,398 432,897 +0.35% ns
Cycles (total) 4,681,002,302 4,665,106,876 -0.34% ns
IPC 2.045 2.052 +0.33% ns
Time elapsed (s) 2.4012 2.3865 -0.62% ns
----------------------------------------------------------------------
L1i-miss: higher = worse. Throughput: higher = better.
** = beyond per-run noise (+-0.1..0.36%), ns = within noise.
At constant instructions, inlining raises L1i misses +9.11% (kernel) and
+3.90% (userspace), both well beyond noise. Throughput, cycles, IPC and
wall-time all stay within run-to-run noise. So the i-cache cost is real,
but at IPC ~2 db_bench isn't fetch-bound at the app level, so it doesn't
surface.
No benefit from _raw_spin_unlock() inlining.
KERNEL BUILD
Building locking/core (defconfig), GCC 11.
make -j80
Metric Baseline Experiment Delta Sig
-------------------------------------------------------------
L1i-miss :k 36.72G 37.51G +2.16% **
L1i-miss :u 246.99G 246.06G -0.38% **
Sys (s) 478.250 482.420 +0.87% **
Time elapsed (s) 105.221 105.373 +0.14% ns
User (s) 4022.046 4024.012 +0.05% flat
Cycles 8,894.10G 8,902.12G +0.09% flat
Instructions 8,424.28G 8,426.48G +0.03% flat
IPC 0.947 0.947 -0.06% flat
-------------------------------------------------------------
L1i-miss/Sys: higher = worse.
** = beyond per-run noise, ns = within noise.
Kernel i-cache misses (+2.16%) and sys time (+0.87%) both rise and are
significant. Wall-time and userspace L1i are flat. Kernel build is
GCC/userspace-bound (User 4022s vs Sys 478s), so the added kernel fetch
cost is real but appears to sit off the critical path.
No benefit from _raw_spin_unlock() inlining.
NGINX
I ran nginx with taskset -c 2.
perf stat -C 2 ... -- ab -n 100000 -c 80 http://127.0.0.1:8080/
Config for nginx was the following.
worker_processes 1;
error_log /tmp/ngx/error.log;
pid /tmp/ngx/nginx.pid;
events { worker_connections 16384; }
http {
access_log off;
server { listen 8080 reuseport; location / { return 200 "ok\n"; } }
}
I used nginx version 1.20.1 (prebuilt, from CentOS repo).
Metric Baseline Experiment Delta Sig
------------------------------------------------------------
req/s (ab) 25,113 24,795 -1.27% **
L1i MPKI :k 70.06 72.10 +2.92% **
L1i MPKI :u 20.16 20.66 +2.50% **
instructions 5.86G 5.83G -0.50% **
L1i-miss :k 0.41G 0.42G +2.44% **
L1i-miss :u 0.12G 0.12G +1.95% **
cycles 4.82G 4.81G -0.28% ns
IPC 1.215 1.213 -0.22% ns
perf time (s) 4.077 4.129 +1.26% **
failed reqs 0 0 - valid
------------------------------------------------------------
req/s: higher=better. MPKI: higher=worse.
** = beyond per-run noise, ns = within noise.
nginx connection-churn is the one workload that is genuinely
kernel-fetch-bound: MPKI:k ~70 and IPC ~1.2 (vs db_bench's 2.05). Here
the cost surfaces: req/s −1.27%. Misses rise in both domains (+2.9%
MPKI:k, +2.5% MPKI:u). Unlike kernel build, userspace is hit too,
because nginx runs user and kernel hot on the same core and the kernel
bloat pollutes the shared L1i.
And the kicker: instructions fell 0.5% (inlining removed the call/ret)
yet throughput dropped.
Caveat: ab is single-threaded, so it seems the worker core is
under-saturated: cycles is flat (−0.28%, ns) while wall-time rose
(+1.26%).
Measurable throughput regression from _raw_spin_unlock() inlining.
CONCLUSION
Inlining _raw_spin_unlock() raises kernel L1i misses on every workload.
It's an unconditional cost. Whether it costs the application throughput
depends on how kernel-fetch-bound the workload is.
The cost is real everywhere. It only surfaces as throughput regression
where the kernel is on the fetch critical path. And inlining did not
help in any workload I measured. The one micro-effect inlining produced
(-0.5% instructions on nginx) was erased by the added i-cache pressure.
From 99502328caed3c195e20cf194a1e8aa1563f3896 Mon Sep 17 00:00:00 2001
From: Dmitry Ilvokhin <d@ilvokhin.com>
Date: Thu, 4 Jun 2026 07:43:00 -0700
Subject: [PATCH] x86/locking: Inline the spin_unlock()
Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fdaef60b46d6..c9a0638225fd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -113,6 +113,10 @@ config X86
select ARCH_HAS_ZONE_DMA_SET if EXPERT
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_HAVE_EXTRA_ELF_NOTES
+ select ARCH_INLINE_SPIN_UNLOCK
+ select ARCH_INLINE_SPIN_UNLOCK_BH
+ select ARCH_INLINE_SPIN_UNLOCK_IRQ
+ select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE
select ARCH_MEMORY_ORDER_TSO
select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
--
2.53.0-Meta
^ permalink raw reply related
* Re: [PATCH net] virtio_net: do not allow tunnel csum offload for non GSO packets
From: Paolo Abeni @ 2026-06-11 7:28 UTC (permalink / raw)
To: Gabriel Goller
Cc: netdev, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, virtualization, Willem de Bruijn
In-Reply-To: <178109396600.129329.5073003425673349392.b4-review@b4>
On 6/10/26 2:19 PM, Gabriel Goller wrote:
> On Tue, 09 Jun 2026 16:44:26 +0200, Paolo Abeni <pabeni@redhat.com> wrote:
>> [...]
>>
>> Fixes: 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support.")
>> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
>> Closes: https://bugzilla.proxmox.com/show_bug.cgi?id=7627
>> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> Gave it a spin and it works alright, so consider:
>
>>
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index f4adcfee7a80..07b8710639f9 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -6222,6 +6222,18 @@ static void virtnet_free_irq_moder(struct virtnet_info *vi)
>> rtnl_unlock();
>> }
>>
>> +static netdev_features_t virtnet_features_check(struct sk_buff *skb,
>> + struct net_device *dev,
>> + netdev_features_t features)
>> +{
>> + /* Inner csum offload is only available for GSO packets. */
>> + if (skb->encapsulation && !skb_is_gso(skb))
>
> A small question -- should we maybe check for skb_gso_ok here as well?
> So add:
>
> (!skb_is_gso(skb) || !skb_gso_ok(skb, features)))
>
> Because skb_is_gso alone doesn't guarantee that the packets leaving virtio will
> be gso'd, they could be software gso'd at validate_xmit_skb, which is called
> after ndo_feature_check.
> leaving the virtio device.
Good point. Indeed disabling tx-udp_tnl-segmentation inside the guest
after successful VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO negotiation would
again break the connectivity in the critical scenarios.
Let me test a v2.
Thanks,
Paolo
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Michael S. Tsirkin @ 2026-06-11 7:30 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: <aio83ZWadVTiuNpR@gondor.apana.org.au>
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);
>
> I don't see how nospec can help here. Please enlighten me.
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?
> Thanks,
> --
> 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 splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Miaohe Lin @ 2026-06-11 7:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Zi Yan, David Hildenbrand (Arm), Andrew Morton, linux-kernel,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Muchun Song,
Oscar Salvador, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <20260611013644-mutt-send-email-mst@kernel.org>
On 2026/6/11 13:43, Michael S. Tsirkin wrote:
> On Thu, Jun 11, 2026 at 11:35:36AM +0800, Miaohe Lin wrote:
>> On 2026/6/11 5:18, Michael S. Tsirkin wrote:
>>> On Wed, Jun 10, 2026 at 03:24:30PM +0800, Miaohe Lin wrote:
>>>> On 2026/6/10 5:00, Michael S. Tsirkin wrote:
>>>>> On Tue, Jun 09, 2026 at 04:54:01PM -0400, Zi Yan wrote:
>>>>>> On 9 Jun 2026, at 16:34, Michael S. Tsirkin wrote:
>>>>>>
>>>>>>> On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
>>>>>>>> On 9 Jun 2026, at 14:39, Zi Yan wrote:
>>>>>>>>
>>>>>>>>> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
>>>>>>>>>
>>>>>>>>>> On 6/9/26 20:10, Andrew Morton wrote:
>>>>>>>>>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
>>>>>>>>>>>> update to page->flags can race with non-atomic flag operations
>>>>>>>>>>>> that run under zone->lock in the buddy allocator.
>>>>>>>>>>>>
>>>>>>>>>>>> In particular, __free_pages_prepare() does:
>>>>>>>>>>>>
>>>>>>>>>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>>>>>>>>>>
>>>>>>>>>>>> This non-atomic read-modify-write, while correctly excluding
>>>>>>>>>>>> __PG_HWPOISON from the mask, can still lose a concurrent
>>>>>>>>>>>> TestSetPageHWPoison if the read happens before the poison bit
>>>>>>>>>>>> is set and the write happens after. Will only get worse if/when
>>>>>>>>>>>> we add more non-atomic flag operations.
>>>>>>>>>>>>
>>>>>>>>>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
>>>>>>>>>>>> around ClearPageHWPoison in the retry path. This
>>>>>>>>>>>> serializes with all buddy flag manipulation. The cost is
>>>>>>>>>>>> negligible: one lock/unlock in an extremely rare path
>>>>>>>>>>>> (hardware memory errors).
>>>>>>>>>>>>
>>>>>>>>>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
>>>>>>>>>>>> in this file operate on pages already removed from the buddy
>>>>>>>>>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
>>>>>>>>>>>> need zone->lock protection.
>>>>>>>>>>>
>>>>>>>>>>> Sashiko is saying this doesn't do anything "Because
>>>>>>>>>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
>>>>>>>>>>>
>>>>>>>>>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
>>>>>>>>>>
>>>>>>>>>> Battle of the bots: it's right.
>>>>>>>>>
>>>>>>>>> Yep, __free_pages_prepare() changes the page flag without holding
>>>>>>>>> zone->lock.
>>>>>>>>
>>>>>>>> __free_pages_prepare() works on frozen pages and assumes no one else
>>>>>>>> touches the input page. To avoid this race, memory_failure() might
>>>>>>>> want to try_get_page() before TestClearPageHWPoison(), but I am not
>>>>>>>> sure if that works along with memory failure flow.
>>>>>>>>
>>>>>>>> Best Regards,
>>>>>>>> Yan, Zi
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Actually memory failure already plays with this down the road no?
>>>>>>>
>>>>>>> So maybe it's enough to just SetPageHWPoison afterwards again?
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>>>>>> index ee42d4361309..4758fea94a96 100644
>>>>>>> --- a/mm/memory-failure.c
>>>>>>> +++ b/mm/memory-failure.c
>>>>>>> @@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
>>>>>>> if (!res) {
>>>>>>> if (is_free_buddy_page(p)) {
>>>>>>> if (take_page_off_buddy(p)) {
>>>>>>> + SetPageHWPoison(p);
>>>>>>> page_ref_inc(p);
>>>>>>> res = MF_RECOVERED;
>>>>>>> } else {
>>>>>>>
>>>>>>>
>>>>>>> and maybe in a bunch of other places in there?
>>>>>>
>>>>>> You mean for fear of losing HWPoison flag in the earlier TestSetPageHWPoison(),
>>>>>> just set it again here?
>>>>>
>>>>> Yea.
>>>>>
>>>>>> Why not do it after get_hwpoison_page(), since that
>>>>>> is the expected page flag?
>>>>>
>>>>> It's still in the buddy at that point right? I'm worried buddy might
>>>>> poke at flags.
>>>>
>>>> Since __free_pages_prepare() executes entirely locklessly, the only way to ensure
>>>> HWPoison flag won't be lost might be only set hwpoison flag iff we can make sure
>>>> pages are not on the way to buddy...
>>>>
>>>> Thanks.
>>>> .
>>>
>>>
>>> To clarify do you not agree repeating SetPageHWPoison is enough for
>>> this? And if not, do you have suggestions on how to fix this race?
>>
>> Do you mean repeating SetPageHWPoison on every branch?
>
> Right.
>
>> Is it possible
>> to make __free_pages_prepare changes page->flags atomically or this race
>> is specified to memory_failure?
>>
>> Thanks.
>> .
>
>
> Adding an atomic op on every fast path page allocation is, I am
> guessing, going to slow down Linux measureably.
>
> Doing it for the benefit of memory_failure, which is the slowest of
> slow paths, seems unpalatable, to me.
Agree, it's not worth to do so.
>
> Neither am I sure it's the only racy place -
> grep for __SetPage and __ClearPage - all these have the same issue, I
> suspect.
>
> At the same time, I'm not an mm maintainer. If you disagree, try to
> upstream a change converting all non atomics in mm to atomics, and see
> what others say.
Since memory_failure might be the only place, this change would be unacceptable.
We should come up with a better solution. Maybe we can try repeating SetPageHWPoison
and ClearPageHWPoison at a first attempt though it looks somewhat weird to me and makes
code more complicated. But it's already complicated. :)
Thanks.
.
^ permalink raw reply
* Re: [PATCH RESEND] virtio_console: read size from config space during device init
From: Michael S. Tsirkin @ 2026-06-11 7:38 UTC (permalink / raw)
To: Filip Hejsek
Cc: Amit Shah, Arnd Bergmann, Greg Kroah-Hartman, Rusty Russell,
virtualization, linux-kernel
In-Reply-To: <dce7a3af2e8fd674d842efe7e21abb4644248155.camel@gmail.com>
On Thu, Jun 11, 2026 at 08:57:57AM +0200, Filip Hejsek wrote:
> On Wed, 2026-06-10 at 03:04 -0400, Michael S. Tsirkin wrote:
> > On Mon, Feb 23, 2026 at 06:37:02PM +0100, Filip Hejsek wrote:
> > > Previously, the size was only read upon receiving the config interrupt.
> > > This interrupt is sent when the size changes. However, we also need to
> > > read the initial size.
> > >
> > > Also make sure to only read the size from config if F_SIZE is enabled.
> > >
> > > Fixes: 9778829cffd4 ("virtio: console: Store each console's size in the console structure")
> > > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > > ---
> > > This is a resend of [1], which hasn't received any response.
> > >
> > > I found this bug while developing patches for QEMU that add virtio
> > > console resize support. If you want to test this, you can get my QEMU
> > > patches from [2]. You will need to disable multiport
> > > using `-device virtio-serial,max_ports=1`.
> > >
> > > [1]: https://lore.kernel.org/all/20251224-virtio-console-fix-v1-1-69d0349692dc@gmail.com/
> > > [2]: https://lore.kernel.org/all/20250921-console-resize-v5-0-89e3c6727060@gmail.com/
> > >
> > > I'll also repeat my questions from the previous submission here. These are
> > > things that confused me when I was trying to understand the surrounding code,
> > > but should in no way prevent merging this patch.
> > >
> > > - Why does use_multiport use __virtio_test_bit instead of
> > > virtio_has_feature?
> > >
> > > - The VIRTIO_CONSOLE_RESIZE handler sets irq_requested to 1, which I
> > > think makes no sense?
> > > ---
> > > drivers/char/virtio_console.c | 52 ++++++++++++++++++++++++++-----------------
> > > 1 file changed, 31 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > > index 088182e54d..c355f6d392 100644
> > > --- a/drivers/char/virtio_console.c
> > > +++ b/drivers/char/virtio_console.c
> > > @@ -1771,32 +1771,40 @@ static void config_intr(struct virtio_device *vdev)
> > > schedule_work(&portdev->config_work);
> > > }
> > >
> > > -static void config_work_handler(struct work_struct *work)
> > > +static void update_size_from_config(struct ports_device *portdev)
> > > {
> > > - struct ports_device *portdev;
> > > + struct virtio_device *vdev;
> > > + struct port *port;
> > > + u16 rows, cols;
> > >
> > > - portdev = container_of(work, struct ports_device, config_work);
> > > - if (!use_multiport(portdev)) {
> > > - struct virtio_device *vdev;
> > > - struct port *port;
> > > - u16 rows, cols;
> > > + vdev = portdev->vdev;
> > >
> > > - vdev = portdev->vdev;
> > > - virtio_cread(vdev, struct virtio_console_config, cols, &cols);
> > > - virtio_cread(vdev, struct virtio_console_config, rows, &rows);
> > > + /*
> > > + * 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) ||
> >
> > 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? What does "not a valid feature" mean?
I dislike transport code leaking into devices.
> >
> > > + use_multiport(portdev) ||
> > > + !virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> > > + return;
> > >
> > > - port = find_port_by_id(portdev, 0);
> > > - set_console_size(port, rows, cols);
> > > + virtio_cread(vdev, struct virtio_console_config, cols, &cols);
> > > + virtio_cread(vdev, struct virtio_console_config, rows, &rows);
> > >
> > > - /*
> > > - * We'll use this way of resizing only for legacy
> > > - * support. For newer userspace
> > > - * (VIRTIO_CONSOLE_F_MULTPORT+), use control messages
> > > - * to indicate console size changes so that it can be
> > > - * done per-port.
> > > - */
> > > - resize_console(port);
> > > - }
> > > + port = find_port_by_id(portdev, 0);
> > > + set_console_size(port, rows, cols);
> > > + resize_console(port);
> > > +}
> > > +
> > > +static void config_work_handler(struct work_struct *work)
> > > +{
> > > + struct ports_device *portdev;
> > > +
> > > + portdev = container_of(work, struct ports_device, config_work);
> > > + update_size_from_config(portdev);
> > > }
> > >
> > > static int init_vqs(struct ports_device *portdev)
> > > @@ -2054,6 +2062,8 @@ static int virtcons_probe(struct virtio_device *vdev)
> > > __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
> > > VIRTIO_CONSOLE_DEVICE_READY, 1);
> > >
> > > + update_size_from_config(portdev);
> > > +
> > > return 0;
> > >
> > > free_chrdev:
> > >
> > > ---
> > > base-commit: b927546677c876e26eba308550207c2ddf812a43
> > > change-id: 20251224-virtio-console-fix-3d46980ef569
> > >
> > > Best regards,
> > > --
> > > Filip Hejsek <filip.hejsek@gmail.com>
^ permalink raw reply
* Re: [PATCH v3] hwrng: virtio: clamp device-reported used.len at copy_data()
From: Michael S. Tsirkin @ 2026-06-11 7:58 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: <aipn8sIAQ6Ai2sax@gondor.apana.org.au>
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;
memcpy(&value, src, ...)
....
hash = complex_hash_of(value)
....
return p[hash * 512];
is IIUC still a valid spectre v1 gadget leaking a value beyong SIZE, or
did I miss something?
And rng is a kind of a complex hash, but I also think in that "...."
in the kernel is probably large enough to close any transient execution
window.
So sure, we can drop this.
> 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
* 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 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 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 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 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 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 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 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 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 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] 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 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 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: 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: 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] 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 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
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