* [PULL 1/2] hw/nvme: Use #define to avoid variable length array
2023-09-12 14:26 [PULL 0/2] hw/nvme: updates Klaus Jensen
@ 2023-09-12 14:26 ` Klaus Jensen
2023-09-12 14:26 ` [PULL 2/2] hw/nvme: Avoid dynamic stack allocation Klaus Jensen
2023-09-13 19:17 ` [PULL 0/2] hw/nvme: updates Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Klaus Jensen @ 2023-09-12 14:26 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, qemu-block,
Philippe Mathieu-Daudé, Hanna Reitz, Klaus Jensen,
Kevin Wolf, Keith Busch, Philippe Mathieu-Daudé,
Klaus Jensen
From: Philippe Mathieu-Daudé <philmd@redhat.com>
In nvme_map_sgl() we create an array segment[] whose size is the
'const int SEG_CHUNK_SIZE'. Since this is C, rather than C++, a
"const int foo" is not a true constant, it's merely a variable with a
constant value, and so semantically segment[] is a variable-length
array. Switch SEG_CHUNK_SIZE to a #define so that we can make the
segment[] array truly fixed-size, in the sense that it doesn't
trigger the -Wvla warning.
The codebase has very few VLAs, and if we can get rid of them all we
can make the compiler error on new additions. This is a defensive
measure against security bugs where an on-stack dynamic allocation
isn't correctly size-checked (e.g. CVE-2021-3527).
[PMM: rebased (function has moved file), expand commit message
based on discussion from previous version of patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/nvme/ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 539d27355313..d99a6f5c9a2e 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1045,7 +1045,7 @@ static uint16_t nvme_map_sgl(NvmeCtrl *n, NvmeSg *sg, NvmeSglDescriptor sgl,
* descriptors and segment chain) than the command transfer size, so it is
* not bounded by MDTS.
*/
- const int SEG_CHUNK_SIZE = 256;
+#define SEG_CHUNK_SIZE 256
NvmeSglDescriptor segment[SEG_CHUNK_SIZE], *sgld, *last_sgld;
uint64_t nsgld;
--
2.42.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PULL 2/2] hw/nvme: Avoid dynamic stack allocation
2023-09-12 14:26 [PULL 0/2] hw/nvme: updates Klaus Jensen
2023-09-12 14:26 ` [PULL 1/2] hw/nvme: Use #define to avoid variable length array Klaus Jensen
@ 2023-09-12 14:26 ` Klaus Jensen
2023-09-13 19:17 ` [PULL 0/2] hw/nvme: updates Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Klaus Jensen @ 2023-09-12 14:26 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, qemu-block,
Philippe Mathieu-Daudé, Hanna Reitz, Klaus Jensen,
Kevin Wolf, Keith Busch, Klaus Jensen
From: Peter Maydell <peter.maydell@linaro.org>
Instead of using a variable-length array in nvme_map_prp(),
allocate on the stack with a g_autofree pointer.
The codebase has very few VLAs, and if we can get rid of them all we
can make the compiler error on new additions. This is a defensive
measure against security bugs where an on-stack dynamic allocation
isn't correctly size-checked (e.g. CVE-2021-3527).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
hw/nvme/ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index d99a6f5c9a2e..90687b168ae1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -894,7 +894,7 @@ static uint16_t nvme_map_prp(NvmeCtrl *n, NvmeSg *sg, uint64_t prp1,
len -= trans_len;
if (len) {
if (len > n->page_size) {
- uint64_t prp_list[n->max_prp_ents];
+ g_autofree uint64_t *prp_list = g_new(uint64_t, n->max_prp_ents);
uint32_t nents, prp_trans;
int i = 0;
--
2.42.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PULL 0/2] hw/nvme: updates
2023-09-12 14:26 [PULL 0/2] hw/nvme: updates Klaus Jensen
2023-09-12 14:26 ` [PULL 1/2] hw/nvme: Use #define to avoid variable length array Klaus Jensen
2023-09-12 14:26 ` [PULL 2/2] hw/nvme: Avoid dynamic stack allocation Klaus Jensen
@ 2023-09-13 19:17 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2023-09-13 19:17 UTC (permalink / raw)
To: Klaus Jensen
Cc: Peter Maydell, qemu-devel, Stefan Hajnoczi, Fam Zheng, qemu-block,
Philippe Mathieu-Daudé, Hanna Reitz, Klaus Jensen,
Kevin Wolf, Keith Busch, Klaus Jensen
[-- Attachment #1: Type: text/plain, Size: 115 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread