* [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01
@ 2025-11-22 20:55 Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 01/25] target/i386: Fix CR2 handling for non-canonical addresses Michael Tokarev
` (24 more replies)
0 siblings, 25 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Michael Tokarev
The following patches are queued for QEMU stable v7.2.22:
https://gitlab.com/qemu-project/qemu/-/commits/staging-7.2
Patch freeze is 2025-12-01, and the release is planned for 2025-12-03:
https://wiki.qemu.org/Planning/7.2
Please respond here or CC qemu-stable@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.
The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.
This most likely will be the last release in 7.2.x series.
Thanks!
/mjt
--------------------------------------
01 df9a3372ddeb Mathias Krause:
target/i386: Fix CR2 handling for non-canonical addresses
02 df32e5c568c9 Paolo Bonzini:
i386/cpu: Prevent delivering SIPI during SMM in TCG mode
03 cdba90ac1b0a YiFei Zhu:
i386/tcg/smm_helper: Properly apply DR values on SMM entry / exit
04 5142397c7933 Paolo Bonzini:
async: access bottom half flags with qatomic_read
05 58aa1d08bbc4 Paolo Bonzini:
target/i386: user: do not set up a valid LDT on reset
06 7c7089321670 Bastian Blank:
linux-user: Use correct type for FIBMAP and FIGETBSZ emulation
07 0db2de22fcbf Peter Maydell:
linux-user: permit sendto() with NULL buf and 0 len
08 003f15369de4 Daniel P. Berrangé:
io: add trace event when cancelling TLS handshake
09 2c147611cf56 Daniel P. Berrangé:
io: release active GSource in TLS channel finalizer
10 322c3c4f3abe Daniel P. Berrangé:
io: move websock resource release to close method
11 b7a1f2ca45c7 Daniel P. Berrangé:
io: fix use after free in websocket handshake code
12 3995fc238e05 Daniel P. Berrangé:
crypto: stop requiring "key encipherment" usage in x509 certs
13 ed26056d90dd Richard W.M. Jones:
block/curl.c: Use explicit long constants in curl_easy_setopt calls
14 ad97769e9dcf Richard W.M. Jones:
block/curl.c: Fix CURLOPT_VERBOSE parameter type
15 59506e59e0f0 Eric Blake:
qio: Add trace points to net_listener
16 6e03d5cdc991 Eric Blake:
qio: Unwatch before notify in QIONetListener
17 b5676493a08b Eric Blake:
qio: Remember context of qio_net_listener_set_client_func_full
18 9d86181874ab Eric Blake:
qio: Protect NetListener callback with mutex
19 6da0c9828194 Peter Maydell:
hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX
descriptors
20 9d946d56a2ac Peter Maydell:
hw/net/e1000e_core: Correct rx oversize packet checks
21 bab496a18358 Peter Maydell:
hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers()
assert
22 a01344d9d780 Peter Maydell:
net: pad packets to minimum length in qemu_receive_packet()
23 f52db7f34242 Peter Maydell:
hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun
24 032333eba77b Peter Maydell:
hw/display/xlnx_dp: Don't abort for unsupported graphics formats
25 5fc50b4ec841 Peter Maydell:
hw/misc/npcm_clk: Don't divide by zero when calculating frequency
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Stable-7.2.22 01/25] target/i386: Fix CR2 handling for non-canonical addresses
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 02/25] i386/cpu: Prevent delivering SIPI during SMM in TCG mode Michael Tokarev
` (23 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Mathias Krause, Paolo Bonzini, Michael Tokarev
From: Mathias Krause <minipli@grsecurity.net>
Commit 3563362ddfae ("target/i386: Introduce structures for mmu_translate")
accidentally modified CR2 for non-canonical address exceptions while these
should lead to a #GP / #SS instead -- without changing CR2.
Fix that.
A KUT test for this was submitted as [1].
[1] https://lore.kernel.org/kvm/20250612141637.131314-1-minipli@grsecurity.net/
Fixes: 3563362ddfae ("target/i386: Introduce structures for mmu_translate")
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Link: https://lore.kernel.org/r/20250612142155.132175-1-minipli@grsecurity.net
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit df9a3372ddebfcfc135861fa2d53cef6f98065f9)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c
index 9e9e02e1ad..436d65d742 100644
--- a/target/i386/tcg/sysemu/excp_helper.c
+++ b/target/i386/tcg/sysemu/excp_helper.c
@@ -582,7 +582,8 @@ static bool get_physical_address(CPUX86State *env, vaddr addr,
if (sext != 0 && sext != -1) {
*err = (TranslateFault){
.exception_index = EXCP0D_GPF,
- .cr2 = addr,
+ /* non-canonical #GP doesn't change CR2 */
+ .cr2 = env->cr[2],
};
return false;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 02/25] i386/cpu: Prevent delivering SIPI during SMM in TCG mode
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 01/25] target/i386: Fix CR2 handling for non-canonical addresses Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 03/25] i386/tcg/smm_helper: Properly apply DR values on SMM entry / exit Michael Tokarev
` (22 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Paolo Bonzini, YiFei Zhu, Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
[commit message by YiFei Zhu]
A malicious kernel may control the instruction pointer in SMM in a
multi-processor VM by sending a sequence of IPIs via APIC:
CPU0 CPU1
IPI(CPU1, MODE_INIT)
x86_cpu_exec_reset()
apic_init_reset()
s->wait_for_sipi = true
IPI(CPU1, MODE_SMI)
do_smm_enter()
env->hflags |= HF_SMM_MASK;
IPI(CPU1, MODE_STARTUP, vector)
do_cpu_sipi()
apic_sipi()
/* s->wait_for_sipi check passes */
cpu_x86_load_seg_cache_sipi(vector)
A different sequence, SMI INIT SIPI, is also buggy in TCG because
INIT is not blocked or latched during SMM. However, it is not
vulnerable to an instruction pointer control in the same way because
x86_cpu_exec_reset clears env->hflags, exiting SMM.
Fixes: a9bad65d2c1f ("target-i386: wake up processors that receive an SMI")
Analyzed-by: YiFei Zhu <zhuyifei@google.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit df32e5c568c9cf68c15a9bbd98d0c3aff19eab63)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a7c2b301a8..315bef8f43 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -499,8 +499,6 @@ void apic_sipi(DeviceState *dev)
{
APICCommonState *s = APIC(dev);
- cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
-
if (!s->wait_for_sipi)
return;
cpu_x86_load_seg_cache_sipi(s->cpu, s->sipi_vector);
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 290d9d309c..723f50279a 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -602,6 +602,10 @@ void do_cpu_init(X86CPU *cpu)
void do_cpu_sipi(X86CPU *cpu)
{
+ CPUX86State *env = &cpu->env;
+ if (env->hflags & HF_SMM_MASK) {
+ return;
+ }
apic_sipi(cpu->apic_state);
}
#else
diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
index 2c9bd007ad..8294a668d6 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -146,6 +146,7 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
apic_poll_irq(cpu->apic_state);
break;
case CPU_INTERRUPT_SIPI:
+ cpu_reset_interrupt(cs, CPU_INTERRUPT_SIPI);
do_cpu_sipi(cpu);
break;
case CPU_INTERRUPT_SMI:
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 03/25] i386/tcg/smm_helper: Properly apply DR values on SMM entry / exit
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 01/25] target/i386: Fix CR2 handling for non-canonical addresses Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 02/25] i386/cpu: Prevent delivering SIPI during SMM in TCG mode Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 04/25] async: access bottom half flags with qatomic_read Michael Tokarev
` (21 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, YiFei Zhu, unvariant.winter, Paolo Bonzini,
Michael Tokarev
From: YiFei Zhu <zhuyifei@google.com>
do_smm_enter and helper_rsm sets the env->dr, but does not sync the
values with cpu_x86_update_dr7. A malicious kernel may control the
instruction pointer in SMM by setting a breakpoint on the SMI
entry point, and after do_smm_enter cpu->breakpoints contains the
stale breakpoint; and because IDT is not reloaded upon SMI entry,
the debug exception handler controlled by the malicious kernel
is invoked.
Fixes: 01df040b5247 ("x86: Debug register emulation (Jan Kiszka)")
Reported-by: unvariant.winter@gmail.com
Signed-off-by: YiFei Zhu <zhuyifei@google.com>
Link: https://lore.kernel.org/r/2bacb9b24e9d337dbe48791aa25d349eb9c52c3a.1758794468.git.zhuyifei@google.com
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit cdba90ac1b0ac789b10c0b5f6ef7e9558237ec66)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/i386/tcg/sysemu/smm_helper.c b/target/i386/tcg/sysemu/smm_helper.c
index a45b5651c3..f3d0bbb8c3 100644
--- a/target/i386/tcg/sysemu/smm_helper.c
+++ b/target/i386/tcg/sysemu/smm_helper.c
@@ -168,7 +168,7 @@ void do_smm_enter(X86CPU *cpu)
env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
CR0_PG_MASK));
cpu_x86_update_cr4(env, 0);
- env->dr[7] = 0x00000400;
+ helper_set_dr(env, 7, 0x00000400);
cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
0xffffffff,
@@ -233,8 +233,8 @@ void helper_rsm(CPUX86State *env)
env->eip = x86_ldq_phys(cs, sm_state + 0x7f78);
cpu_load_eflags(env, x86_ldl_phys(cs, sm_state + 0x7f70),
~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C | DF_MASK));
- env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7f68);
- env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7f60);
+ helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7f68));
+ helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7f60));
cpu_x86_update_cr4(env, x86_ldl_phys(cs, sm_state + 0x7f48));
cpu_x86_update_cr3(env, x86_ldq_phys(cs, sm_state + 0x7f50));
@@ -268,8 +268,8 @@ void helper_rsm(CPUX86State *env)
env->regs[R_EDX] = x86_ldl_phys(cs, sm_state + 0x7fd8);
env->regs[R_ECX] = x86_ldl_phys(cs, sm_state + 0x7fd4);
env->regs[R_EAX] = x86_ldl_phys(cs, sm_state + 0x7fd0);
- env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7fcc);
- env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7fc8);
+ helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7fcc));
+ helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7fc8));
env->tr.selector = x86_ldl_phys(cs, sm_state + 0x7fc4) & 0xffff;
env->tr.base = x86_ldl_phys(cs, sm_state + 0x7f64);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 04/25] async: access bottom half flags with qatomic_read
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (2 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 03/25] i386/tcg/smm_helper: Properly apply DR values on SMM entry / exit Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 05/25] target/i386: user: do not set up a valid LDT on reset Michael Tokarev
` (20 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Paolo Bonzini, Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
Running test-aio-multithread under TSAN reveals data races on bh->flags.
Because bottom halves may be scheduled or canceled asynchronously,
without taking a lock, adjust aio_compute_bh_timeout() and aio_ctx_check()
to use a relaxed read to access the flags.
Use an acquire load to ensure that anything that was written prior to
qemu_bh_schedule() is visible.
Closes: https://gitlab.com/qemu-project/qemu/-/issues/2749
Closes: https://gitlab.com/qemu-project/qemu/-/issues/851
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 5142397c79330aab9bef3230991c8ac0c251110f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/util/async.c b/util/async.c
index 0cc3037e0c..01961898be 100644
--- a/util/async.c
+++ b/util/async.c
@@ -247,8 +247,9 @@ static int64_t aio_compute_bh_timeout(BHList *head, int timeout)
QEMUBH *bh;
QSLIST_FOREACH_RCU(bh, head, next) {
- if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
- if (bh->flags & BH_IDLE) {
+ int flags = qatomic_load_acquire(&bh->flags);
+ if ((flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
+ if (flags & BH_IDLE) {
/* idle bottom halves will be polled at least
* every 10ms */
timeout = 10000000;
@@ -326,14 +327,16 @@ aio_ctx_check(GSource *source)
aio_notify_accept(ctx);
QSLIST_FOREACH_RCU(bh, &ctx->bh_list, next) {
- if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
+ int flags = qatomic_load_acquire(&bh->flags);
+ if ((flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
return true;
}
}
QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) {
QSLIST_FOREACH_RCU(bh, &s->bh_list, next) {
- if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
+ int flags = qatomic_load_acquire(&bh->flags);
+ if ((flags & (BH_SCHEDULED | BH_DELETED)) == BH_SCHEDULED) {
return true;
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 05/25] target/i386: user: do not set up a valid LDT on reset
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (3 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 04/25] async: access bottom half flags with qatomic_read Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 06/25] linux-user: Use correct type for FIBMAP and FIGETBSZ emulation Michael Tokarev
` (19 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Paolo Bonzini, Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
In user-mode emulation, QEMU uses the default setting of the LDT base
and limit, which places it at the bottom 64K of virtual address space.
However, by default there is no LDT at all in Linux processes, and
therefore the limit should be 0.
This is visible as a NULL pointer dereference in LSL and LAR instructions
when they try to read the LDT at an unmapped address.
Resolves: #1376
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 58aa1d08bbc406ba3982f32ffb1bef0ff4f8f369)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 489ab9cd41..682d71be88 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5906,7 +5906,11 @@ static void x86_cpu_reset(DeviceState *dev)
env->idt.limit = 0xffff;
env->gdt.limit = 0xffff;
+#if defined(CONFIG_USER_ONLY)
+ env->ldt.limit = 0;
+#else
env->ldt.limit = 0xffff;
+#endif
env->ldt.flags = DESC_P_MASK | (2 << DESC_TYPE_SHIFT);
env->tr.limit = 0xffff;
env->tr.flags = DESC_P_MASK | (11 << DESC_TYPE_SHIFT);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 06/25] linux-user: Use correct type for FIBMAP and FIGETBSZ emulation
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (4 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 05/25] target/i386: user: do not set up a valid LDT on reset Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 07/25] linux-user: permit sendto() with NULL buf and 0 len Michael Tokarev
` (18 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Bastian Blank, Bastian Blank, Peter Maydell,
Helge Deller, Michael Tokarev
From: Bastian Blank <bblank@thinkmo.de>
Both the FIBMAP and FIGETBSZ ioctl get "int *" (pointer to 32bit
integer) as argument, not "long *" as specified in qemu. Using the
correct type makes the emulation work in cross endian context.
Both ioctl does not seem to be documented. However the kernel
implementation has always used "int *".
Signed-off-by: Bastian Blank <waldi@debian.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3185
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Helge Deller <deller@gmx.de>
Reviwed-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(cherry picked from commit 7c7089321670fb51022a1c4493cbcc69aa288a0f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 071f7ca253..261cef1e75 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -129,13 +129,13 @@
IOCTL(FDTWADDLE, 0, TYPE_NULL)
IOCTL(FDEJECT, 0, TYPE_NULL)
- IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
+ IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_INT))
#ifdef FICLONE
IOCTL(FICLONE, IOC_W, TYPE_INT)
IOCTL(FICLONERANGE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_file_clone_range)))
#endif
- IOCTL(FIGETBSZ, IOC_R, MK_PTR(TYPE_LONG))
+ IOCTL(FIGETBSZ, IOC_R, MK_PTR(TYPE_INT))
#ifdef CONFIG_FIEMAP
IOCTL_SPECIAL(FS_IOC_FIEMAP, IOC_W | IOC_R, do_ioctl_fs_ioc_fiemap,
MK_PTR(MK_STRUCT(STRUCT_fiemap)))
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 07/25] linux-user: permit sendto() with NULL buf and 0 len
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (5 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 06/25] linux-user: Use correct type for FIBMAP and FIGETBSZ emulation Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 08/25] io: add trace event when cancelling TLS handshake Michael Tokarev
` (17 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Michael Tokarev,
Philippe Mathieu-Daudé, Richard Henderson
From: Peter Maydell <peter.maydell@linaro.org>
If you pass sendto() a NULL buffer, this is usually an error
(causing an EFAULT return); however if you pass a 0 length then
we should not try to validate the buffer provided. Instead we
skip the copying of the user data and possible processing
through fd_trans_target_to_host_data, and call the host syscall
with NULL, 0.
(unlock_user() permits a NULL buffer pointer for "do nothing"
so we don't need to special case the unlock code.)
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3102
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20251028142001.3011630-1-peter.maydell@linaro.org>
(cherry picked from commit 0db2de22fcbf90adafab9d9dd1fc8203c66bfa75)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 737065c28c..93b66bd2d7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3529,7 +3529,7 @@ static abi_long do_sendto(int fd, abi_ulong msg, size_t len, int flags,
abi_ulong target_addr, socklen_t addrlen)
{
void *addr;
- void *host_msg;
+ void *host_msg = NULL;
void *copy_msg = NULL;
abi_long ret;
@@ -3537,16 +3537,19 @@ static abi_long do_sendto(int fd, abi_ulong msg, size_t len, int flags,
return -TARGET_EINVAL;
}
- host_msg = lock_user(VERIFY_READ, msg, len, 1);
- if (!host_msg)
- return -TARGET_EFAULT;
- if (fd_trans_target_to_host_data(fd)) {
- copy_msg = host_msg;
- host_msg = g_malloc(len);
- memcpy(host_msg, copy_msg, len);
- ret = fd_trans_target_to_host_data(fd)(host_msg, len);
- if (ret < 0) {
- goto fail;
+ if (len != 0) {
+ host_msg = lock_user(VERIFY_READ, msg, len, 1);
+ if (!host_msg) {
+ return -TARGET_EFAULT;
+ }
+ if (fd_trans_target_to_host_data(fd)) {
+ copy_msg = host_msg;
+ host_msg = g_malloc(len);
+ memcpy(host_msg, copy_msg, len);
+ ret = fd_trans_target_to_host_data(fd)(host_msg, len);
+ if (ret < 0) {
+ goto fail;
+ }
}
}
if (target_addr) {
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 08/25] io: add trace event when cancelling TLS handshake
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (6 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 07/25] linux-user: permit sendto() with NULL buf and 0 len Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 09/25] io: release active GSource in TLS channel finalizer Michael Tokarev
` (16 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Daniel P. Berrangé, Philippe Mathieu-Daudé,
Michael Tokarev
From: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 003f15369de4e290a4d2e58292d96c5a506e4ee6)
(Mjt: pick this commit up so that the next changes applies (more) cleanly)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/io/channel-tls.c b/io/channel-tls.c
index a91efb57f3..686271acf7 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -380,6 +380,7 @@ static int qio_channel_tls_close(QIOChannel *ioc,
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
if (tioc->hs_ioc_tag) {
+ trace_qio_channel_tls_handshake_cancel(ioc);
g_clear_handle_id(&tioc->hs_ioc_tag, g_source_remove);
}
diff --git a/io/trace-events b/io/trace-events
index 3cc5cf1efd..d4c0f84a9a 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -43,6 +43,7 @@ qio_channel_tls_handshake_start(void *ioc) "TLS handshake start ioc=%p"
qio_channel_tls_handshake_pending(void *ioc, int status) "TLS handshake pending ioc=%p status=%d"
qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p"
qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p"
+qio_channel_tls_handshake_cancel(void *ioc) "TLS handshake cancel ioc=%p"
qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p"
qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p"
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 09/25] io: release active GSource in TLS channel finalizer
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (7 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 08/25] io: add trace event when cancelling TLS handshake Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 10/25] io: move websock resource release to close method Michael Tokarev
` (15 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Daniel P. Berrangé, Eric Blake, Michael Tokarev
From: Daniel P. Berrangé <berrange@redhat.com>
While code is supposed to call qio_channel_close() before releasing the
last reference on an QIOChannel, this is not guaranteed. QIOChannelFile
and QIOChannelSocket both cleanup resources in their finalizer if the
close operation was missed.
This ensures the TLS channel will do the same failsafe cleanup.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 2c147611cf568eb1cd7dc8bf4479b272bad3b9d6)
(Mjt: remove releasing of ioc->bye_ioc_tag due to missing in 7.2.x
v9.2.0-1773-g30ee88622edf "io: tls: Add qio_channel_tls_bye")
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 686271acf7..3b3934db72 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -255,6 +255,11 @@ static void qio_channel_tls_finalize(Object *obj)
{
QIOChannelTLS *ioc = QIO_CHANNEL_TLS(obj);
+ if (ioc->hs_ioc_tag) {
+ trace_qio_channel_tls_handshake_cancel(ioc);
+ g_clear_handle_id(&ioc->hs_ioc_tag, g_source_remove);
+ }
+
object_unref(OBJECT(ioc->master));
qcrypto_tls_session_free(ioc->session);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 10/25] io: move websock resource release to close method
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (8 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 09/25] io: release active GSource in TLS channel finalizer Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 11/25] io: fix use after free in websocket handshake code Michael Tokarev
` (14 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Daniel P. Berrangé, Eric Blake, Michael Tokarev
From: Daniel P. Berrangé <berrange@redhat.com>
The QIOChannelWebsock object releases all its resources in the
finalize callback. This is later than desired, as callers expect
to be able to call qio_channel_close() to fully close a channel
and release resources related to I/O.
The logic in the finalize method is at most a failsafe to handle
cases where a consumer forgets to call qio_channel_close.
This adds equivalent logic to the close method to release the
resources, using g_clear_handle_id/g_clear_pointer to be robust
against repeated invocations. The finalize method is tweaked
so that the GSource is removed before releasing the underlying
channel.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 322c3c4f3abee616a18b3bfe563ec29dd67eae63)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/io/channel-websock.c b/io/channel-websock.c
index fb4932ade7..205df0b25a 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -921,13 +921,13 @@ static void qio_channel_websock_finalize(Object *obj)
buffer_free(&ioc->encinput);
buffer_free(&ioc->encoutput);
buffer_free(&ioc->rawinput);
- object_unref(OBJECT(ioc->master));
if (ioc->io_tag) {
g_source_remove(ioc->io_tag);
}
if (ioc->io_err) {
error_free(ioc->io_err);
}
+ object_unref(OBJECT(ioc->master));
}
@@ -1217,6 +1217,15 @@ static int qio_channel_websock_close(QIOChannel *ioc,
QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc);
trace_qio_channel_websock_close(ioc);
+ buffer_free(&wioc->encinput);
+ buffer_free(&wioc->encoutput);
+ buffer_free(&wioc->rawinput);
+ if (wioc->io_tag) {
+ g_clear_handle_id(&wioc->io_tag, g_source_remove);
+ }
+ if (wioc->io_err) {
+ g_clear_pointer(&wioc->io_err, error_free);
+ }
return qio_channel_close(wioc->master, errp);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 11/25] io: fix use after free in websocket handshake code
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (9 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 10/25] io: move websock resource release to close method Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 12/25] crypto: stop requiring "key encipherment" usage in x509 certs Michael Tokarev
` (13 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Daniel P. Berrangé, Grant Millar | Cylo,
Eric Blake, Michael Tokarev
From: Daniel P. Berrangé <berrange@redhat.com>
If the QIOChannelWebsock object is freed while it is waiting to
complete a handshake, a GSource is leaked. This can lead to the
callback firing later on and triggering a use-after-free in the
use of the channel. This was observed in the VNC server with the
following trace from valgrind:
==2523108== Invalid read of size 4
==2523108== at 0x4054A24: vnc_disconnect_start (vnc.c:1296)
==2523108== by 0x4054A24: vnc_client_error (vnc.c:1392)
==2523108== by 0x4068A09: vncws_handshake_done (vnc-ws.c:105)
==2523108== by 0x44863B4: qio_task_complete (task.c:197)
==2523108== by 0x448343D: qio_channel_websock_handshake_io (channel-websock.c:588)
==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398)
==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 (gmain.c:4249)
==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237)
==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287)
==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310)
==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108== Address 0x57a6e0dc is 28 bytes inside a block of size 103,608 free'd
==2523108== at 0x5F2FE43: free (vg_replace_malloc.c:989)
==2523108== by 0x6EDC444: g_free (gmem.c:208)
==2523108== by 0x4053F23: vnc_update_client (vnc.c:1153)
==2523108== by 0x4053F23: vnc_refresh (vnc.c:3225)
==2523108== by 0x4042881: dpy_refresh (console.c:880)
==2523108== by 0x4042881: gui_update (console.c:90)
==2523108== by 0x45EFA1B: timerlist_run_timers.part.0 (qemu-timer.c:562)
==2523108== by 0x45EFC8F: timerlist_run_timers (qemu-timer.c:495)
==2523108== by 0x45EFC8F: qemu_clock_run_timers (qemu-timer.c:576)
==2523108== by 0x45EFC8F: qemu_clock_run_all_timers (qemu-timer.c:663)
==2523108== by 0x45EC765: main_loop_wait (main-loop.c:600)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108== Block was alloc'd at
==2523108== at 0x5F343F3: calloc (vg_replace_malloc.c:1675)
==2523108== by 0x6EE2F81: g_malloc0 (gmem.c:133)
==2523108== by 0x4057DA3: vnc_connect (vnc.c:3245)
==2523108== by 0x448591B: qio_net_listener_channel_func (net-listener.c:54)
==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398)
==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 (gmain.c:4249)
==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237)
==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287)
==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310)
==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108==
The above can be reproduced by launching QEMU with
$ qemu-system-x86_64 -vnc localhost:0,websocket=5700
and then repeatedly running:
for i in {1..100}; do
(echo -n "GET / HTTP/1.1" && sleep 0.05) | nc -w 1 localhost 5700 &
done
CVE-2025-11234
Reported-by: Grant Millar | Cylo <rid@cylo.io>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit b7a1f2ca45c7865b9e98e02ae605a65fc9458ae9)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
index e180827c57..6700cf8946 100644
--- a/include/io/channel-websock.h
+++ b/include/io/channel-websock.h
@@ -61,7 +61,8 @@ struct QIOChannelWebsock {
size_t payload_remain;
size_t pong_remain;
QIOChannelWebsockMask mask;
- guint io_tag;
+ guint hs_io_tag; /* tracking handshake task */
+ guint io_tag; /* tracking watch task */
Error *io_err;
gboolean io_eof;
uint8_t opcode;
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 205df0b25a..bc0bb80c9a 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -545,6 +545,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
+ wioc->hs_io_tag = 0;
return FALSE;
}
@@ -560,6 +561,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
trace_qio_channel_websock_handshake_complete(ioc);
qio_task_complete(task);
}
+ wioc->hs_io_tag = 0;
return FALSE;
}
trace_qio_channel_websock_handshake_pending(ioc, G_IO_OUT);
@@ -586,6 +588,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
+ wioc->hs_io_tag = 0;
return FALSE;
}
if (ret == 0) {
@@ -597,7 +600,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
error_propagate(&wioc->io_err, err);
trace_qio_channel_websock_handshake_reply(ioc);
- qio_channel_add_watch(
+ wioc->hs_io_tag = qio_channel_add_watch(
wioc->master,
G_IO_OUT,
qio_channel_websock_handshake_send,
@@ -906,11 +909,12 @@ void qio_channel_websock_handshake(QIOChannelWebsock *ioc,
trace_qio_channel_websock_handshake_start(ioc);
trace_qio_channel_websock_handshake_pending(ioc, G_IO_IN);
- qio_channel_add_watch(ioc->master,
- G_IO_IN,
- qio_channel_websock_handshake_io,
- task,
- NULL);
+ ioc->hs_io_tag = qio_channel_add_watch(
+ ioc->master,
+ G_IO_IN,
+ qio_channel_websock_handshake_io,
+ task,
+ NULL);
}
@@ -921,6 +925,9 @@ static void qio_channel_websock_finalize(Object *obj)
buffer_free(&ioc->encinput);
buffer_free(&ioc->encoutput);
buffer_free(&ioc->rawinput);
+ if (ioc->hs_io_tag) {
+ g_source_remove(ioc->hs_io_tag);
+ }
if (ioc->io_tag) {
g_source_remove(ioc->io_tag);
}
@@ -1220,6 +1227,9 @@ static int qio_channel_websock_close(QIOChannel *ioc,
buffer_free(&wioc->encinput);
buffer_free(&wioc->encoutput);
buffer_free(&wioc->rawinput);
+ if (wioc->hs_io_tag) {
+ g_clear_handle_id(&wioc->hs_io_tag, g_source_remove);
+ }
if (wioc->io_tag) {
g_clear_handle_id(&wioc->io_tag, g_source_remove);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 12/25] crypto: stop requiring "key encipherment" usage in x509 certs
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (10 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 11/25] io: fix use after free in websocket handshake code Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 13/25] block/curl.c: Use explicit long constants in curl_easy_setopt calls Michael Tokarev
` (12 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Daniel P. Berrangé, Eric Blake, Michael Tokarev
From: Daniel P. Berrangé <berrange@redhat.com>
This usage flag was deprecated by RFC8813, such that it is
forbidden to be present for certs using ECDSA/ECDH algorithms,
and in TLS 1.3 is conceptually obsolete.
As such many valid certs will no longer have this key usage
flag set, and QEMU should not be rejecting them, as this
prevents use of otherwise valid & desirable algorithms.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 3995fc238e0599e0417ba958ffc5c7a609e82a7f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index d14313925d..27ed568637 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -144,7 +144,7 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 *creds,
if (status < 0) {
if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT;
+ GNUTLS_KEY_DIGITAL_SIGNATURE;
} else {
error_setg(errp,
"Unable to query certificate %s key usage: %s",
@@ -171,14 +171,6 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 *creds,
return -1;
}
}
- if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
- if (critical) {
- error_setg(errp,
- "Certificate %s usage does not permit key "
- "encipherment", certFile);
- return -1;
- }
- }
}
return 0;
diff --git a/docs/system/tls.rst b/docs/system/tls.rst
index e284c82801..a4f6781d62 100644
--- a/docs/system/tls.rst
+++ b/docs/system/tls.rst
@@ -118,7 +118,6 @@ information for each server, and use it to issue server certificates.
ip_address = 2620:0:cafe::87
ip_address = 2001:24::92
tls_www_server
- encryption_key
signing_key
EOF
# certtool --generate-privkey > server-hostNNN-key.pem
@@ -134,9 +133,8 @@ the subject alt name extension data. The ``tls_www_server`` keyword is
the key purpose extension to indicate this certificate is intended for
usage in a web server. Although QEMU network services are not in fact
HTTP servers (except for VNC websockets), setting this key purpose is
-still recommended. The ``encryption_key`` and ``signing_key`` keyword is
-the key usage extension to indicate this certificate is intended for
-usage in the data session.
+still recommended. The ``signing_key`` keyword is the key usage extension
+to indicate this certificate is intended for usage in the data session.
The ``server-hostNNN-key.pem`` and ``server-hostNNN-cert.pem`` files
should now be securely copied to the server for which they were
@@ -171,7 +169,6 @@ certificates.
organization = Name of your organization
cn = hostNNN.foo.example.com
tls_www_client
- encryption_key
signing_key
EOF
# certtool --generate-privkey > client-hostNNN-key.pem
@@ -187,9 +184,8 @@ the ``dns_name`` and ``ip_address`` fields are not included. The
``tls_www_client`` keyword is the key purpose extension to indicate this
certificate is intended for usage in a web client. Although QEMU network
clients are not in fact HTTP clients, setting this key purpose is still
-recommended. The ``encryption_key`` and ``signing_key`` keyword is the
-key usage extension to indicate this certificate is intended for usage
-in the data session.
+recommended. The ``signing_key`` keyword is the key usage extension to
+indicate this certificate is intended for usage in the data session.
The ``client-hostNNN-key.pem`` and ``client-hostNNN-cert.pem`` files
should now be securely copied to the client for which they were
@@ -222,7 +218,6 @@ client and server instructions in one.
ip_address = 2001:24::92
tls_www_server
tls_www_client
- encryption_key
signing_key
EOF
# certtool --generate-privkey > both-hostNNN-key.pem
diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h
index 247e7160eb..d422838ef6 100644
--- a/tests/unit/crypto-tls-x509-helpers.h
+++ b/tests/unit/crypto-tls-x509-helpers.h
@@ -143,8 +143,7 @@ void test_tls_cleanup(const char *keyfile);
.basicConstraintsIsCA = false, \
.keyUsageEnable = true, \
.keyUsageCritical = true, \
- .keyUsageValue = \
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyUsageValue = GNUTLS_KEY_DIGITAL_SIGNATURE, \
.keyPurposeEnable = true, \
.keyPurposeCritical = true, \
.keyPurposeOID1 = GNUTLS_KP_TLS_WWW_CLIENT, \
@@ -163,8 +162,7 @@ void test_tls_cleanup(const char *keyfile);
.basicConstraintsIsCA = false, \
.keyUsageEnable = true, \
.keyUsageCritical = true, \
- .keyUsageValue = \
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyUsageValue = GNUTLS_KEY_DIGITAL_SIGNATURE, \
.keyPurposeEnable = true, \
.keyPurposeCritical = true, \
.keyPurposeOID1 = GNUTLS_KP_TLS_WWW_SERVER, \
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 3c25d75ca1..2025d75365 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -166,14 +166,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -196,7 +196,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -211,7 +211,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -226,7 +226,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -250,7 +250,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* no-basic */
@@ -264,7 +264,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* Key usage:dig-sig:critical */
@@ -278,7 +278,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -303,7 +303,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT |
+ GNUTLS_KEY_DIGITAL_SIGNATURE |
GNUTLS_KEY_KEY_CERT_SIGN,
false, false, NULL, NULL,
0, 0);
@@ -406,7 +406,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT |
+ GNUTLS_KEY_DIGITAL_SIGNATURE |
GNUTLS_KEY_KEY_CERT_SIGN,
false, false, NULL, NULL,
0, 0);
@@ -508,21 +508,21 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(servercertexp1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, -1);
TLS_CERT_REQ(clientcertexp1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, -1);
@@ -546,21 +546,21 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(servercertnew1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
1, 2);
TLS_CERT_REQ(clientcertnew1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
1, 2);
@@ -599,14 +599,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertlevel2breq, cacertlevel1breq,
"UK", "qemu client level 2b", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 615a1344b4..10eaade834 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -454,14 +454,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -469,7 +469,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -488,7 +488,7 @@ int main(int argc, char **argv)
"192.168.122.1", "fec0::dead:beaf",
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* This intentionally doesn't replicate */
@@ -497,7 +497,7 @@ int main(int argc, char **argv)
"192.168.122.1", "fec0::dead:beaf",
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -601,14 +601,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertlevel2breq, cacertlevel1breq,
"UK", "qemu client level 2b", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
diff --git a/tests/unit/test-io-channel-tls.c b/tests/unit/test-io-channel-tls.c
index cc39247556..ff73477671 100644
--- a/tests/unit/test-io-channel-tls.c
+++ b/tests/unit/test-io-channel-tls.c
@@ -302,14 +302,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 13/25] block/curl.c: Use explicit long constants in curl_easy_setopt calls
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (11 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 12/25] crypto: stop requiring "key encipherment" usage in x509 certs Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 14/25] block/curl.c: Fix CURLOPT_VERBOSE parameter type Michael Tokarev
` (11 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard W.M. Jones, Chenxi Mao,
Daniel P. Berrangé, Akihiko Odaki, Thomas Huth,
Richard Henderson, Michael Tokarev
From: "Richard W.M. Jones" <rjones@redhat.com>
curl_easy_setopt takes a variable argument that depends on what
CURLOPT you are setting. Some require a long constant. Passing a
plain int constant is potentially wrong on some platforms.
With warnings enabled, multiple warnings like this were printed:
../block/curl.c: In function ‘curl_init_state’:
../block/curl.c:474:13: warning: call to ‘_curl_easy_setopt_err_long’ declared with attribute warning: curl_easy_setopt expects a long argument [-Wattribute-warning]
474 | curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1) ||
| ^
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Chenxi Mao <maochenxi@bosc.ac.cn>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20251009141026.4042021-2-rjones@redhat.com>
(cherry picked from commit ed26056d90ddff21351f3efd2cb47fea4f0e1d45)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/curl.c b/block/curl.c
index f9c5a1c182..d2b094802e 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -478,11 +478,11 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state)
(void *)curl_read_cb) ||
curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state) ||
curl_easy_setopt(state->curl, CURLOPT_PRIVATE, (void *)state) ||
- curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1) ||
- curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1) ||
- curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1) ||
+ curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1L) ||
+ curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1L) ||
+ curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1L) ||
curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg) ||
- curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1)) {
+ curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1L)) {
goto err;
}
if (s->username) {
@@ -805,7 +805,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
}
s->accept_range = false;
- if (curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1) ||
+ if (curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1L) ||
curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb) ||
curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s)) {
pstrcpy(state->errmsg, CURL_ERROR_SIZE,
diff --git a/contrib/elf2dmp/download.c b/contrib/elf2dmp/download.c
index bd7650a7a2..e0c1075446 100644
--- a/contrib/elf2dmp/download.c
+++ b/contrib/elf2dmp/download.c
@@ -28,8 +28,8 @@ int download_url(const char *name, const char *url)
if (curl_easy_setopt(curl, CURLOPT_URL, url) != CURLE_OK
|| curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, NULL) != CURLE_OK
|| curl_easy_setopt(curl, CURLOPT_WRITEDATA, file) != CURLE_OK
- || curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1) != CURLE_OK
- || curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L) != CURLE_OK
|| curl_easy_perform(curl) != CURLE_OK) {
unlink(name);
fclose(file);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 14/25] block/curl.c: Fix CURLOPT_VERBOSE parameter type
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (12 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 13/25] block/curl.c: Use explicit long constants in curl_easy_setopt calls Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 15/25] qio: Add trace points to net_listener Michael Tokarev
` (10 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Richard W.M. Jones, Kevin Wolf, Michael Tokarev
From: "Richard W.M. Jones" <rjones@redhat.com>
In commit ed26056d90 ("block/curl.c: Use explicit long constants in
curl_easy_setopt calls") we missed a further call that takes a long
parameter.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Message-ID: <20251013124127.604401-1-rjones@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit ad97769e9dcf4dbdaae6d859176e5f37fd6a7c66)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/curl.c b/block/curl.c
index d2b094802e..3b6f18d02c 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -531,7 +531,7 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state)
#endif
#ifdef DEBUG_VERBOSE
- if (curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1)) {
+ if (curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1L)) {
goto err;
}
#endif
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 15/25] qio: Add trace points to net_listener
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (13 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 14/25] block/curl.c: Fix CURLOPT_VERBOSE parameter type Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 16/25] qio: Unwatch before notify in QIONetListener Michael Tokarev
` (9 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Eric Blake, Daniel P. Berrangé, Michael Tokarev
From: Eric Blake <eblake@redhat.com>
Upcoming patches will adjust how net_listener watches for new client
connections; adding trace points now makes it easier to debug that the
changes work as intended. For example, adding
--trace='qio_net_listener*' to the qemu-storage-daemon command line
before --nbd-server will track when the server first starts listening
for clients.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20251113011625.878876-17-eblake@redhat.com>
(cherry picked from commit 59506e59e0f0a773e892104b945d0f15623381a7)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/io/net-listener.c b/io/net-listener.c
index 1c984d69c6..99c3ef0fdb 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -23,6 +23,7 @@
#include "io/dns-resolver.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "trace.h"
QIONetListener *qio_net_listener_new(void)
{
@@ -50,6 +51,7 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
return TRUE;
}
+ trace_qio_net_listener_callback(listener, listener->io_func);
if (listener->io_func) {
listener->io_func(listener, sioc, listener->io_data);
}
@@ -125,6 +127,7 @@ void qio_net_listener_add(QIONetListener *listener,
object_ref(OBJECT(sioc));
listener->connected = true;
+ trace_qio_net_listener_watch(listener, listener->io_func, "add");
if (listener->io_func != NULL) {
object_ref(OBJECT(listener));
listener->io_source[listener->nsioc] = qio_channel_add_watch_source(
@@ -145,6 +148,8 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
{
size_t i;
+ trace_qio_net_listener_unwatch(listener, listener->io_func,
+ "set_client_func");
if (listener->io_notify) {
listener->io_notify(listener->io_data);
}
@@ -160,6 +165,8 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
}
}
+ trace_qio_net_listener_watch(listener, listener->io_func,
+ "set_client_func");
if (listener->io_func != NULL) {
for (i = 0; i < listener->nsioc; i++) {
object_ref(OBJECT(listener));
@@ -220,6 +227,7 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
};
size_t i;
+ trace_qio_net_listener_unwatch(listener, listener->io_func, "wait_client");
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
@@ -249,6 +257,7 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
g_main_loop_unref(loop);
g_main_context_unref(ctxt);
+ trace_qio_net_listener_watch(listener, listener->io_func, "wait_client");
if (listener->io_func != NULL) {
for (i = 0; i < listener->nsioc; i++) {
object_ref(OBJECT(listener));
@@ -270,6 +279,7 @@ void qio_net_listener_disconnect(QIONetListener *listener)
return;
}
+ trace_qio_net_listener_unwatch(listener, listener->io_func, "disconnect");
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
diff --git a/io/trace-events b/io/trace-events
index d4c0f84a9a..263a8b82bd 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -67,3 +67,8 @@ qio_channel_command_new_pid(void *ioc, int writefd, int readfd, int pid) "Comman
qio_channel_command_new_spawn(void *ioc, const char *binary, int flags) "Command new spawn ioc=%p binary=%s flags=%d"
qio_channel_command_abort(void *ioc, int pid) "Command abort ioc=%p pid=%d"
qio_channel_command_wait(void *ioc, int pid, int ret, int status) "Command abort ioc=%p pid=%d ret=%d status=%d"
+
+# net-listener.c
+qio_net_listener_watch(void *listener, void *func, const char *extra) "Net listener=%p watch enabled func=%p by %s"
+qio_net_listener_unwatch(void *listener, void *func, const char *extra) "Net listener=%p watch disabled func=%p by %s"
+qio_net_listener_callback(void *listener, void *func) "Net listener=%p callback forwarding to func=%p"
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 16/25] qio: Unwatch before notify in QIONetListener
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (14 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 15/25] qio: Add trace points to net_listener Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 17/25] qio: Remember context of qio_net_listener_set_client_func_full Michael Tokarev
` (8 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Eric Blake, Daniel P. Berrangé, Michael Tokarev
From: Eric Blake <eblake@redhat.com>
When changing the callback registered with QIONetListener, the code
was calling notify on the old opaque data prior to actually removing
the old GSource objects still pointing to that data. Similarly,
during finalize, it called notify before tearing down the various
GSource objects tied to the data.
In practice, a grep of the QEMU code base found that every existing
client of QIONetListener passes in a NULL notifier (the opaque data,
if non-NULL, outlives the NetListener and so does not need cleanup
when the NetListener is torn down), so this patch has no impact. And
even if a caller had passed in a reference-counted object with a
notifier of object_unref but kept its own reference on the data, then
the early notify would merely reduce a refcount from (say) 2 to 1, but
not free the object. However, it is a latent bug waiting to bite any
future caller that passes in data where the notifier actually frees
the object, because the GSource could then trigger a use-after-free if
it loses the race on a last-minute client connection resulting in the
data being passed to one final use of the async callback.
Better is to delay the notify call until after all GSource that have
been given a copy of the opaque data are torn down.
CC: qemu-stable@nongnu.org
Fixes: 530473924d "io: introduce a network socket listener API", v2.12.0
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20251113011625.878876-18-eblake@redhat.com>
(cherry picked from commit 6e03d5cdc991f5db86969fc6aeaca96234426263)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/io/net-listener.c b/io/net-listener.c
index 99c3ef0fdb..70e70c49fb 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -150,13 +150,6 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
trace_qio_net_listener_unwatch(listener, listener->io_func,
"set_client_func");
- if (listener->io_notify) {
- listener->io_notify(listener->io_data);
- }
- listener->io_func = func;
- listener->io_data = data;
- listener->io_notify = notify;
-
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
@@ -165,6 +158,13 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
}
}
+ if (listener->io_notify) {
+ listener->io_notify(listener->io_data);
+ }
+ listener->io_func = func;
+ listener->io_data = data;
+ listener->io_notify = notify;
+
trace_qio_net_listener_watch(listener, listener->io_func,
"set_client_func");
if (listener->io_func != NULL) {
@@ -302,10 +302,10 @@ static void qio_net_listener_finalize(Object *obj)
QIONetListener *listener = QIO_NET_LISTENER(obj);
size_t i;
+ qio_net_listener_disconnect(listener);
if (listener->io_notify) {
listener->io_notify(listener->io_data);
}
- qio_net_listener_disconnect(listener);
for (i = 0; i < listener->nsioc; i++) {
object_unref(OBJECT(listener->sioc[i]));
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 17/25] qio: Remember context of qio_net_listener_set_client_func_full
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (15 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 16/25] qio: Unwatch before notify in QIONetListener Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 18/25] qio: Protect NetListener callback with mutex Michael Tokarev
` (7 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Eric Blake, Daniel P. Berrangé, Michael Tokarev
From: Eric Blake <eblake@redhat.com>
io/net-listener.c has two modes of use: asynchronous (the user calls
qio_net_listener_set_client_func to wake up the callback via the
global GMainContext, or qio_net_listener_set_client_func_full to wake
up the callback via the caller's own alternative GMainContext), and
synchronous (the user calls qio_net_listener_wait_client which creates
its own GMainContext and waits for the first client connection before
returning, with no need for a user's callback). But commit 938c8b79
has a latent logic flaw: when qio_net_listener_wait_client finishes on
its temporary context, it reverts all of the siocs back to the global
GMainContext rather than the potentially non-NULL context they might
have been originally registered with. Similarly, if the user creates
a net-listener, adds initial addresses, registers an async callback
with a non-default context (which ties to all siocs for the initial
addresses), then adds more addresses with qio_net_listener_add, the
siocs for later addresses are blindly placed in the global context,
rather than sharing the context of the earlier ones.
In practice, I don't think this has caused issues. As pointed out by
the original commit, all async callers prior to that commit were
already okay with the NULL default context; and the typical usage
pattern is to first add ALL the addresses the listener will pay
attention to before ever setting the async callback. Likewise, if a
file uses only qio_net_listener_set_client_func instead of
qio_net_listener_set_client_func_full, then it is never using a custom
context, so later assignments of async callbacks will still be to the
same global context as earlier ones. Meanwhile, any callers that want
to do the sync operation to grab the first client are unlikely to
register an async callback; altogether bypassing the question of
whether later assignments of a GSource are being tied to a different
context over time.
I do note that chardev/char-socket.c is the only file that calls both
qio_net_listener_wait_client (sync for a single client in
tcp_chr_accept_server_sync), and qio_net_listener_set_client_func_full
(several places, all with chr->gcontext, but sometimes with a NULL
callback function during teardown). But as far as I can tell, the two
uses are mutually exclusive, based on the is_waitconnect parameter to
qmp_chardev_open_socket_server.
That said, it is more robust to remember when an async callback
function is tied to a non-default context, and have both the sync wait
and any late address additions honor that same context. That way, the
code will be robust even if a later user performs a sync wait for a
specific client in the middle of servicing a longer-lived
QIONetListener that has an async callback for all other clients.
CC: qemu-stable@nongnu.org
Fixes: 938c8b79 ("qio: store gsources for net listeners", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20251113011625.878876-19-eblake@redhat.com>
(cherry picked from commit b5676493a08b4ff80680aae7a1b1bfef8797c6e7)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index ab9f291ed6..42fbfab546 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -50,6 +50,7 @@ struct QIONetListener {
QIOChannelSocket **sioc;
GSource **io_source;
size_t nsioc;
+ GMainContext *context;
bool connected;
diff --git a/io/net-listener.c b/io/net-listener.c
index 70e70c49fb..6b8240a658 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -51,7 +51,8 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
return TRUE;
}
- trace_qio_net_listener_callback(listener, listener->io_func);
+ trace_qio_net_listener_callback(listener, listener->io_func,
+ listener->context);
if (listener->io_func) {
listener->io_func(listener, sioc, listener->io_data);
}
@@ -127,13 +128,14 @@ void qio_net_listener_add(QIONetListener *listener,
object_ref(OBJECT(sioc));
listener->connected = true;
- trace_qio_net_listener_watch(listener, listener->io_func, "add");
+ trace_qio_net_listener_watch(listener, listener->io_func,
+ listener->context, "add");
if (listener->io_func != NULL) {
object_ref(OBJECT(listener));
listener->io_source[listener->nsioc] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[listener->nsioc]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, NULL);
+ listener, (GDestroyNotify)object_unref, listener->context);
}
listener->nsioc++;
@@ -149,7 +151,8 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
size_t i;
trace_qio_net_listener_unwatch(listener, listener->io_func,
- "set_client_func");
+ listener->context, "set_client_func");
+
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
@@ -164,9 +167,10 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
listener->io_func = func;
listener->io_data = data;
listener->io_notify = notify;
+ listener->context = context;
trace_qio_net_listener_watch(listener, listener->io_func,
- "set_client_func");
+ listener->context, "set_client_func");
if (listener->io_func != NULL) {
for (i = 0; i < listener->nsioc; i++) {
object_ref(OBJECT(listener));
@@ -227,7 +231,8 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
};
size_t i;
- trace_qio_net_listener_unwatch(listener, listener->io_func, "wait_client");
+ trace_qio_net_listener_unwatch(listener, listener->io_func,
+ listener->context, "wait_client");
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
@@ -257,14 +262,15 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
g_main_loop_unref(loop);
g_main_context_unref(ctxt);
- trace_qio_net_listener_watch(listener, listener->io_func, "wait_client");
+ trace_qio_net_listener_watch(listener, listener->io_func,
+ listener->context, "wait_client");
if (listener->io_func != NULL) {
for (i = 0; i < listener->nsioc; i++) {
object_ref(OBJECT(listener));
listener->io_source[i] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[i]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, NULL);
+ listener, (GDestroyNotify)object_unref, listener->context);
}
}
@@ -279,7 +285,8 @@ void qio_net_listener_disconnect(QIONetListener *listener)
return;
}
- trace_qio_net_listener_unwatch(listener, listener->io_func, "disconnect");
+ trace_qio_net_listener_unwatch(listener, listener->io_func,
+ listener->context, "disconnect");
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
diff --git a/io/trace-events b/io/trace-events
index 263a8b82bd..6f56f8ba2d 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -69,6 +69,6 @@ qio_channel_command_abort(void *ioc, int pid) "Command abort ioc=%p pid=%d"
qio_channel_command_wait(void *ioc, int pid, int ret, int status) "Command abort ioc=%p pid=%d ret=%d status=%d"
# net-listener.c
-qio_net_listener_watch(void *listener, void *func, const char *extra) "Net listener=%p watch enabled func=%p by %s"
-qio_net_listener_unwatch(void *listener, void *func, const char *extra) "Net listener=%p watch disabled func=%p by %s"
-qio_net_listener_callback(void *listener, void *func) "Net listener=%p callback forwarding to func=%p"
+qio_net_listener_watch(void *listener, void *func, void *ctx, const char *extra) "Net listener=%p watch enabled func=%p ctx=%p by %s"
+qio_net_listener_unwatch(void *listener, void *func, void *ctx, const char *extra) "Net listener=%p watch disabled func=%p ctx=%p by %s"
+qio_net_listener_callback(void *listener, void *func, void *ctx) "Net listener=%p callback forwarding to func=%p ctx=%p"
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 18/25] qio: Protect NetListener callback with mutex
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (16 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 17/25] qio: Remember context of qio_net_listener_set_client_func_full Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 19/25] hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors Michael Tokarev
` (6 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Eric Blake, Daniel P. Berrangé, Michael Tokarev
From: Eric Blake <eblake@redhat.com>
Without a mutex, NetListener can run into this data race between a
thread changing the async callback callback function to use when a
client connects, and the thread servicing polling of the listening
sockets:
Thread 1:
qio_net_listener_set_client_func(lstnr, f1, ...);
=> foreach sock: socket
=> object_ref(lstnr)
=> sock_src = qio_channel_socket_add_watch_source(sock, ...., lstnr, object_unref);
Thread 2:
poll()
=> event POLLIN on socket
=> ref(GSourceCallback)
=> if (lstnr->io_func) // while lstnr->io_func is f1
...interrupt..
Thread 1:
qio_net_listener_set_client_func(lstnr, f2, ...);
=> foreach sock: socket
=> g_source_unref(sock_src)
=> foreach sock: socket
=> object_ref(lstnr)
=> sock_src = qio_channel_socket_add_watch_source(sock, ...., lstnr, object_unref);
Thread 2:
=> call lstnr->io_func(lstnr->io_data) // now sees f2
=> return dispatch(sock)
=> unref(GSourceCallback)
=> destroy-notify
=> object_unref
Found by inspection; I did not spend the time trying to add sleeps or
execute under gdb to try and actually trigger the race in practice.
This is a SEGFAULT waiting to happen if f2 can become NULL because
thread 1 deregisters the user's callback while thread 2 is trying to
service the callback. Other messes are also theoretically possible,
such as running callback f1 with an opaque pointer that should only be
passed to f2 (if the client code were to use more than just a binary
choice between a single async function or NULL).
Mitigating factor: if the code that modifies the QIONetListener can
only be reached by the same thread that is executing the polling and
async callbacks, then we are not in a two-thread race documented above
(even though poll can see two clients trying to connect in the same
window of time, any changes made to the listener by the first async
callback will be completed before the thread moves on to the second
client). However, QEMU is complex enough that this is hard to
generically analyze. If QMP commands (like nbd-server-stop) are run
in the main loop and the listener uses the main loop, things should be
okay. But when a client uses an alternative GMainContext, or if
servicing a QMP command hands off to a coroutine to avoid blocking, I
am unable to state with certainty whether a given net listener can be
modified by a thread different from the polling thread running
callbacks.
At any rate, it is worth having the API be robust. To ensure that
modifying a NetListener can be safely done from any thread, add a
mutex that guarantees atomicity to all members of a listener object
related to callbacks. This problem has been present since
QIONetListener was introduced.
Note that this does NOT prevent the case of a second round of the
user's old async callback being invoked with the old opaque data, even
when the user has already tried to change the async callback during
the first async callback; it is only about ensuring that there is no
sharding (the eventual io_func(io_data) call that does get made will
correspond to a particular combination that the user had requested at
some point in time, and not be sharded to a combination that never
existed in practice). In other words, this patch maintains the status
quo that a user's async callback function already needs to be robust
to parallel clients landing in the same window of poll servicing, even
when only one client is desired, if that particular listener can be
amended in a thread other than the one doing the polling.
CC: qemu-stable@nongnu.org
Fixes: 53047392 ("io: introduce a network socket listener API", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20251113011625.878876-20-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[eblake: minor commit message wording improvements]
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 9d86181874ab7b0e95ae988f6f80715943c618c6)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index 42fbfab546..c2165dc166 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -54,6 +54,7 @@ struct QIONetListener {
bool connected;
+ QemuMutex lock; /* Protects remaining fields */
QIONetListenerClientFunc io_func;
gpointer io_data;
GDestroyNotify io_notify;
diff --git a/io/net-listener.c b/io/net-listener.c
index 6b8240a658..7352b5f8d4 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -23,11 +23,16 @@
#include "io/dns-resolver.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "qemu/lockable.h"
#include "trace.h"
QIONetListener *qio_net_listener_new(void)
{
- return QIO_NET_LISTENER(object_new(TYPE_QIO_NET_LISTENER));
+ QIONetListener *listener;
+
+ listener = QIO_NET_LISTENER(object_new(TYPE_QIO_NET_LISTENER));
+ qemu_mutex_init(&listener->lock);
+ return listener;
}
void qio_net_listener_set_name(QIONetListener *listener,
@@ -44,6 +49,9 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
{
QIONetListener *listener = QIO_NET_LISTENER(opaque);
QIOChannelSocket *sioc;
+ QIONetListenerClientFunc io_func;
+ gpointer io_data;
+ GMainContext *context;
sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
NULL);
@@ -51,10 +59,15 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
return TRUE;
}
- trace_qio_net_listener_callback(listener, listener->io_func,
- listener->context);
- if (listener->io_func) {
- listener->io_func(listener, sioc, listener->io_data);
+ WITH_QEMU_LOCK_GUARD(&listener->lock) {
+ io_func = listener->io_func;
+ io_data = listener->io_data;
+ context = listener->context;
+ }
+
+ trace_qio_net_listener_callback(listener, io_func, context);
+ if (io_func) {
+ io_func(listener, sioc, io_data);
}
object_unref(OBJECT(sioc));
@@ -111,6 +124,9 @@ int qio_net_listener_open_sync(QIONetListener *listener,
void qio_net_listener_add(QIONetListener *listener,
QIOChannelSocket *sioc)
{
+ QIONetListenerClientFunc io_func;
+ GMainContext *context;
+
if (listener->name) {
char *name = g_strdup_printf("%s-listen", listener->name);
qio_channel_set_name(QIO_CHANNEL(sioc), name);
@@ -128,14 +144,18 @@ void qio_net_listener_add(QIONetListener *listener,
object_ref(OBJECT(sioc));
listener->connected = true;
- trace_qio_net_listener_watch(listener, listener->io_func,
- listener->context, "add");
- if (listener->io_func != NULL) {
+ WITH_QEMU_LOCK_GUARD(&listener->lock) {
+ io_func = listener->io_func;
+ context = listener->context;
+ }
+
+ trace_qio_net_listener_watch(listener, io_func, context, "add");
+ if (io_func) {
object_ref(OBJECT(listener));
listener->io_source[listener->nsioc] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[listener->nsioc]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, listener->context);
+ listener, (GDestroyNotify)object_unref, context);
}
listener->nsioc++;
@@ -150,6 +170,7 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
{
size_t i;
+ QEMU_LOCK_GUARD(&listener->lock);
trace_qio_net_listener_unwatch(listener, listener->io_func,
listener->context, "set_client_func");
@@ -230,9 +251,15 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
.loop = loop
};
size_t i;
+ QIONetListenerClientFunc io_func;
+ GMainContext *context;
- trace_qio_net_listener_unwatch(listener, listener->io_func,
- listener->context, "wait_client");
+ WITH_QEMU_LOCK_GUARD(&listener->lock) {
+ io_func = listener->io_func;
+ context = listener->context;
+ }
+
+ trace_qio_net_listener_unwatch(listener, io_func, context, "wait_client");
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
g_source_destroy(listener->io_source[i]);
@@ -262,15 +289,14 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
g_main_loop_unref(loop);
g_main_context_unref(ctxt);
- trace_qio_net_listener_watch(listener, listener->io_func,
- listener->context, "wait_client");
- if (listener->io_func != NULL) {
+ trace_qio_net_listener_watch(listener, io_func, context, "wait_client");
+ if (io_func != NULL) {
for (i = 0; i < listener->nsioc; i++) {
object_ref(OBJECT(listener));
listener->io_source[i] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[i]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, listener->context);
+ listener, (GDestroyNotify)object_unref, context);
}
}
@@ -285,6 +311,7 @@ void qio_net_listener_disconnect(QIONetListener *listener)
return;
}
+ QEMU_LOCK_GUARD(&listener->lock);
trace_qio_net_listener_unwatch(listener, listener->io_func,
listener->context, "disconnect");
for (i = 0; i < listener->nsioc; i++) {
@@ -320,6 +347,7 @@ static void qio_net_listener_finalize(Object *obj)
g_free(listener->io_source);
g_free(listener->sioc);
g_free(listener->name);
+ qemu_mutex_destroy(&listener->lock);
}
static const TypeInfo qio_net_listener_info = {
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 19/25] hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (17 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 18/25] qio: Protect NetListener callback with mutex Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 20/25] hw/net/e1000e_core: Correct rx oversize packet checks Michael Tokarev
` (5 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Akihiko Odaki, Jason Wang,
Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
In e1000e_write_packet_to_guest() we don't write data for RX descriptors
where the buffer address is NULL (as required by the i82574 datasheet
section 7.1.7.2). However, when we do this we still update desc_offset
by the amount of data we would have written to the RX descriptor if
it had a valid buffer pointer, resulting in our dropping that data
entirely. The data sheet is not 100% clear on the subject, but this
seems unlikely to be the correct behaviour.
Rearrange the null-descriptor logic so that we don't treat these
do-nothing descriptors as if we'd really written the data.
This both fixes a bug and also is a prerequisite to cleaning up
the size calculation logic in the next patch.
(Cc to stable largely because it will be needed for the next patch,
which fixes a more serious bug.)
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit 6da0c9828194eb21e54fe4264cd29a1b85a29f33)
(Mjt: context fixup in hw/net/e1000e_core.c:e1000e_write_packet_to_guest())
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 5def4cfc1c..be9c15f01e 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1512,7 +1512,6 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
PCIDevice *d = core->owner;
dma_addr_t base;
uint8_t desc[E1000_MAX_RX_DESC_LEN];
- size_t desc_size;
size_t desc_offset = 0;
size_t iov_ofs = 0;
@@ -1531,12 +1530,6 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
e1000e_ba_state bastate = { { 0 } };
bool is_last = false;
- desc_size = total_size - desc_offset;
-
- if (desc_size > core->rx_desc_buf_size) {
- desc_size = core->rx_desc_buf_size;
- }
-
if (e1000e_ring_empty(core, rxi)) {
return;
}
@@ -1550,6 +1543,12 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
e1000e_read_rx_descr(core, desc, &ba);
if (ba[0]) {
+ size_t desc_size = total_size - desc_offset;
+
+ if (desc_size > core->rx_desc_buf_size) {
+ desc_size = core->rx_desc_buf_size;
+ }
+
if (desc_offset < size) {
static const uint32_t fcs_pad;
size_t iov_copy;
@@ -1609,13 +1608,13 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
(const char *) &fcs_pad, e1000x_fcs_len(core->mac));
}
}
+ desc_offset += desc_size;
+ if (desc_offset >= total_size) {
+ is_last = true;
+ }
} else { /* as per intel docs; skip descriptors with null buf addr */
trace_e1000e_rx_null_descriptor();
}
- desc_offset += desc_size;
- if (desc_offset >= total_size) {
- is_last = true;
- }
e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL,
rss_info, do_ps ? ps_hdr_len : 0, &bastate.written);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 20/25] hw/net/e1000e_core: Correct rx oversize packet checks
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (18 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 19/25] hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 21/25] hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert Michael Tokarev
` (4 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Akihiko Odaki, Jason Wang,
Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
In e1000e_write_packet_to_guest() we attempt to ensure that we don't
write more of a packet to a descriptor than will fit in the guest
configured receive buffers. However, this code does not allow for
the "packet split" feature. When packet splitting is enabled, the
first of up to 4 buffers in the descriptor is used for the packet
header only, with the payload going into buffers 2, 3 and 4. Our
length check only checks against the total sizes of all 4 buffers,
which meant that if an incoming packet was large enough to fit in (1
+ 2 + 3 + 4) but not into (2 + 3 + 4) and packet splitting was
enabled, we would run into the assertion in
e1000e_write_hdr_frag_to_rx_buffers() that we had enough buffers for
the data:
qemu-system-i386: ../../hw/net/e1000e_core.c:1418: void e1000e_write_payload_frag_to_rx_buffers(E1000ECore *, hwaddr *, E1000EBAState *, const char *, dma_addr_t): Assertion `bastate->cur_idx < MAX_PS_BUFFERS' failed.
A malicious guest could provoke this assertion by configuring the
device into loopback mode, and then sending itself a suitably sized
packet into a suitably arrange rx descriptor.
The code also fails to deal with the possibility that the descriptor
buffers are sized such that the trailing checksum word does not fit
into the last descriptor which has actual data, which might also
trigger this assertion.
Rework the length handling to use two variables:
* desc_size is the total amount of data DMA'd to the guest
for the descriptor being processed in this iteration of the loop
* rx_desc_buf_size is the total amount of space left in it
As we copy data to the guest (packet header, payload, checksum),
update these two variables. (Previously we attempted to calculate
desc_size once at the top of the loop, but this is too difficult to
do correctly.) Then we can use the variables to ensure that we clamp
the amount of copied payload data to the remaining space in the
descriptor's buffers, even if we've used one of the buffers up in the
packet-split code, and we can tell whether we have enough space for
the full checksum word in this descriptor or whether we're going to
need to split that to the following descriptor.
I have included comments that hopefully help to make the loop
logic a little clearer.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/537
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit 9d946d56a2ac8a6c2df186e20d24810255c83a3f)
(Mjt: rename e1000e_write_payload_frag_to_rx_buffers back to
e1000e_write_to_rx_buffers for 7.2.x, to compensate for missing in 7.2.x
v8.1.0-693-g17ccd0164796 "igb: RX payload guest writting refactoring")
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index be9c15f01e..deb070d17e 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1526,6 +1526,13 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
rxi = rxr->i;
do {
+ /*
+ * Loop processing descriptors while we have packet data to
+ * DMA to the guest. desc_offset tracks how much data we have
+ * sent to the guest in total over all descriptors, and goes
+ * from 0 up to total_size (the size of everything to send to
+ * the guest including possible trailing 4 bytes of CRC data).
+ */
hwaddr ba[MAX_PS_BUFFERS];
e1000e_ba_state bastate = { { 0 } };
bool is_last = false;
@@ -1543,23 +1550,27 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
e1000e_read_rx_descr(core, desc, &ba);
if (ba[0]) {
- size_t desc_size = total_size - desc_offset;
-
- if (desc_size > core->rx_desc_buf_size) {
- desc_size = core->rx_desc_buf_size;
- }
+ /* Total amount of data DMA'd to the guest in this iteration */
+ size_t desc_size = 0;
+ /*
+ * Total space available in this descriptor (we will update
+ * this as we use it up)
+ */
+ size_t rx_desc_buf_size = core->rx_desc_buf_size;
if (desc_offset < size) {
- static const uint32_t fcs_pad;
size_t iov_copy;
+ /* Amount of data to copy from the incoming packet */
size_t copy_size = size - desc_offset;
- if (copy_size > core->rx_desc_buf_size) {
- copy_size = core->rx_desc_buf_size;
- }
/* For PS mode copy the packet header first */
if (do_ps) {
if (is_first) {
+ /*
+ * e1000e_do_ps() guarantees that buffer 0 has enough
+ * space for the header; otherwise we will not split
+ * the packet (i.e. do_ps is false).
+ */
size_t ps_hdr_copied = 0;
do {
iov_copy = MIN(ps_hdr_len - ps_hdr_copied,
@@ -1579,14 +1590,26 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
} while (ps_hdr_copied < ps_hdr_len);
is_first = false;
+ desc_size += ps_hdr_len;
} else {
/* Leave buffer 0 of each descriptor except first */
/* empty as per spec 7.1.5.1 */
e1000e_write_hdr_to_rx_buffers(core, &ba, &bastate,
NULL, 0);
}
+ rx_desc_buf_size -= core->rxbuf_sizes[0];
}
+ /*
+ * Clamp the amount of packet data we copy into what will fit
+ * into the remaining buffers in the descriptor.
+ */
+ if (copy_size > rx_desc_buf_size) {
+ copy_size = rx_desc_buf_size;
+ }
+ desc_size += copy_size;
+ rx_desc_buf_size -= copy_size;
+
/* Copy packet payload */
while (copy_size) {
iov_copy = MIN(copy_size, iov->iov_len - iov_ofs);
@@ -1601,12 +1624,22 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
iov_ofs = 0;
}
}
+ }
- if (desc_offset + desc_size >= total_size) {
- /* Simulate FCS checksum presence in the last descriptor */
- e1000e_write_to_rx_buffers(core, &ba, &bastate,
- (const char *) &fcs_pad, e1000x_fcs_len(core->mac));
- }
+ if (rx_desc_buf_size &&
+ desc_offset >= size && desc_offset < total_size) {
+ /*
+ * We are in the last 4 bytes corresponding to the FCS checksum.
+ * We only ever write zeroes here (unlike the hardware).
+ */
+ static const uint32_t fcs_pad;
+ /* Amount of space for the trailing checksum */
+ size_t fcs_len = MIN(rx_desc_buf_size,
+ total_size - desc_offset);
+ e1000e_write_to_rx_buffers(core, &ba, &bastate,
+ (const char *)&fcs_pad,
+ fcs_len);
+ desc_size += fcs_len;
}
desc_offset += desc_size;
if (desc_offset >= total_size) {
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 21/25] hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (19 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 20/25] hw/net/e1000e_core: Correct rx oversize packet checks Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 22/25] net: pad packets to minimum length in qemu_receive_packet() Michael Tokarev
` (3 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Akihiko Odaki, Jason Wang,
Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
An assertion in e1000e_write_payload_frag_to_rx_buffers() attempts to
guard against the calling code accidentally trying to write too much
data to a single RX descriptor, such that the E1000EBAState::cur_idx
indexes off the end of the EB1000BAState::written[] array.
Unfortunately it is overzealous: it asserts that cur_idx is in
range after it has been incremented. This will fire incorrectly
for the case where the guest configures four buffers and exactly
enough bytes are written to fill all four of them.
The only places where we use cur_idx and index in to the written[]
array are the functions e1000e_write_hdr_frag_to_rx_buffers() and
e1000e_write_payload_frag_to_rx_buffers(), so we can rewrite this to
assert before doing the array dereference, rather than asserting
after updating cur_idx.
Cc: qemu-stable@nongnu.org
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit bab496a18358643b686f69e2b97d73fb98d37e79)
(Mjt: in 7.2.x it is e1000e_write_to_rx_buffers, not
e1000e_write_payload_frag_to_rx_buffers, due to missing in 7.2.x
v8.1.0-693-g17ccd0164796 "igb: RX payload guest writting refactoring")
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index deb070d17e..14bf3ee948 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1411,10 +1411,13 @@ e1000e_write_to_rx_buffers(E1000ECore *core,
dma_addr_t data_len)
{
while (data_len > 0) {
- uint32_t cur_buf_len = core->rxbuf_sizes[bastate->cur_idx];
- uint32_t cur_buf_bytes_left = cur_buf_len -
- bastate->written[bastate->cur_idx];
- uint32_t bytes_to_write = MIN(data_len, cur_buf_bytes_left);
+ uint32_t cur_buf_len, cur_buf_bytes_left, bytes_to_write;
+
+ assert(bastate->cur_idx < MAX_PS_BUFFERS);
+
+ cur_buf_len = core->rxbuf_sizes[bastate->cur_idx];
+ cur_buf_bytes_left = cur_buf_len - bastate->written[bastate->cur_idx];
+ bytes_to_write = MIN(data_len, cur_buf_bytes_left);
trace_e1000e_rx_desc_buff_write(bastate->cur_idx,
(*ba)[bastate->cur_idx],
@@ -1433,8 +1436,6 @@ e1000e_write_to_rx_buffers(E1000ECore *core,
if (bastate->written[bastate->cur_idx] == cur_buf_len) {
bastate->cur_idx++;
}
-
- assert(bastate->cur_idx < MAX_PS_BUFFERS);
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 22/25] net: pad packets to minimum length in qemu_receive_packet()
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (20 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 21/25] hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 23/25] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun Michael Tokarev
` (2 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Akihiko Odaki, Jason Wang,
Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
In commits like 969e50b61a28 ("net: Pad short frames to minimum size
before sending from SLiRP/TAP") we switched away from requiring
network devices to handle short frames to instead having the net core
code do the padding of short frames out to the ETH_ZLEN minimum size.
We then dropped the code for handling short frames from the network
devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
Remove the logic of padding short frames in the receive path").
This missed one route where the device's receive code can still see a
short frame: if the device is in loopback mode and it transmits a
short frame via the qemu_receive_packet() function, this will be fed
back into its own receive code without being padded.
Add the padding logic to qemu_receive_packet().
This fixes a buffer overrun which can be triggered in the
e1000_receive_iov() logic via the loopback code path.
Other devices that use qemu_receive_packet() to implement loopback
are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
and sungem.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit a01344d9d78089e9e585faaeb19afccff2050abf)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/net/net.c b/net/net.c
index c3391168f6..40db3b5dc0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -728,10 +728,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
{
+ uint8_t min_pkt[ETH_ZLEN];
+ size_t min_pktsz = sizeof(min_pkt);
+
if (!qemu_can_receive_packet(nc)) {
return 0;
}
+ if (net_peer_needs_padding(nc)) {
+ if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+ buf = min_pkt;
+ size = min_pktsz;
+ }
+ }
+
return qemu_net_queue_receive(nc->incoming_queue, buf, size);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 23/25] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (21 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 22/25] net: pad packets to minimum length in qemu_receive_packet() Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 24/25] hw/display/xlnx_dp: Don't abort for unsupported graphics formats Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 25/25] hw/misc/npcm_clk: Don't divide by zero when calculating frequency Michael Tokarev
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Philippe Mathieu-Daudé,
Edgar E. Iglesias, Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
The documentation of the Xilinx DisplayPort subsystem at
https://www.xilinx.com/support/documents/ip_documentation/v_dp_txss1/v3_1/pg299-v-dp-txss1.pdf
doesn't say what happens if a guest tries to issue an AUX write
command with a length greater than the amount of data in the AUX
write FIFO, or tries to write more data to the write FIFO than it can
hold, or issues multiple commands that put data into the AUX read
FIFO without reading it such that it overflows.
Currently QEMU will abort() in these guest-error situations, either
in xlnx_dp.c itself or in the fifo8 code. Make these cases all be
logged as guest errors instead. We choose to ignore the new data on
overflow, and return 0 on underflow. This is in line with how we handled
the "read from empty RX FIFO" case in commit a09ef5040477.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1418
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1419
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1424
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Message-id: 20251106145209.1083998-2-peter.maydell@linaro.org
(cherry picked from commit f52db7f34242d3398bab0bacaa3e5dde99be5258)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index b0828d65aa..d5c6d19ab5 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -432,7 +432,18 @@ static void xlnx_dp_aux_clear_rx_fifo(XlnxDPState *s)
static void xlnx_dp_aux_push_rx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
{
+ size_t avail = fifo8_num_free(&s->rx_fifo);
DPRINTF("Push %u data in rx_fifo\n", (unsigned)len);
+ if (len > avail) {
+ /*
+ * Data sheet doesn't specify behaviour here: we choose to ignore
+ * the excess data.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: ignoring %zu bytes pushed to full RX_FIFO\n",
+ __func__, len - avail);
+ len = avail;
+ }
fifo8_push_all(&s->rx_fifo, buf, len);
}
@@ -463,7 +474,18 @@ static void xlnx_dp_aux_clear_tx_fifo(XlnxDPState *s)
static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
{
+ size_t avail = fifo8_num_free(&s->tx_fifo);
DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
+ if (len > avail) {
+ /*
+ * Data sheet doesn't specify behaviour here: we choose to ignore
+ * the excess data.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: ignoring %zu bytes pushed to full TX_FIFO\n",
+ __func__, len - avail);
+ len = avail;
+ }
fifo8_push_all(&s->tx_fifo, buf, len);
}
@@ -472,8 +494,10 @@ static uint8_t xlnx_dp_aux_pop_tx_fifo(XlnxDPState *s)
uint8_t ret;
if (fifo8_is_empty(&s->tx_fifo)) {
- error_report("%s: TX_FIFO underflow", __func__);
- abort();
+ /* Data sheet doesn't specify behaviour here: we choose to return 0 */
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: attempt to read empty TX_FIFO\n",
+ __func__);
+ return 0;
}
ret = fifo8_pop(&s->tx_fifo);
DPRINTF("pop 0x%2.2X from tx_fifo.\n", ret);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 24/25] hw/display/xlnx_dp: Don't abort for unsupported graphics formats
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (22 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 23/25] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 25/25] hw/misc/npcm_clk: Don't divide by zero when calculating frequency Michael Tokarev
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Edgar E. Iglesias,
Philippe Mathieu-Daudé, Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
If the guest writes an invalid or unsupported value to the
AV_BUF_FORMAT register, currently we abort(). Instead, log this as
either a guest error or an unimplemented error and continue.
The existing code treats DP_NL_VID_CB_Y0_CR_Y1 as x8b8g8r8
via a "case 0" that does not use the enum constant name for some
reason; we leave that alone beyond adding a comment about the
weird code.
Documentation of this register seems to be at:
https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/AV_BUF_FORMAT-DISPLAY_PORT-Register
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1415
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20251106145209.1083998-3-peter.maydell@linaro.org
(cherry picked from commit 032333eba77b83dfbd74071cc2971f0bda9a3d4f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index d5c6d19ab5..2cbbfeef59 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -662,14 +662,28 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
case DP_GRAPHIC_BGR888:
s->g_plane.format = PIXMAN_b8g8r8;
break;
+ case DP_GRAPHIC_RGBA5551:
+ case DP_GRAPHIC_RGBA4444:
+ case DP_GRAPHIC_8BPP:
+ case DP_GRAPHIC_4BPP:
+ case DP_GRAPHIC_2BPP:
+ case DP_GRAPHIC_1BPP:
+ qemu_log_mask(LOG_UNIMP, "%s: unimplemented graphic format %u",
+ __func__,
+ s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
+ s->g_plane.format = PIXMAN_r8g8b8a8;
+ break;
default:
- error_report("%s: unsupported graphic format %u", __func__,
- s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
- abort();
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid graphic format %u",
+ __func__,
+ s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
+ s->g_plane.format = PIXMAN_r8g8b8a8;
+ break;
}
switch (s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK) {
case 0:
+ /* This is DP_NL_VID_CB_Y0_CR_Y1 ??? */
s->v_plane.format = PIXMAN_x8b8g8r8;
break;
case DP_NL_VID_Y0_CB_Y1_CR:
@@ -678,10 +692,39 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
case DP_NL_VID_RGBA8880:
s->v_plane.format = PIXMAN_x8b8g8r8;
break;
+ case DP_NL_VID_CR_Y0_CB_Y1:
+ case DP_NL_VID_Y0_CR_Y1_CB:
+ case DP_NL_VID_YV16:
+ case DP_NL_VID_YV24:
+ case DP_NL_VID_YV16CL:
+ case DP_NL_VID_MONO:
+ case DP_NL_VID_YV16CL2:
+ case DP_NL_VID_YUV444:
+ case DP_NL_VID_RGB888:
+ case DP_NL_VID_RGB888_10BPC:
+ case DP_NL_VID_YUV444_10BPC:
+ case DP_NL_VID_YV16CL2_10BPC:
+ case DP_NL_VID_YV16CL_10BPC:
+ case DP_NL_VID_YV16_10BPC:
+ case DP_NL_VID_YV24_10BPC:
+ case DP_NL_VID_Y_ONLY_10BPC:
+ case DP_NL_VID_YV16_420:
+ case DP_NL_VID_YV16CL_420:
+ case DP_NL_VID_YV16CL2_420:
+ case DP_NL_VID_YV16_420_10BPC:
+ case DP_NL_VID_YV16CL_420_10BPC:
+ case DP_NL_VID_YV16CL2_420_10BPC:
+ qemu_log_mask(LOG_UNIMP, "%s: unimplemented video format %u",
+ __func__,
+ s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
+ s->v_plane.format = PIXMAN_x8b8g8r8;
+ break;
default:
- error_report("%s: unsupported video format %u", __func__,
- s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
- abort();
+ qemu_log_mask(LOG_UNIMP, "%s: invalid video format %u",
+ __func__,
+ s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
+ s->v_plane.format = PIXMAN_x8b8g8r8;
+ break;
}
xlnx_dp_recreate_surface(s);
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Stable-7.2.22 25/25] hw/misc/npcm_clk: Don't divide by zero when calculating frequency
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
` (23 preceding siblings ...)
2025-11-22 20:55 ` [Stable-7.2.22 24/25] hw/display/xlnx_dp: Don't abort for unsupported graphics formats Michael Tokarev
@ 2025-11-22 20:55 ` Michael Tokarev
24 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2025-11-22 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Maydell, Philippe Mathieu-Daudé,
Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
If the guest misprograms the PLL registers to request a zero
divisor, we currently fall over with a division by zero:
../../hw/misc/npcm_clk.c:221:14: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/misc/npcm_clk.c:221:14
Thread 1 "qemu-system-aar" received signal SIGFPE, Arithmetic exception.
0x00005555584d8f6d in npcm7xx_clk_update_pll (opaque=0x7fffed159a20) at ../../hw/misc/npcm_clk.c:221
221 freq /= PLLCON_INDV(con) * PLLCON_OTDV1(con) * PLLCON_OTDV2(con);
Avoid this by treating this invalid setting like a stopped clock
(setting freq to 0).
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/549
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20251107150137.1353532-1-peter.maydell@linaro.org
(cherry picked from commit 5fc50b4ec841c8a01e7346c2c804088fc3accb6b)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index bc2b879feb..6d86f54113 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -122,13 +122,14 @@ static void npcm7xx_clk_update_pll(void *opaque)
{
NPCM7xxClockPLLState *s = opaque;
uint32_t con = s->clk->regs[s->reg];
- uint64_t freq;
+ uint64_t freq, freq_div;
/* The PLL is grounded if it is not locked yet. */
if (con & PLLCON_LOKI) {
freq = clock_get_hz(s->clock_in);
freq *= PLLCON_FBDV(con);
- freq /= PLLCON_INDV(con) * PLLCON_OTDV1(con) * PLLCON_OTDV2(con);
+ freq_div = PLLCON_INDV(con) * PLLCON_OTDV1(con) * PLLCON_OTDV2(con);
+ freq = freq_div ? freq / freq_div : 0;
} else {
freq = 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-11-22 21:24 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-22 20:55 [Stable-7.2.22 00/25] Patch Round-up for stable 7.2.22, freeze on 2025-12-01 Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 01/25] target/i386: Fix CR2 handling for non-canonical addresses Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 02/25] i386/cpu: Prevent delivering SIPI during SMM in TCG mode Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 03/25] i386/tcg/smm_helper: Properly apply DR values on SMM entry / exit Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 04/25] async: access bottom half flags with qatomic_read Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 05/25] target/i386: user: do not set up a valid LDT on reset Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 06/25] linux-user: Use correct type for FIBMAP and FIGETBSZ emulation Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 07/25] linux-user: permit sendto() with NULL buf and 0 len Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 08/25] io: add trace event when cancelling TLS handshake Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 09/25] io: release active GSource in TLS channel finalizer Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 10/25] io: move websock resource release to close method Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 11/25] io: fix use after free in websocket handshake code Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 12/25] crypto: stop requiring "key encipherment" usage in x509 certs Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 13/25] block/curl.c: Use explicit long constants in curl_easy_setopt calls Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 14/25] block/curl.c: Fix CURLOPT_VERBOSE parameter type Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 15/25] qio: Add trace points to net_listener Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 16/25] qio: Unwatch before notify in QIONetListener Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 17/25] qio: Remember context of qio_net_listener_set_client_func_full Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 18/25] qio: Protect NetListener callback with mutex Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 19/25] hw/net/e1000e_core: Don't advance desc_offset for NULL buffer RX descriptors Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 20/25] hw/net/e1000e_core: Correct rx oversize packet checks Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 21/25] hw/net/e1000e_core: Adjust e1000e_write_payload_frag_to_rx_buffers() assert Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 22/25] net: pad packets to minimum length in qemu_receive_packet() Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 23/25] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 24/25] hw/display/xlnx_dp: Don't abort for unsupported graphics formats Michael Tokarev
2025-11-22 20:55 ` [Stable-7.2.22 25/25] hw/misc/npcm_clk: Don't divide by zero when calculating frequency Michael Tokarev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).