mirror of
https://git.yoctoproject.org/poky
synced 2026-01-29 21:08:42 +01:00
qemu: upgrade 8.2.3 -> 8.2.7
This includes fix for: CVE-2024-4693, CVE-2024-6505 and CVE-2024-7730 General changelog for 8.2: https://wiki.qemu.org/ChangeLog/8.2 Droped: 0001-target-riscv-kvm-change-KVM_REG_RISCV_FP_F-to-u32.patch 0002-target-riscv-kvm-change-KVM_REG_RISCV_FP_D-to-u64.patch 0003-target-riscv-kvm-change-timer-regs-size-to-u64.patch CVE-2024-4467 and CVE-2024-7409 since already contained the fix. (From OE-Core rev: 7983ad282c37f8c1125da5bab96489e5d0039948) Signed-off-by: Yogita Urade <yogita.urade@windriver.com> Signed-off-by: Steve Sakoman <steve@sakoman.com>
This commit is contained in:
committed by
Steve Sakoman
parent
c6ec0e1bfd
commit
2775596cb2
@@ -40,18 +40,6 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \
|
||||
file://0005-tests-tcg-Check-that-shmat-does-not-break-proc-self-.patch \
|
||||
file://qemu-guest-agent.init \
|
||||
file://qemu-guest-agent.udev \
|
||||
file://CVE-2024-4467-0001.patch \
|
||||
file://CVE-2024-4467-0002.patch \
|
||||
file://CVE-2024-4467-0003.patch \
|
||||
file://CVE-2024-4467-0004.patch \
|
||||
file://CVE-2024-4467-0005.patch \
|
||||
file://CVE-2024-7409-0001.patch \
|
||||
file://CVE-2024-7409-0002.patch \
|
||||
file://CVE-2024-7409-0003.patch \
|
||||
file://CVE-2024-7409-0004.patch \
|
||||
file://0001-target-riscv-kvm-change-KVM_REG_RISCV_FP_F-to-u32.patch \
|
||||
file://0002-target-riscv-kvm-change-KVM_REG_RISCV_FP_D-to-u64.patch \
|
||||
file://0003-target-riscv-kvm-change-timer-regs-size-to-u64.patch \
|
||||
"
|
||||
UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
|
||||
|
||||
@@ -68,7 +56,7 @@ SRC_URI:append:class-native = " \
|
||||
file://0012-linux-user-workaround-for-missing-MAP_SHARED_VALIDAT.patch \
|
||||
"
|
||||
|
||||
SRC_URI[sha256sum] = "dc747fb366809455317601c4876bd1f6829a32a23e83fb76e45ab12c2a569964"
|
||||
SRC_URI[sha256sum] = "1f0604f296ab9acb4854c054764a1ba408643fc299bd54a6500cccfaaca65b55"
|
||||
|
||||
CVE_STATUS[CVE-2007-0998] = "not-applicable-config: The VNC server can expose host files uder some circumstances. We don't enable it by default."
|
||||
|
||||
|
||||
@@ -1,75 +0,0 @@
|
||||
From bbdcc89678daa5cb131ef22a6cd41a5f7f9dcea9 Mon Sep 17 00:00:00 2001
|
||||
From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Date: Fri, 8 Dec 2023 15:38:31 -0300
|
||||
Subject: [PATCH 1/3] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32
|
||||
|
||||
KVM_REG_RISCV_FP_F regs have u32 size according to the API, but by using
|
||||
kvm_riscv_reg_id() in RISCV_FP_F_REG() we're returning u64 sizes when
|
||||
running with TARGET_RISCV64. The most likely reason why no one noticed
|
||||
this is because we're not implementing kvm_cpu_synchronize_state() in
|
||||
RISC-V yet.
|
||||
|
||||
Create a new helper that returns a KVM ID with u32 size and use it in
|
||||
RISCV_FP_F_REG().
|
||||
|
||||
Reported-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Message-ID: <20231208183835.2411523-2-dbarboza@ventanamicro.com>
|
||||
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
|
||||
(cherry picked from commit 49c211ffca00fdf7c0c29072c224e88527a14838)
|
||||
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
||||
|
||||
Upstream-Status: Backport [bbdcc89678daa5cb131ef22a6cd41a5f7f9dcea9]
|
||||
|
||||
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
|
||||
---
|
||||
target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
|
||||
1 file changed, 8 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
|
||||
index c1675158fe..2eef2be86a 100644
|
||||
--- a/target/riscv/kvm/kvm-cpu.c
|
||||
+++ b/target/riscv/kvm/kvm-cpu.c
|
||||
@@ -72,6 +72,11 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
|
||||
return id;
|
||||
}
|
||||
|
||||
+static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
|
||||
+{
|
||||
+ return KVM_REG_RISCV | KVM_REG_SIZE_U32 | type | idx;
|
||||
+}
|
||||
+
|
||||
#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
|
||||
KVM_REG_RISCV_CORE_REG(name))
|
||||
|
||||
@@ -81,7 +86,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
|
||||
#define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_TIMER, \
|
||||
KVM_REG_RISCV_TIMER_REG(name))
|
||||
|
||||
-#define RISCV_FP_F_REG(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_F, idx)
|
||||
+#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
|
||||
|
||||
#define RISCV_FP_D_REG(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
|
||||
|
||||
@@ -586,7 +591,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
|
||||
if (riscv_has_ext(env, RVF)) {
|
||||
uint32_t reg;
|
||||
for (i = 0; i < 32; i++) {
|
||||
- ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
|
||||
+ ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(i), ®);
|
||||
if (ret) {
|
||||
return ret;
|
||||
}
|
||||
@@ -620,7 +625,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
|
||||
uint32_t reg;
|
||||
for (i = 0; i < 32; i++) {
|
||||
reg = env->fpr[i];
|
||||
- ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
|
||||
+ ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(i), ®);
|
||||
if (ret) {
|
||||
return ret;
|
||||
}
|
||||
--
|
||||
2.25.1
|
||||
|
||||
@@ -1,73 +0,0 @@
|
||||
From 125b95d79e746cbab6b72683b3382dd372e38c61 Mon Sep 17 00:00:00 2001
|
||||
From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Date: Fri, 8 Dec 2023 15:38:32 -0300
|
||||
Subject: [PATCH 2/3] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64
|
||||
|
||||
KVM_REG_RISCV_FP_D regs are always u64 size. Using kvm_riscv_reg_id() in
|
||||
RISCV_FP_D_REG() ends up encoding the wrong size if we're running with
|
||||
TARGET_RISCV32.
|
||||
|
||||
Create a new helper that returns a KVM ID with u64 size and use it with
|
||||
RISCV_FP_D_REG().
|
||||
|
||||
Reported-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Message-ID: <20231208183835.2411523-3-dbarboza@ventanamicro.com>
|
||||
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
|
||||
(cherry picked from commit 450bd6618fda3d2e2ab02b2fce1c79efd5b66084)
|
||||
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
||||
|
||||
Upstream-Status: Backport [125b95d79e746cbab6b72683b3382dd372e38c61]
|
||||
|
||||
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
|
||||
---
|
||||
target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
|
||||
1 file changed, 8 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
|
||||
index 2eef2be86a..82ed4455a5 100644
|
||||
--- a/target/riscv/kvm/kvm-cpu.c
|
||||
+++ b/target/riscv/kvm/kvm-cpu.c
|
||||
@@ -77,6 +77,11 @@ static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
|
||||
return KVM_REG_RISCV | KVM_REG_SIZE_U32 | type | idx;
|
||||
}
|
||||
|
||||
+static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
|
||||
+{
|
||||
+ return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
|
||||
+}
|
||||
+
|
||||
#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
|
||||
KVM_REG_RISCV_CORE_REG(name))
|
||||
|
||||
@@ -88,7 +93,7 @@ static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
|
||||
|
||||
#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
|
||||
|
||||
-#define RISCV_FP_D_REG(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
|
||||
+#define RISCV_FP_D_REG(idx) kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
|
||||
|
||||
#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
|
||||
do { \
|
||||
@@ -579,7 +584,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
|
||||
if (riscv_has_ext(env, RVD)) {
|
||||
uint64_t reg;
|
||||
for (i = 0; i < 32; i++) {
|
||||
- ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
|
||||
+ ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(i), ®);
|
||||
if (ret) {
|
||||
return ret;
|
||||
}
|
||||
@@ -613,7 +618,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
|
||||
uint64_t reg;
|
||||
for (i = 0; i < 32; i++) {
|
||||
reg = env->fpr[i];
|
||||
- ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
|
||||
+ ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(i), ®);
|
||||
if (ret) {
|
||||
return ret;
|
||||
}
|
||||
--
|
||||
2.25.1
|
||||
|
||||
@@ -1,107 +0,0 @@
|
||||
From cbae1080988e0f1af0fb4c816205f7647f6de16f Mon Sep 17 00:00:00 2001
|
||||
From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Date: Fri, 8 Dec 2023 15:38:33 -0300
|
||||
Subject: [PATCH 3/3] target/riscv/kvm: change timer regs size to u64
|
||||
|
||||
KVM_REG_RISCV_TIMER regs are always u64 according to the KVM API, but at
|
||||
this moment we'll return u32 regs if we're running a RISCV32 target.
|
||||
|
||||
Use the kvm_riscv_reg_id_u64() helper in RISCV_TIMER_REG() to fix it.
|
||||
|
||||
Reported-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
|
||||
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
|
||||
Message-ID: <20231208183835.2411523-4-dbarboza@ventanamicro.com>
|
||||
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
|
||||
(cherry picked from commit 10f86d1b845087d14b58d65dd2a6e3411d1b6529)
|
||||
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
||||
|
||||
Upstream-Status: Backport [cbae1080988e0f1af0fb4c816205f7647f6de16f]
|
||||
|
||||
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
|
||||
---
|
||||
target/riscv/kvm/kvm-cpu.c | 26 +++++++++++++-------------
|
||||
1 file changed, 13 insertions(+), 13 deletions(-)
|
||||
|
||||
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
|
||||
index 82ed4455a5..ddbe820e10 100644
|
||||
--- a/target/riscv/kvm/kvm-cpu.c
|
||||
+++ b/target/riscv/kvm/kvm-cpu.c
|
||||
@@ -88,7 +88,7 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
|
||||
#define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
|
||||
KVM_REG_RISCV_CSR_REG(name))
|
||||
|
||||
-#define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_TIMER, \
|
||||
+#define RISCV_TIMER_REG(name) kvm_riscv_reg_id_u64(KVM_REG_RISCV_TIMER, \
|
||||
KVM_REG_RISCV_TIMER_REG(name))
|
||||
|
||||
#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
|
||||
@@ -111,17 +111,17 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
|
||||
} \
|
||||
} while (0)
|
||||
|
||||
-#define KVM_RISCV_GET_TIMER(cs, env, name, reg) \
|
||||
+#define KVM_RISCV_GET_TIMER(cs, name, reg) \
|
||||
do { \
|
||||
- int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
|
||||
+ int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(name), ®); \
|
||||
if (ret) { \
|
||||
abort(); \
|
||||
} \
|
||||
} while (0)
|
||||
|
||||
-#define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
|
||||
+#define KVM_RISCV_SET_TIMER(cs, name, reg) \
|
||||
do { \
|
||||
- int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
|
||||
+ int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(name), ®); \
|
||||
if (ret) { \
|
||||
abort(); \
|
||||
} \
|
||||
@@ -649,10 +649,10 @@ static void kvm_riscv_get_regs_timer(CPUState *cs)
|
||||
return;
|
||||
}
|
||||
|
||||
- KVM_RISCV_GET_TIMER(cs, env, time, env->kvm_timer_time);
|
||||
- KVM_RISCV_GET_TIMER(cs, env, compare, env->kvm_timer_compare);
|
||||
- KVM_RISCV_GET_TIMER(cs, env, state, env->kvm_timer_state);
|
||||
- KVM_RISCV_GET_TIMER(cs, env, frequency, env->kvm_timer_frequency);
|
||||
+ KVM_RISCV_GET_TIMER(cs, time, env->kvm_timer_time);
|
||||
+ KVM_RISCV_GET_TIMER(cs, compare, env->kvm_timer_compare);
|
||||
+ KVM_RISCV_GET_TIMER(cs, state, env->kvm_timer_state);
|
||||
+ KVM_RISCV_GET_TIMER(cs, frequency, env->kvm_timer_frequency);
|
||||
|
||||
env->kvm_timer_dirty = true;
|
||||
}
|
||||
@@ -666,8 +666,8 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
|
||||
return;
|
||||
}
|
||||
|
||||
- KVM_RISCV_SET_TIMER(cs, env, time, env->kvm_timer_time);
|
||||
- KVM_RISCV_SET_TIMER(cs, env, compare, env->kvm_timer_compare);
|
||||
+ KVM_RISCV_SET_TIMER(cs, time, env->kvm_timer_time);
|
||||
+ KVM_RISCV_SET_TIMER(cs, compare, env->kvm_timer_compare);
|
||||
|
||||
/*
|
||||
* To set register of RISCV_TIMER_REG(state) will occur a error from KVM
|
||||
@@ -676,7 +676,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
|
||||
* TODO If KVM changes, adapt here.
|
||||
*/
|
||||
if (env->kvm_timer_state) {
|
||||
- KVM_RISCV_SET_TIMER(cs, env, state, env->kvm_timer_state);
|
||||
+ KVM_RISCV_SET_TIMER(cs, state, env->kvm_timer_state);
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -685,7 +685,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
|
||||
* during the migration.
|
||||
*/
|
||||
if (migration_is_running(migrate_get_current()->state)) {
|
||||
- KVM_RISCV_GET_TIMER(cs, env, frequency, reg);
|
||||
+ KVM_RISCV_GET_TIMER(cs, frequency, reg);
|
||||
if (reg != env->kvm_timer_frequency) {
|
||||
error_report("Dst Hosts timer frequency != Src Hosts");
|
||||
}
|
||||
--
|
||||
2.25.1
|
||||
|
||||
@@ -1,112 +0,0 @@
|
||||
From bd385a5298d7062668e804d73944d52aec9549f1 Mon Sep 17 00:00:00 2001
|
||||
From: Kevin Wolf <kwolf@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 08:29:04 +0000
|
||||
Subject: [PATCH] qcow2: Don't open data_file with BDRV_O_NO_IO
|
||||
|
||||
One use case for 'qemu-img info' is verifying that untrusted images
|
||||
don't reference an unwanted external file, be it as a backing file or an
|
||||
external data file. To make sure that calling 'qemu-img info' can't
|
||||
already have undesired side effects with a malicious image, just don't
|
||||
open the data file at all with BDRV_O_NO_IO. If nothing ever tries to do
|
||||
I/O, we don't need to have it open.
|
||||
|
||||
This changes the output of iotests case 061, which used 'qemu-img info'
|
||||
to show that opening an image with an invalid data file fails. After
|
||||
this patch, it succeeds. Replace this part of the test with a qemu-io
|
||||
call, but keep the final 'qemu-img info' to show that the invalid data
|
||||
file is correctly displayed in the output.
|
||||
|
||||
Fixes: CVE-2024-4467
|
||||
Cc: qemu-stable@nongnu.org
|
||||
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
||||
Reviewed-by: Eric Blake <eblake@redhat.com>
|
||||
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
|
||||
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
|
||||
|
||||
CVE: CVE-2024-4667
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/bd385a5298d7062668e804d73944d52aec9549f1]
|
||||
|
||||
Signed-off-by: Yogita Urade <yogita.urade@windriver.com>
|
||||
---
|
||||
block/qcow2.c | 17 ++++++++++++++++-
|
||||
tests/qemu-iotests/061 | 6 ++++--
|
||||
tests/qemu-iotests/061.out | 8 ++++++--
|
||||
3 files changed, 26 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/block/qcow2.c b/block/qcow2.c
|
||||
index 13e032bd5..7af7c0bee 100644
|
||||
--- a/block/qcow2.c
|
||||
+++ b/block/qcow2.c
|
||||
@@ -1636,7 +1636,22 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
|
||||
goto fail;
|
||||
}
|
||||
|
||||
- if (open_data_file) {
|
||||
+ if (open_data_file && (flags & BDRV_O_NO_IO)) {
|
||||
+ /*
|
||||
+ * Don't open the data file for 'qemu-img info' so that it can be used
|
||||
+ * to verify that an untrusted qcow2 image doesn't refer to external
|
||||
+ * files.
|
||||
+ *
|
||||
+ * Note: This still makes has_data_file() return true.
|
||||
+ */
|
||||
+ if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
|
||||
+ s->data_file = NULL;
|
||||
+ } else {
|
||||
+ s->data_file = bs->file;
|
||||
+ }
|
||||
+ qdict_extract_subqdict(options, NULL, "data-file.");
|
||||
+ qdict_del(options, "data-file");
|
||||
+ } else if (open_data_file) {
|
||||
/* Open external data file */
|
||||
bdrv_graph_co_rdunlock();
|
||||
s->data_file = bdrv_co_open_child(NULL, options, "data-file", bs,
|
||||
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
|
||||
index 53c7d428e..b71ac097d 100755
|
||||
--- a/tests/qemu-iotests/061
|
||||
+++ b/tests/qemu-iotests/061
|
||||
@@ -326,12 +326,14 @@ $QEMU_IMG amend -o "data_file=foo" "$TEST_IMG"
|
||||
echo
|
||||
_make_test_img -o "compat=1.1,data_file=$TEST_IMG.data" 64M
|
||||
$QEMU_IMG amend -o "data_file=foo" "$TEST_IMG"
|
||||
-_img_info --format-specific
|
||||
+$QEMU_IO -c "read 0 4k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
|
||||
+$QEMU_IO -c "open -o data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" -c "read 0 4k" | _filter_qemu_io
|
||||
TEST_IMG="data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" _img_info --format-specific --image-opts
|
||||
|
||||
echo
|
||||
$QEMU_IMG amend -o "data_file=" --image-opts "data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG"
|
||||
-_img_info --format-specific
|
||||
+$QEMU_IO -c "read 0 4k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
|
||||
+$QEMU_IO -c "open -o data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" -c "read 0 4k" | _filter_qemu_io
|
||||
TEST_IMG="data-file.filename=$TEST_IMG.data,file.filename=$TEST_IMG" _img_info --format-specific --image-opts
|
||||
|
||||
echo
|
||||
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
|
||||
index 139fc6817..24c33add7 100644
|
||||
--- a/tests/qemu-iotests/061.out
|
||||
+++ b/tests/qemu-iotests/061.out
|
||||
@@ -545,7 +545,9 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
|
||||
qemu-img: data-file can only be set for images that use an external data file
|
||||
|
||||
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 data_file=TEST_DIR/t.IMGFMT.data
|
||||
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open 'foo': No such file or directory
|
||||
+qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'foo': No such file or directory
|
||||
+read 4096/4096 bytes at offset 0
|
||||
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
||||
image: TEST_DIR/t.IMGFMT
|
||||
file format: IMGFMT
|
||||
virtual size: 64 MiB (67108864 bytes)
|
||||
@@ -560,7 +562,9 @@ Format specific information:
|
||||
corrupt: false
|
||||
extended l2: false
|
||||
|
||||
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'data-file' is required for this image
|
||||
+qemu-io: can't open device TEST_DIR/t.IMGFMT: 'data-file' is required for this image
|
||||
+read 4096/4096 bytes at offset 0
|
||||
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
|
||||
image: TEST_DIR/t.IMGFMT
|
||||
file format: IMGFMT
|
||||
virtual size: 64 MiB (67108864 bytes)
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,55 +0,0 @@
|
||||
From 2eb42a728d27a43fdcad5f37d3f65706ce6deba5 Mon Sep 17 00:00:00 2001
|
||||
From: Kevin Wolf <kwolf@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 09:35:24 +0000
|
||||
Subject: [PATCH] iotests/244: Don't store data-file with protocol in image
|
||||
|
||||
We want to disable filename parsing for data files because it's too easy
|
||||
to abuse in malicious image files. Make the test ready for the change by
|
||||
passing the data file explicitly in command line options.
|
||||
|
||||
Cc: qemu-stable@nongnu.org
|
||||
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
||||
Reviewed-by: Eric Blake <eblake@redhat.com>
|
||||
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
|
||||
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
|
||||
|
||||
CVE: CVE-2024-4467
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/2eb42a728d27a43fdcad5f37d3f65706ce6deba5]
|
||||
|
||||
Signed-off-by: Yogita Urade <yogita.urade@windriver.com>
|
||||
---
|
||||
tests/qemu-iotests/244 | 19 ++++++++++++++++---
|
||||
1 file changed, 16 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244
|
||||
index 3e61fa25b..bb9cc6512 100755
|
||||
--- a/tests/qemu-iotests/244
|
||||
+++ b/tests/qemu-iotests/244
|
||||
@@ -215,9 +215,22 @@ $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n -C "$TEST_IMG.src" "$TEST_IMG"
|
||||
$QEMU_IMG compare -f $IMGFMT -F $IMGFMT "$TEST_IMG.src" "$TEST_IMG"
|
||||
|
||||
# blkdebug doesn't support copy offloading, so this tests the error path
|
||||
-$QEMU_IMG amend -f $IMGFMT -o "data_file=blkdebug::$TEST_IMG.data" "$TEST_IMG"
|
||||
-$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n -C "$TEST_IMG.src" "$TEST_IMG"
|
||||
-$QEMU_IMG compare -f $IMGFMT -F $IMGFMT "$TEST_IMG.src" "$TEST_IMG"
|
||||
+test_img_with_blkdebug="json:{
|
||||
+ 'driver': 'qcow2',
|
||||
+ 'file': {
|
||||
+ 'driver': 'file',
|
||||
+ 'filename': '$TEST_IMG'
|
||||
+ },
|
||||
+ 'data-file': {
|
||||
+ 'driver': 'blkdebug',
|
||||
+ 'image': {
|
||||
+ 'driver': 'file',
|
||||
+ 'filename': '$TEST_IMG.data'
|
||||
+ }
|
||||
+ }
|
||||
+}"
|
||||
+$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n -C "$TEST_IMG.src" "$test_img_with_blkdebug"
|
||||
+$QEMU_IMG compare -f $IMGFMT -F $IMGFMT "$TEST_IMG.src" "$test_img_with_blkdebug"
|
||||
|
||||
echo
|
||||
echo "=== Flushing should flush the data file ==="
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,57 +0,0 @@
|
||||
From 7e1110664ecbc4826f3c978ccb06b6c1bce823e6 Mon Sep 17 00:00:00 2001
|
||||
From: Kevin Wolf <kwolf@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 10:24:58 +0000
|
||||
Subject: [PATCH] iotests/270: Don't store data-file with json: prefix in image
|
||||
|
||||
We want to disable filename parsing for data files because it's too easy
|
||||
to abuse in malicious image files. Make the test ready for the change by
|
||||
passing the data file explicitly in command line options.
|
||||
|
||||
Cc: qemu-stable@nongnu.org
|
||||
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
||||
Reviewed-by: Eric Blake <eblake@redhat.com>
|
||||
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
|
||||
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
|
||||
|
||||
CVE: CVE-2024-4467
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/7e1110664ecbc4826f3c978ccb06b6c1bce823e6]
|
||||
|
||||
Signed-off-by: Yogita Urade <yogita.urade@windriver.com>
|
||||
---
|
||||
tests/qemu-iotests/270 | 14 +++++++++++---
|
||||
1 file changed, 11 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/tests/qemu-iotests/270 b/tests/qemu-iotests/270
|
||||
index 74352342d..c37b674aa 100755
|
||||
--- a/tests/qemu-iotests/270
|
||||
+++ b/tests/qemu-iotests/270
|
||||
@@ -60,8 +60,16 @@ _make_test_img -o cluster_size=2M,data_file="$TEST_IMG.orig" \
|
||||
# "write" 2G of data without using any space.
|
||||
# (qemu-img create does not like it, though, because null-co does not
|
||||
# support image creation.)
|
||||
-$QEMU_IMG amend -o data_file="json:{'driver':'null-co',,'size':'4294967296'}" \
|
||||
- "$TEST_IMG"
|
||||
+test_img_with_null_data="json:{
|
||||
+ 'driver': '$IMGFMT',
|
||||
+ 'file': {
|
||||
+ 'filename': '$TEST_IMG'
|
||||
+ },
|
||||
+ 'data-file': {
|
||||
+ 'driver': 'null-co',
|
||||
+ 'size':'4294967296'
|
||||
+ }
|
||||
+}"
|
||||
|
||||
# This gives us a range of:
|
||||
# 2^31 - 512 + 768 - 1 = 2^31 + 255 > 2^31
|
||||
@@ -74,7 +82,7 @@ $QEMU_IMG amend -o data_file="json:{'driver':'null-co',,'size':'4294967296'}" \
|
||||
# on L2 boundaries, we need large L2 tables; hence the cluster size of
|
||||
# 2 MB. (Anything from 256 kB should work, though, because then one L2
|
||||
# table covers 8 GB.)
|
||||
-$QEMU_IO -c "write 768 $((2 ** 31 - 512))" "$TEST_IMG" | _filter_qemu_io
|
||||
+$QEMU_IO -c "write 768 $((2 ** 31 - 512))" "$test_img_with_null_data" | _filter_qemu_io
|
||||
|
||||
_check_test_img
|
||||
|
||||
--
|
||||
2.40.0
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,239 +0,0 @@
|
||||
From 7ead946998610657d38d1a505d5f25300d4ca613 Mon Sep 17 00:00:00 2001
|
||||
From: Kevin Wolf <kwolf@redhat.com>
|
||||
Date: Thu, 25 Apr 2024 14:56:02 +0000
|
||||
Subject: [PATCH] block: Parse filenames only when explicitly requested
|
||||
|
||||
When handling image filenames from legacy options such as -drive or from
|
||||
tools, these filenames are parsed for protocol prefixes, including for
|
||||
the json:{} pseudo-protocol.
|
||||
|
||||
This behaviour is intended for filenames that come directly from the
|
||||
command line and for backing files, which may come from the image file
|
||||
itself. Higher level management tools generally take care to verify that
|
||||
untrusted images don't contain a bad (or any) backing file reference;
|
||||
'qemu-img info' is a suitable tool for this.
|
||||
|
||||
However, for other files that can be referenced in images, such as
|
||||
qcow2 data files or VMDK extents, the string from the image file is
|
||||
usually not verified by management tools - and 'qemu-img info' wouldn't
|
||||
be suitable because in contrast to backing files, it already opens these
|
||||
other referenced files. So here the string should be interpreted as a
|
||||
literal local filename. More complex configurations need to be specified
|
||||
explicitly on the command line or in QMP...
|
||||
|
||||
CVE: CVE-2024-4467
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/7ead946998610657d38d1a505d5f25300d4ca613]
|
||||
|
||||
Signed-off-by: Yogita Urade <yogita.urade@windriver.com>
|
||||
---
|
||||
block.c | 94 ++++++++++++++++++++++++++++++++++-----------------------
|
||||
1 file changed, 57 insertions(+), 37 deletions(-)
|
||||
|
||||
diff --git a/block.c b/block.c
|
||||
index 25e1ebc60..f3cb32cd7 100644
|
||||
--- a/block.c
|
||||
+++ b/block.c
|
||||
@@ -86,6 +86,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
|
||||
BlockDriverState *parent,
|
||||
const BdrvChildClass *child_class,
|
||||
BdrvChildRole child_role,
|
||||
+ bool parse_filename,
|
||||
Error **errp);
|
||||
|
||||
static bool bdrv_recurse_has_child(BlockDriverState *bs,
|
||||
@@ -2047,7 +2048,8 @@ static void parse_json_protocol(QDict *options, const char **pfilename,
|
||||
* block driver has been specified explicitly.
|
||||
*/
|
||||
static int bdrv_fill_options(QDict **options, const char *filename,
|
||||
- int *flags, Error **errp)
|
||||
+ int *flags, bool allow_parse_filename,
|
||||
+ Error **errp)
|
||||
{
|
||||
const char *drvname;
|
||||
bool protocol = *flags & BDRV_O_PROTOCOL;
|
||||
@@ -2089,7 +2091,7 @@ static int bdrv_fill_options(QDict **options, const char *filename,
|
||||
if (protocol && filename) {
|
||||
if (!qdict_haskey(*options, "filename")) {
|
||||
qdict_put_str(*options, "filename", filename);
|
||||
- parse_filename = true;
|
||||
+ parse_filename = allow_parse_filename;
|
||||
} else {
|
||||
error_setg(errp, "Can't specify 'file' and 'filename' options at "
|
||||
"the same time");
|
||||
@@ -3675,7 +3677,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
|
||||
}
|
||||
|
||||
backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
|
||||
- &child_of_bds, bdrv_backing_role(bs), errp);
|
||||
+ &child_of_bds, bdrv_backing_role(bs), true,
|
||||
+ errp);
|
||||
if (!backing_hd) {
|
||||
bs->open_flags |= BDRV_O_NO_BACKING;
|
||||
error_prepend(errp, "Could not open backing file: ");
|
||||
@@ -3712,7 +3715,8 @@ free_exit:
|
||||
static BlockDriverState *
|
||||
bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
|
||||
BlockDriverState *parent, const BdrvChildClass *child_class,
|
||||
- BdrvChildRole child_role, bool allow_none, Error **errp)
|
||||
+ BdrvChildRole child_role, bool allow_none,
|
||||
+ bool parse_filename, Error **errp)
|
||||
{
|
||||
BlockDriverState *bs = NULL;
|
||||
QDict *image_options;
|
||||
@@ -3743,7 +3747,8 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
|
||||
}
|
||||
|
||||
bs = bdrv_open_inherit(filename, reference, image_options, 0,
|
||||
- parent, child_class, child_role, errp);
|
||||
+ parent, child_class, child_role, parse_filename,
|
||||
+ errp);
|
||||
if (!bs) {
|
||||
goto done;
|
||||
}
|
||||
@@ -3753,6 +3758,33 @@ done:
|
||||
return bs;
|
||||
}
|
||||
|
||||
+static BdrvChild *bdrv_open_child_common(const char *filename,
|
||||
+ QDict *options, const char *bdref_key,
|
||||
+ BlockDriverState *parent,
|
||||
+ const BdrvChildClass *child_class,
|
||||
+ BdrvChildRole child_role,
|
||||
+ bool allow_none, bool parse_filename,
|
||||
+ Error **errp)
|
||||
+{
|
||||
+ BlockDriverState *bs;
|
||||
+ BdrvChild *child;
|
||||
+
|
||||
+ GLOBAL_STATE_CODE();
|
||||
+
|
||||
+ bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
|
||||
+ child_role, allow_none, parse_filename, errp);
|
||||
+ if (bs == NULL) {
|
||||
+ return NULL;
|
||||
+ }
|
||||
+
|
||||
+ bdrv_graph_wrlock();
|
||||
+ child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
|
||||
+ errp);
|
||||
+ bdrv_graph_wrunlock();
|
||||
+
|
||||
+ return child;
|
||||
+}
|
||||
+
|
||||
/*
|
||||
* Opens a disk image whose options are given as BlockdevRef in another block
|
||||
* device's options.
|
||||
@@ -3778,31 +3810,15 @@ BdrvChild *bdrv_open_child(const char *filename,
|
||||
BdrvChildRole child_role,
|
||||
bool allow_none, Error **errp)
|
||||
{
|
||||
- BlockDriverState *bs;
|
||||
- BdrvChild *child;
|
||||
- AioContext *ctx;
|
||||
-
|
||||
- GLOBAL_STATE_CODE();
|
||||
-
|
||||
- bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_class,
|
||||
- child_role, allow_none, errp);
|
||||
- if (bs == NULL) {
|
||||
- return NULL;
|
||||
- }
|
||||
-
|
||||
- bdrv_graph_wrlock();
|
||||
- ctx = bdrv_get_aio_context(bs);
|
||||
- aio_context_acquire(ctx);
|
||||
- child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
|
||||
- errp);
|
||||
- aio_context_release(ctx);
|
||||
- bdrv_graph_wrunlock();
|
||||
-
|
||||
- return child;
|
||||
+ return bdrv_open_child_common(filename, options, bdref_key, parent,
|
||||
+ child_class, child_role, allow_none, false,
|
||||
+ errp);
|
||||
}
|
||||
|
||||
/*
|
||||
- * Wrapper on bdrv_open_child() for most popular case: open primary child of bs.
|
||||
+ * This does mostly the same as bdrv_open_child(), but for opening the primary
|
||||
+ * child of a node. A notable difference from bdrv_open_child() is that it
|
||||
+ * enables filename parsing for protocol names (including json:).
|
||||
*
|
||||
* The caller must hold the lock of the main AioContext and no other AioContext.
|
||||
* @parent can move to a different AioContext in this function. Callers must
|
||||
@@ -3819,8 +3835,8 @@ int bdrv_open_file_child(const char *filename,
|
||||
role = parent->drv->is_filter ?
|
||||
(BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY) : BDRV_CHILD_IMAGE;
|
||||
|
||||
- if (!bdrv_open_child(filename, options, bdref_key, parent,
|
||||
- &child_of_bds, role, false, errp))
|
||||
+ if (!bdrv_open_child_common(filename, options, bdref_key, parent,
|
||||
+ &child_of_bds, role, false, true, errp))
|
||||
{
|
||||
return -EINVAL;
|
||||
}
|
||||
@@ -3865,7 +3881,8 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)
|
||||
|
||||
}
|
||||
|
||||
- bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, 0, errp);
|
||||
+ bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, 0, false,
|
||||
+ errp);
|
||||
obj = NULL;
|
||||
qobject_unref(obj);
|
||||
visit_free(v);
|
||||
@@ -3962,7 +3979,7 @@ static BlockDriverState * no_coroutine_fn
|
||||
bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
|
||||
int flags, BlockDriverState *parent,
|
||||
const BdrvChildClass *child_class, BdrvChildRole child_role,
|
||||
- Error **errp)
|
||||
+ bool parse_filename, Error **errp)
|
||||
{
|
||||
int ret;
|
||||
BlockBackend *file = NULL;
|
||||
@@ -4011,9 +4028,11 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
|
||||
}
|
||||
|
||||
/* json: syntax counts as explicit options, as if in the QDict */
|
||||
- parse_json_protocol(options, &filename, &local_err);
|
||||
- if (local_err) {
|
||||
- goto fail;
|
||||
+ if (parse_filename) {
|
||||
+ parse_json_protocol(options, &filename, &local_err);
|
||||
+ if (local_err) {
|
||||
+ goto fail;
|
||||
+ }
|
||||
}
|
||||
|
||||
bs->explicit_options = qdict_clone_shallow(options);
|
||||
@@ -4038,7 +4057,8 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
|
||||
parent->open_flags, parent->options);
|
||||
}
|
||||
|
||||
- ret = bdrv_fill_options(&options, filename, &flags, &local_err);
|
||||
+ ret = bdrv_fill_options(&options, filename, &flags, parse_filename,
|
||||
+ &local_err);
|
||||
if (ret < 0) {
|
||||
goto fail;
|
||||
}
|
||||
@@ -4107,7 +4127,7 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
|
||||
|
||||
file_bs = bdrv_open_child_bs(filename, options, "file", bs,
|
||||
&child_of_bds, BDRV_CHILD_IMAGE,
|
||||
- true, &local_err);
|
||||
+ true, true, &local_err);
|
||||
if (local_err) {
|
||||
goto fail;
|
||||
}
|
||||
@@ -4270,7 +4290,7 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference,
|
||||
GLOBAL_STATE_CODE();
|
||||
|
||||
return bdrv_open_inherit(filename, reference, options, flags, NULL,
|
||||
- NULL, 0, errp);
|
||||
+ NULL, 0, true, errp);
|
||||
}
|
||||
|
||||
/* Return true if the NULL-terminated @list contains @str */
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,167 +0,0 @@
|
||||
From fb1c2aaa981e0a2fa6362c9985f1296b74f055ac Mon Sep 17 00:00:00 2001
|
||||
From: Eric Blake <eblake@redhat.com>
|
||||
Date: Wed, 7 Aug 2024 08:50:01 -0500
|
||||
Subject: [PATCH] nbd/server: Plumb in new args to nbd_client_add()
|
||||
|
||||
Upcoming patches to fix a CVE need to track an opaque pointer passed
|
||||
in by the owner of a client object, as well as request for a time
|
||||
limit on how fast negotiation must complete. Prepare for that by
|
||||
changing the signature of nbd_client_new() and adding an accessor to
|
||||
get at the opaque pointer, although for now the two servers
|
||||
(qemu-nbd.c and blockdev-nbd.c) do not change behavior even though
|
||||
they pass in a new default timeout value.
|
||||
|
||||
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
Message-ID: <20240807174943.771624-11-eblake@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
[eblake: s/LIMIT/MAX_SECS/ as suggested by Dan]
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
|
||||
CVE: CVE-2024-7409
|
||||
|
||||
Upstream-Status: Backport [https://github.com/qemu/qemu/commit/fb1c2aaa981e0a2fa6362c9985f1296b74f055ac]
|
||||
|
||||
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
|
||||
---
|
||||
blockdev-nbd.c | 6 ++++--
|
||||
include/block/nbd.h | 11 ++++++++++-
|
||||
nbd/server.c | 20 +++++++++++++++++---
|
||||
qemu-nbd.c | 4 +++-
|
||||
4 files changed, 34 insertions(+), 7 deletions(-)
|
||||
|
||||
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
|
||||
index 213012435..267a1de90 100644
|
||||
--- a/blockdev-nbd.c
|
||||
+++ b/blockdev-nbd.c
|
||||
@@ -64,8 +64,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
|
||||
nbd_update_server_watch(nbd_server);
|
||||
|
||||
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
|
||||
- nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz,
|
||||
- nbd_blockdev_client_closed);
|
||||
+ /* TODO - expose handshake timeout as QMP option */
|
||||
+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
|
||||
+ nbd_server->tlscreds, nbd_server->tlsauthz,
|
||||
+ nbd_blockdev_client_closed, NULL);
|
||||
}
|
||||
|
||||
static void nbd_update_server_watch(NBDServerData *s)
|
||||
diff --git a/include/block/nbd.h b/include/block/nbd.h
|
||||
index 4e7bd6342..1d4d65922 100644
|
||||
--- a/include/block/nbd.h
|
||||
+++ b/include/block/nbd.h
|
||||
@@ -33,6 +33,12 @@ typedef struct NBDMetaContexts NBDMetaContexts;
|
||||
|
||||
extern const BlockExportDriver blk_exp_nbd;
|
||||
|
||||
+/*
|
||||
+ * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must
|
||||
+ * succeed at NBD_OPT_GO before being forcefully dropped as too slow.
|
||||
+ */
|
||||
+#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
|
||||
+
|
||||
/* Handshake phase structs - this struct is passed on the wire */
|
||||
|
||||
typedef struct NBDOption {
|
||||
@@ -403,9 +409,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp);
|
||||
NBDExport *nbd_export_find(const char *name);
|
||||
|
||||
void nbd_client_new(QIOChannelSocket *sioc,
|
||||
+ uint32_t handshake_max_secs,
|
||||
QCryptoTLSCreds *tlscreds,
|
||||
const char *tlsauthz,
|
||||
- void (*close_fn)(NBDClient *, bool));
|
||||
+ void (*close_fn)(NBDClient *, bool),
|
||||
+ void *owner);
|
||||
+void *nbd_client_owner(NBDClient *client);
|
||||
void nbd_client_get(NBDClient *client);
|
||||
void nbd_client_put(NBDClient *client);
|
||||
|
||||
diff --git a/nbd/server.c b/nbd/server.c
|
||||
index 091b57119..f8881936e 100644
|
||||
--- a/nbd/server.c
|
||||
+++ b/nbd/server.c
|
||||
@@ -124,12 +124,14 @@ struct NBDMetaContexts {
|
||||
struct NBDClient {
|
||||
int refcount; /* atomic */
|
||||
void (*close_fn)(NBDClient *client, bool negotiated);
|
||||
+ void *owner;
|
||||
|
||||
QemuMutex lock;
|
||||
|
||||
NBDExport *exp;
|
||||
QCryptoTLSCreds *tlscreds;
|
||||
char *tlsauthz;
|
||||
+ uint32_t handshake_max_secs;
|
||||
QIOChannelSocket *sioc; /* The underlying data channel */
|
||||
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
|
||||
|
||||
@@ -3160,6 +3162,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
|
||||
|
||||
qemu_co_mutex_init(&client->send_lock);
|
||||
|
||||
+ /* TODO - utilize client->handshake_max_secs */
|
||||
if (nbd_negotiate(client, &local_err)) {
|
||||
if (local_err) {
|
||||
error_report_err(local_err);
|
||||
@@ -3174,14 +3177,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
|
||||
}
|
||||
|
||||
/*
|
||||
- * Create a new client listener using the given channel @sioc.
|
||||
+ * Create a new client listener using the given channel @sioc and @owner.
|
||||
* Begin servicing it in a coroutine. When the connection closes, call
|
||||
- * @close_fn with an indication of whether the client completed negotiation.
|
||||
+ * @close_fn with an indication of whether the client completed negotiation
|
||||
+ * within @handshake_max_secs seconds (0 for unbounded).
|
||||
*/
|
||||
void nbd_client_new(QIOChannelSocket *sioc,
|
||||
+ uint32_t handshake_max_secs,
|
||||
QCryptoTLSCreds *tlscreds,
|
||||
const char *tlsauthz,
|
||||
- void (*close_fn)(NBDClient *, bool))
|
||||
+ void (*close_fn)(NBDClient *, bool),
|
||||
+ void *owner)
|
||||
{
|
||||
NBDClient *client;
|
||||
Coroutine *co;
|
||||
@@ -3194,13 +3200,21 @@ void nbd_client_new(QIOChannelSocket *sioc,
|
||||
object_ref(OBJECT(client->tlscreds));
|
||||
}
|
||||
client->tlsauthz = g_strdup(tlsauthz);
|
||||
+ client->handshake_max_secs = handshake_max_secs;
|
||||
client->sioc = sioc;
|
||||
qio_channel_set_delay(QIO_CHANNEL(sioc), false);
|
||||
object_ref(OBJECT(client->sioc));
|
||||
client->ioc = QIO_CHANNEL(sioc);
|
||||
object_ref(OBJECT(client->ioc));
|
||||
client->close_fn = close_fn;
|
||||
+ client->owner = owner;
|
||||
|
||||
co = qemu_coroutine_create(nbd_co_client_start, client);
|
||||
qemu_coroutine_enter(co);
|
||||
}
|
||||
+
|
||||
+void *
|
||||
+nbd_client_owner(NBDClient *client)
|
||||
+{
|
||||
+ return client->owner;
|
||||
+}
|
||||
diff --git a/qemu-nbd.c b/qemu-nbd.c
|
||||
index 186e6468b..5fa399c0b 100644
|
||||
--- a/qemu-nbd.c
|
||||
+++ b/qemu-nbd.c
|
||||
@@ -389,7 +389,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
|
||||
|
||||
nb_fds++;
|
||||
nbd_update_server_watch();
|
||||
- nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed);
|
||||
+ /* TODO - expose handshake timeout as command line option */
|
||||
+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
|
||||
+ tlscreds, tlsauthz, nbd_client_closed, NULL);
|
||||
}
|
||||
|
||||
static void nbd_update_server_watch(void)
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,175 +0,0 @@
|
||||
From c8a76dbd90c2f48df89b75bef74917f90a59b623 Mon Sep 17 00:00:00 2001
|
||||
From: Eric Blake <eblake@redhat.com>
|
||||
Date: Tue, 6 Aug 2024 13:53:00 -0500
|
||||
Subject: [PATCH] nbd/server: CVE-2024-7409: Cap default max-connections to 100
|
||||
|
||||
Allowing an unlimited number of clients to any web service is a recipe
|
||||
for a rudimentary denial of service attack: the client merely needs to
|
||||
open lots of sockets without closing them, until qemu no longer has
|
||||
any more fds available to allocate.
|
||||
|
||||
For qemu-nbd, we default to allowing only 1 connection unless more are
|
||||
explicitly asked for (-e or --shared); this was historically picked as
|
||||
a nice default (without an explicit -t, a non-persistent qemu-nbd goes
|
||||
away after a client disconnects, without needing any additional
|
||||
follow-up commands), and we are not going to change that interface now
|
||||
(besides, someday we want to point people towards qemu-storage-daemon
|
||||
instead of qemu-nbd).
|
||||
|
||||
But for qemu proper, and the newer qemu-storage-daemon, the QMP
|
||||
nbd-server-start command has historically had a default of unlimited
|
||||
number of connections, in part because unlike qemu-nbd it is
|
||||
inherently persistent until nbd-server-stop. Allowing multiple client
|
||||
sockets is particularly useful for clients that can take advantage of
|
||||
MULTI_CONN (creating parallel sockets to increase throughput),
|
||||
although known clients that do so (such as libnbd's nbdcopy) typically
|
||||
use only 8 or 16 connections (the benefits of scaling diminish once
|
||||
more sockets are competing for kernel attention). Picking a number
|
||||
large enough for typical use cases, but not unlimited, makes it
|
||||
slightly harder for a malicious client to perform a denial of service
|
||||
merely by opening lots of connections withot progressing through the
|
||||
handshake.
|
||||
|
||||
This change does not eliminate CVE-2024-7409 on its own, but reduces
|
||||
the chance for fd exhaustion or unlimited memory usage as an attack
|
||||
surface. On the other hand, by itself, it makes it more obvious that
|
||||
with a finite limit, we have the problem of an unauthenticated client
|
||||
holding 100 fds opened as a way to block out a legitimate client from
|
||||
being able to connect; thus, later patches will further add timeouts
|
||||
to reject clients that are not making progress.
|
||||
|
||||
This is an INTENTIONAL change in behavior, and will break any client
|
||||
of nbd-server-start that was not passing an explicit max-connections
|
||||
parameter, yet expects more than 100 simultaneous connections. We are
|
||||
not aware of any such client (as stated above, most clients aware of
|
||||
MULTI_CONN get by just fine on 8 or 16 connections, and probably cope
|
||||
with later connections failing by relying on the earlier connections;
|
||||
libvirt has not yet been passing max-connections, but generally
|
||||
creates NBD servers with the intent for a single client for the sake
|
||||
of live storage migration; meanwhile, the KubeSAN project anticipates
|
||||
a large cluster sharing multiple clients [up to 8 per node, and up to
|
||||
100 nodes in a cluster], but it currently uses qemu-nbd with an
|
||||
explicit --shared=0 rather than qemu-storage-daemon with
|
||||
nbd-server-start).
|
||||
|
||||
We considered using a deprecation period (declare that omitting
|
||||
max-parameters is deprecated, and make it mandatory in 3 releases -
|
||||
then we don't need to pick an arbitrary default); that has zero risk
|
||||
of breaking any apps that accidentally depended on more than 100
|
||||
connections, and where such breakage might not be noticed under unit
|
||||
testing but only under the larger loads of production usage. But it
|
||||
does not close the denial-of-service hole until far into the future,
|
||||
and requires all apps to change to add the parameter even if 100 was
|
||||
good enough. It also has a drawback that any app (like libvirt) that
|
||||
is accidentally relying on an unlimited default should seriously
|
||||
consider their own CVE now, at which point they are going to change to
|
||||
pass explicit max-connections sooner than waiting for 3 qemu releases.
|
||||
Finally, if our changed default breaks an app, that app can always
|
||||
pass in an explicit max-parameters with a larger value.
|
||||
|
||||
It is also intentional that the HMP interface to nbd-server-start is
|
||||
not changed to expose max-connections (any client needing to fine-tune
|
||||
things should be using QMP).
|
||||
|
||||
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
Message-ID: <20240807174943.771624-12-eblake@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
[ericb: Expand commit message to summarize Dan's argument for why we
|
||||
break corner-case back-compat behavior without a deprecation period]
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
|
||||
CVE: CVE-2024-7409
|
||||
|
||||
Upstream-Status: Backport [https://github.com/qemu/qemu/commit/c8a76dbd90c2f48df89b75bef74917f90a59b623]
|
||||
|
||||
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
|
||||
---
|
||||
block/monitor/block-hmp-cmds.c | 3 ++-
|
||||
blockdev-nbd.c | 8 ++++++++
|
||||
include/block/nbd.h | 7 +++++++
|
||||
qapi/block-export.json | 4 ++--
|
||||
4 files changed, 19 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
|
||||
index c729cbf1e..78a697585 100644
|
||||
--- a/block/monitor/block-hmp-cmds.c
|
||||
+++ b/block/monitor/block-hmp-cmds.c
|
||||
@@ -415,7 +415,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
|
||||
goto exit;
|
||||
}
|
||||
|
||||
- nbd_server_start(addr, NULL, NULL, 0, &local_err);
|
||||
+ nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS,
|
||||
+ &local_err);
|
||||
qapi_free_SocketAddress(addr);
|
||||
if (local_err != NULL) {
|
||||
goto exit;
|
||||
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
|
||||
index 267a1de90..24ba5382d 100644
|
||||
--- a/blockdev-nbd.c
|
||||
+++ b/blockdev-nbd.c
|
||||
@@ -170,6 +170,10 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
|
||||
|
||||
void nbd_server_start_options(NbdServerOptions *arg, Error **errp)
|
||||
{
|
||||
+ if (!arg->has_max_connections) {
|
||||
+ arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
|
||||
+ }
|
||||
+
|
||||
nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz,
|
||||
arg->max_connections, errp);
|
||||
}
|
||||
@@ -182,6 +186,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
|
||||
{
|
||||
SocketAddress *addr_flat = socket_address_flatten(addr);
|
||||
|
||||
+ if (!has_max_connections) {
|
||||
+ max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
|
||||
+ }
|
||||
+
|
||||
nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp);
|
||||
qapi_free_SocketAddress(addr_flat);
|
||||
}
|
||||
diff --git a/include/block/nbd.h b/include/block/nbd.h
|
||||
index 1d4d65922..d4f8b21ae 100644
|
||||
--- a/include/block/nbd.h
|
||||
+++ b/include/block/nbd.h
|
||||
@@ -39,6 +39,13 @@ extern const BlockExportDriver blk_exp_nbd;
|
||||
*/
|
||||
#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
|
||||
|
||||
+/*
|
||||
+ * NBD_DEFAULT_MAX_CONNECTIONS: Number of client sockets to allow at
|
||||
+ * once; must be large enough to allow a MULTI_CONN-aware client like
|
||||
+ * nbdcopy to create its typical number of 8-16 sockets.
|
||||
+ */
|
||||
+#define NBD_DEFAULT_MAX_CONNECTIONS 100
|
||||
+
|
||||
/* Handshake phase structs - this struct is passed on the wire */
|
||||
|
||||
typedef struct NBDOption {
|
||||
diff --git a/qapi/block-export.json b/qapi/block-export.json
|
||||
index 7874a49ba..1d255d77e 100644
|
||||
--- a/qapi/block-export.json
|
||||
+++ b/qapi/block-export.json
|
||||
@@ -28,7 +28,7 @@
|
||||
# @max-connections: The maximum number of connections to allow at the
|
||||
# same time, 0 for unlimited. Setting this to 1 also stops the
|
||||
# server from advertising multiple client support (since 5.2;
|
||||
-# default: 0)
|
||||
+# default: 100)
|
||||
#
|
||||
# Since: 4.2
|
||||
##
|
||||
@@ -63,7 +63,7 @@
|
||||
# @max-connections: The maximum number of connections to allow at the
|
||||
# same time, 0 for unlimited. Setting this to 1 also stops the
|
||||
# server from advertising multiple client support (since 5.2;
|
||||
-# default: 0).
|
||||
+# default: 100).
|
||||
#
|
||||
# Returns: error if the server is already running.
|
||||
#
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,126 +0,0 @@
|
||||
From b9b72cb3ce15b693148bd09cef7e50110566d8a0 Mon Sep 17 00:00:00 2001
|
||||
From: Eric Blake <eblake@redhat.com>
|
||||
Date: Thu, 8 Aug 2024 16:05:08 -0500
|
||||
Subject: [PATCH] nbd/server: CVE-2024-7409: Drop non-negotiating clients
|
||||
|
||||
A client that opens a socket but does not negotiate is merely hogging
|
||||
qemu's resources (an open fd and a small amount of memory); and a
|
||||
malicious client that can access the port where NBD is listening can
|
||||
attempt a denial of service attack by intentionally opening and
|
||||
abandoning lots of unfinished connections. The previous patch put a
|
||||
default bound on the number of such ongoing connections, but once that
|
||||
limit is hit, no more clients can connect (including legitimate ones).
|
||||
The solution is to insist that clients complete handshake within a
|
||||
reasonable time limit, defaulting to 10 seconds. A client that has
|
||||
not successfully completed NBD_OPT_GO by then (including the case of
|
||||
where the client didn't know TLS credentials to even reach the point
|
||||
of NBD_OPT_GO) is wasting our time and does not deserve to stay
|
||||
connected. Later patches will allow fine-tuning the limit away from
|
||||
the default value (including disabling it for doing integration
|
||||
testing of the handshake process itself).
|
||||
|
||||
Note that this patch in isolation actually makes it more likely to see
|
||||
qemu SEGV after nbd-server-stop, as any client socket still connected
|
||||
when the server shuts down will now be closed after 10 seconds rather
|
||||
than at the client's whims. That will be addressed in the next patch.
|
||||
|
||||
For a demo of this patch in action:
|
||||
$ qemu-nbd -f raw -r -t -e 10 file &
|
||||
$ nbdsh --opt-mode -c '
|
||||
H = list()
|
||||
for i in range(20):
|
||||
print(i)
|
||||
H.insert(i, nbd.NBD())
|
||||
H[i].set_opt_mode(True)
|
||||
H[i].connect_uri("nbd://localhost")
|
||||
'
|
||||
$ kill $!
|
||||
|
||||
where later connections get to start progressing once earlier ones are
|
||||
forcefully dropped for taking too long, rather than hanging.
|
||||
|
||||
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
Message-ID: <20240807174943.771624-13-eblake@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
[eblake: rebase to changes earlier in series, reduce scope of timer]
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
|
||||
CVE: CVE-2024-7409
|
||||
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/b9b72cb3ce15b693148bd09cef7e50110566d8a0]
|
||||
|
||||
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
|
||||
---
|
||||
nbd/server.c | 28 +++++++++++++++++++++++++++-
|
||||
nbd/trace-events | 1 +
|
||||
2 files changed, 28 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/nbd/server.c b/nbd/server.c
|
||||
index f8881936e..6155e329a 100644
|
||||
--- a/nbd/server.c
|
||||
+++ b/nbd/server.c
|
||||
@@ -3155,22 +3155,48 @@ static void nbd_client_receive_next_request(NBDClient *client)
|
||||
}
|
||||
}
|
||||
|
||||
+static void nbd_handshake_timer_cb(void *opaque)
|
||||
+{
|
||||
+ QIOChannel *ioc = opaque;
|
||||
+
|
||||
+ trace_nbd_handshake_timer_cb();
|
||||
+ qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
|
||||
+}
|
||||
+
|
||||
static coroutine_fn void nbd_co_client_start(void *opaque)
|
||||
{
|
||||
NBDClient *client = opaque;
|
||||
Error *local_err = NULL;
|
||||
+ QEMUTimer *handshake_timer = NULL;
|
||||
|
||||
qemu_co_mutex_init(&client->send_lock);
|
||||
|
||||
- /* TODO - utilize client->handshake_max_secs */
|
||||
+ /*
|
||||
+ * Create a timer to bound the time spent in negotiation. If the
|
||||
+ * timer expires, it is likely nbd_negotiate will fail because the
|
||||
+ * socket was shutdown.
|
||||
+ */
|
||||
+ if (client->handshake_max_secs > 0) {
|
||||
+ handshake_timer = aio_timer_new(qemu_get_aio_context(),
|
||||
+ QEMU_CLOCK_REALTIME,
|
||||
+ SCALE_NS,
|
||||
+ nbd_handshake_timer_cb,
|
||||
+ client->sioc);
|
||||
+ timer_mod(handshake_timer,
|
||||
+ qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
|
||||
+ client->handshake_max_secs * NANOSECONDS_PER_SECOND);
|
||||
+ }
|
||||
+
|
||||
if (nbd_negotiate(client, &local_err)) {
|
||||
if (local_err) {
|
||||
error_report_err(local_err);
|
||||
}
|
||||
+ timer_free(handshake_timer);
|
||||
client_close(client, false);
|
||||
return;
|
||||
}
|
||||
|
||||
+ timer_free(handshake_timer);
|
||||
WITH_QEMU_LOCK_GUARD(&client->lock) {
|
||||
nbd_client_receive_next_request(client);
|
||||
}
|
||||
diff --git a/nbd/trace-events b/nbd/trace-events
|
||||
index 00ae3216a..cbd0a4ab7 100644
|
||||
--- a/nbd/trace-events
|
||||
+++ b/nbd/trace-events
|
||||
@@ -76,6 +76,7 @@ nbd_co_receive_request_payload_received(uint64_t cookie, uint64_t len) "Payload
|
||||
nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client sent non-compliant write without payload flag: from=0x%" PRIx64 ", len=0x%" PRIx64
|
||||
nbd_co_receive_align_compliance(const char *op, uint64_t from, uint64_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx64 ", align=0x%" PRIx32
|
||||
nbd_trip(void) "Reading request"
|
||||
+nbd_handshake_timer_cb(void) "client took too long to negotiate"
|
||||
|
||||
# client-connection.c
|
||||
nbd_connect_thread_sleep(uint64_t timeout) "timeout %" PRIu64
|
||||
--
|
||||
2.40.0
|
||||
@@ -1,164 +0,0 @@
|
||||
From 3e7ef738c8462c45043a1d39f702a0990406a3b3 Mon Sep 17 00:00:00 2001
|
||||
From: Eric Blake <eblake@redhat.com>
|
||||
Date: Wed, 7 Aug 2024 12:23:13 -0500
|
||||
Subject: [PATCH] nbd/server: CVE-2024-7409: Close stray clients at server-stop
|
||||
|
||||
A malicious client can attempt to connect to an NBD server, and then
|
||||
intentionally delay progress in the handshake, including if it does
|
||||
not know the TLS secrets. Although the previous two patches reduce
|
||||
this behavior by capping the default max-connections parameter and
|
||||
killing slow clients, they did not eliminate the possibility of a
|
||||
client waiting to close the socket until after the QMP nbd-server-stop
|
||||
command is executed, at which point qemu would SEGV when trying to
|
||||
dereference the NULL nbd_server global which is no longer present.
|
||||
This amounts to a denial of service attack. Worse, if another NBD
|
||||
server is started before the malicious client disconnects, I cannot
|
||||
rule out additional adverse effects when the old client interferes
|
||||
with the connection count of the new server (although the most likely
|
||||
is a crash due to an assertion failure when checking
|
||||
nbd_server->connections > 0).
|
||||
|
||||
For environments without this patch, the CVE can be mitigated by
|
||||
ensuring (such as via a firewall) that only trusted clients can
|
||||
connect to an NBD server. Note that using frameworks like libvirt
|
||||
that ensure that TLS is used and that nbd-server-stop is not executed
|
||||
while any trusted clients are still connected will only help if there
|
||||
is also no possibility for an untrusted client to open a connection
|
||||
but then stall on the NBD handshake.
|
||||
|
||||
Given the previous patches, it would be possible to guarantee that no
|
||||
clients remain connected by having nbd-server-stop sleep for longer
|
||||
than the default handshake deadline before finally freeing the global
|
||||
nbd_server object, but that could make QMP non-responsive for a long
|
||||
time. So intead, this patch fixes the problem by tracking all client
|
||||
sockets opened while the server is running, and forcefully closing any
|
||||
such sockets remaining without a completed handshake at the time of
|
||||
nbd-server-stop, then waiting until the coroutines servicing those
|
||||
sockets notice the state change. nbd-server-stop now has a second
|
||||
AIO_WAIT_WHILE_UNLOCKED (the first is indirectly through the
|
||||
blk_exp_close_all_type() that disconnects all clients that completed
|
||||
handshakes), but forced socket shutdown is enough to progress the
|
||||
coroutines and quickly tear down all clients before the server is
|
||||
freed, thus finally fixing the CVE.
|
||||
|
||||
This patch relies heavily on the fact that nbd/server.c guarantees
|
||||
that it only calls nbd_blockdev_client_closed() from the main loop
|
||||
(see the assertion in nbd_client_put() and the hoops used in
|
||||
nbd_client_put_nonzero() to achieve that); if we did not have that
|
||||
guarantee, we would also need a mutex protecting our accesses of the
|
||||
list of connections to survive re-entrancy from independent iothreads.
|
||||
|
||||
Although I did not actually try to test old builds, it looks like this
|
||||
problem has existed since at least commit 862172f45c (v2.12.0, 2017) -
|
||||
even back when that patch started using a QIONetListener to handle
|
||||
listening on multiple sockets, nbd_server_free() was already unaware
|
||||
that the nbd_blockdev_client_closed callback can be reached later by a
|
||||
client thread that has not completed handshakes (and therefore the
|
||||
client's socket never got added to the list closed in
|
||||
nbd_export_close_all), despite that patch intentionally tearing down
|
||||
the QIONetListener to prevent new clients.
|
||||
|
||||
Reported-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
|
||||
Fixes: CVE-2024-7409
|
||||
CC: qemu-stable@nongnu.org
|
||||
Signed-off-by: Eric Blake <eblake@redhat.com>
|
||||
Message-ID: <20240807174943.771624-14-eblake@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
|
||||
CVE: CVE-2024-7409
|
||||
|
||||
Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/3e7ef738c8462c45043a1d39f702a0990406a3b3]
|
||||
|
||||
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
|
||||
---
|
||||
blockdev-nbd.c | 35 ++++++++++++++++++++++++++++++++++-
|
||||
1 file changed, 34 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
|
||||
index 24ba5382d..f73409ae4 100644
|
||||
--- a/blockdev-nbd.c
|
||||
+++ b/blockdev-nbd.c
|
||||
@@ -21,12 +21,18 @@
|
||||
#include "io/channel-socket.h"
|
||||
#include "io/net-listener.h"
|
||||
|
||||
+typedef struct NBDConn {
|
||||
+ QIOChannelSocket *cioc;
|
||||
+ QLIST_ENTRY(NBDConn) next;
|
||||
+} NBDConn;
|
||||
+
|
||||
typedef struct NBDServerData {
|
||||
QIONetListener *listener;
|
||||
QCryptoTLSCreds *tlscreds;
|
||||
char *tlsauthz;
|
||||
uint32_t max_connections;
|
||||
uint32_t connections;
|
||||
+ QLIST_HEAD(, NBDConn) conns;
|
||||
} NBDServerData;
|
||||
|
||||
static NBDServerData *nbd_server;
|
||||
@@ -51,6 +57,14 @@ int nbd_server_max_connections(void)
|
||||
|
||||
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
|
||||
{
|
||||
+ NBDConn *conn = nbd_client_owner(client);
|
||||
+
|
||||
+ assert(qemu_in_main_thread() && nbd_server);
|
||||
+
|
||||
+ object_unref(OBJECT(conn->cioc));
|
||||
+ QLIST_REMOVE(conn, next);
|
||||
+ g_free(conn);
|
||||
+
|
||||
nbd_client_put(client);
|
||||
assert(nbd_server->connections > 0);
|
||||
nbd_server->connections--;
|
||||
@@ -60,14 +74,20 @@ static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
|
||||
static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
|
||||
gpointer opaque)
|
||||
{
|
||||
+ NBDConn *conn = g_new0(NBDConn, 1);
|
||||
+
|
||||
+ assert(qemu_in_main_thread() && nbd_server);
|
||||
nbd_server->connections++;
|
||||
+ object_ref(OBJECT(cioc));
|
||||
+ conn->cioc = cioc;
|
||||
+ QLIST_INSERT_HEAD(&nbd_server->conns, conn, next);
|
||||
nbd_update_server_watch(nbd_server);
|
||||
|
||||
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
|
||||
/* TODO - expose handshake timeout as QMP option */
|
||||
nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
|
||||
nbd_server->tlscreds, nbd_server->tlsauthz,
|
||||
- nbd_blockdev_client_closed, NULL);
|
||||
+ nbd_blockdev_client_closed, conn);
|
||||
}
|
||||
|
||||
static void nbd_update_server_watch(NBDServerData *s)
|
||||
@@ -81,12 +101,25 @@ static void nbd_update_server_watch(NBDServerData *s)
|
||||
|
||||
static void nbd_server_free(NBDServerData *server)
|
||||
{
|
||||
+ NBDConn *conn, *tmp;
|
||||
+
|
||||
if (!server) {
|
||||
return;
|
||||
}
|
||||
|
||||
+ /*
|
||||
+ * Forcefully close the listener socket, and any clients that have
|
||||
+ * not yet disconnected on their own.
|
||||
+ */
|
||||
qio_net_listener_disconnect(server->listener);
|
||||
object_unref(OBJECT(server->listener));
|
||||
+ QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) {
|
||||
+ qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH,
|
||||
+ NULL);
|
||||
+ }
|
||||
+
|
||||
+ AIO_WAIT_WHILE_UNLOCKED(NULL, server->connections > 0);
|
||||
+
|
||||
if (server->tlscreds) {
|
||||
object_unref(OBJECT(server->tlscreds));
|
||||
}
|
||||
--
|
||||
2.40.0
|
||||
Reference in New Issue
Block a user