| Message ID | 20240221162636.173136-2-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace variable length arrays in ppc KVM code | expand |
On Wed, 21 Feb 2024 at 16:26, Thomas Huth <thuth@redhat.com> wrote: > > To be able to compile QEMU with -Wvla (to prevent potential security > issues), we need to get rid of the variable length array in the > kvmppc_save_htab() function. Replace it with a heap allocation instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > target/ppc/kvm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 26fa9d0575..e7e39c3091 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -2688,7 +2688,7 @@ int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp) > int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns) > { > int64_t starttime = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > - uint8_t buf[bufsize]; > + g_autofree uint8_t *buf = g_malloc(bufsize); > ssize_t rc; > This works, so Reviewed-by: Peter Maydell <peter.maydell@linaro.org> but you could also drop the bufsize argument, because there are only two callers and they both pass MAX_KVM_BUF_SIZE, and then declare the array as fixed size with "uint8_t buf[MAX_KVM_BUF_SIZE]". thanks -- PMM
On 21/02/2024 17.29, Peter Maydell wrote: > On Wed, 21 Feb 2024 at 16:26, Thomas Huth <thuth@redhat.com> wrote: >> >> To be able to compile QEMU with -Wvla (to prevent potential security >> issues), we need to get rid of the variable length array in the >> kvmppc_save_htab() function. Replace it with a heap allocation instead. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> target/ppc/kvm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c >> index 26fa9d0575..e7e39c3091 100644 >> --- a/target/ppc/kvm.c >> +++ b/target/ppc/kvm.c >> @@ -2688,7 +2688,7 @@ int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp) >> int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns) >> { >> int64_t starttime = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); >> - uint8_t buf[bufsize]; >> + g_autofree uint8_t *buf = g_malloc(bufsize); >> ssize_t rc; >> > > This works, so > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > but you could also drop the bufsize argument, because there are only > two callers and they both pass MAX_KVM_BUF_SIZE, and then declare the > array as fixed size with "uint8_t buf[MAX_KVM_BUF_SIZE]". Yes, that's an alternative ... my thinking was that MAX_KVM_BUF_SIZE = 2048 is already a rather big buffer which should maybe rather be allocated on the heap than the stack? But I don't mind too much, so if ppc folks prefer the stack allocation, I can change the patch, too. Thomas
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 26fa9d0575..e7e39c3091 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -2688,7 +2688,7 @@ int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp) int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns) { int64_t starttime = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); - uint8_t buf[bufsize]; + g_autofree uint8_t *buf = g_malloc(bufsize); ssize_t rc; do {
To be able to compile QEMU with -Wvla (to prevent potential security issues), we need to get rid of the variable length array in the kvmppc_save_htab() function. Replace it with a heap allocation instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- target/ppc/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)