Re: [PATCH 0/2] Enable -Wvla, forbidding use of variable length arrays
On Wed, 31 Jan 2024 at 14:56, Thomas Huth wrote: > There's still a vla left in the ppc kvm code: > > https://gitlab.com/thuth/qemu/-/jobs/6063230079#L2005 > > ../target/ppc/kvm.c: In function ‘kvmppc_save_htab’: > ../target/ppc/kvm.c:2691:5: error: ISO C90 forbids variable length array > ‘buf’ [-Werror=vla] > 2691 | uint8_t buf[bufsize]; >| ^~~ > ../target/ppc/kvm.c: In function ‘kvmppc_read_hptes’: > ../target/ppc/kvm.c:2773:9: error: ISO C90 forbids variable length array > ‘buf’ [-Werror=vla] > 2773 | char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64]; >| ^~~~ > cc1: all warnings being treated as errors Thanks for catching that -- it being in code built only on ppc hosts I missed it. kvm_ppc_save_htab() is called twice, and in both cases the bufsize passed in is MAX_KVM_BUF_SIZE. So we could drop that argument and have the buf[] array always be MAX_KVM_BUF_SIZE. kvmppc_read_hptes() does this: int m = n < HPTES_PER_GROUP ? n : HPTES_PER_GROUP; char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64]; HPTES_PER_GROUP is 8 and HASH_PTE_SIZE_64 is 16, so we aren't saving many bytes of stack by trying to make the buf smaller based on the value of n. So we could have the buf always be [sizeof(*hdr) + HPTES_PER_GROUP * HASH_PTE_SIZE_64]. thanks -- PMM
Re: [PATCH 0/2] Enable -Wvla, forbidding use of variable length arrays
On 25/01/2024 18.32, Peter Maydell wrote: For a while now I've had an on-and-off-again campaign to get rid of the handful of uses of C variable-length-array syntax in our codebase. The rationale for this is that if the array size can be controlled by the guest and we don't get the size limit checking right, this is an easy to exploit security issue. (An example problem of this kind from the past is CVE-2021-3527). Forbidding them entirely is a defensive measure against further bugs of this kind. I submitted a bunch of patches to this effect last year, and the result is we're now down to just a single use of VLAs, in a test program. This patchset removes that last VLA usage, and enables -Wvla in our warning options, so that we will catch any future attempts to use this C feature. thanks -- PMM Peter Maydell (2): tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array meson: Enable -Wvla meson.build | 1 + tests/qtest/xlnx-versal-trng-test.c | 19 +++ 2 files changed, 12 insertions(+), 8 deletions(-) There's still a vla left in the ppc kvm code: https://gitlab.com/thuth/qemu/-/jobs/6063230079#L2005 ../target/ppc/kvm.c: In function ‘kvmppc_save_htab’: ../target/ppc/kvm.c:2691:5: error: ISO C90 forbids variable length array ‘buf’ [-Werror=vla] 2691 | uint8_t buf[bufsize]; | ^~~ ../target/ppc/kvm.c: In function ‘kvmppc_read_hptes’: ../target/ppc/kvm.c:2773:9: error: ISO C90 forbids variable length array ‘buf’ [-Werror=vla] 2773 | char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64]; | ^~~~ cc1: all warnings being treated as errors Thomas
Re: [PATCH 0/2] Enable -Wvla, forbidding use of variable length arrays
On 1/26/24 03:32, Peter Maydell wrote: Peter Maydell (2): tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array meson: Enable -Wvla Reviewed-by: Richard Henderson r~
[PATCH 0/2] Enable -Wvla, forbidding use of variable length arrays
For a while now I've had an on-and-off-again campaign to get rid of the handful of uses of C variable-length-array syntax in our codebase. The rationale for this is that if the array size can be controlled by the guest and we don't get the size limit checking right, this is an easy to exploit security issue. (An example problem of this kind from the past is CVE-2021-3527). Forbidding them entirely is a defensive measure against further bugs of this kind. I submitted a bunch of patches to this effect last year, and the result is we're now down to just a single use of VLAs, in a test program. This patchset removes that last VLA usage, and enables -Wvla in our warning options, so that we will catch any future attempts to use this C feature. thanks -- PMM Peter Maydell (2): tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array meson: Enable -Wvla meson.build | 1 + tests/qtest/xlnx-versal-trng-test.c | 19 +++ 2 files changed, 12 insertions(+), 8 deletions(-) -- 2.34.1
