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;
 
     do {
-- 
2.43.2

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