[Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors

[Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors

[Francisco Iglesias]

Coverity found that the string return by 'object_get_canonical_path' was not
being freed at two locations in the model (CID 1391294 and CID 1391293) and
also that a memset was being called with a value greater than the max of a byte
on the second argument (CID 1391286). This patch corrects this by adding the
freeing of the strings and also changing to memset to zero instead on
descriptor unaligned errors.

Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
---
 hw/dma/xlnx-zdma.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index 14d86c254b..8eea757aff 100644
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
         qemu_log_mask(LOG_GUEST_ERROR,
                       "zdma: unaligned descriptor at %" PRIx64,
                       addr);
-        memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr));
+        memset(buf, 0x0, sizeof(XlnxZDMADescr));
         s->error = true;
         return false;
     }
@@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, unsigned size)
     RegisterInfo *r = &s->regs_info[addr / 4];
 
     if (!r->data) {
+        gchar *path = object_get_canonical_path(OBJECT(s));
         qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n",
-                 object_get_canonical_path(OBJECT(s)),
+                 path,
                  addr);
+        g_free(path);
         ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
         zdma_ch_imr_update_irq(s);
         return 0;
@@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, uint64_t value,
     RegisterInfo *r = &s->regs_info[addr / 4];
 
     if (!r->data) {
+        gchar *path = object_get_canonical_path(OBJECT(s));
         qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n",
-                 object_get_canonical_path(OBJECT(s)),
+                 path,
                  addr, value);
+        g_free(path);
         ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
         zdma_ch_imr_update_irq(s);
         return;
-- 
2.11.0

Re: [Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors

[Edgar E. Iglesias]

On Mon, May 28, 2018 at 08:48:59PM +0200, Francisco Iglesias wrote:
> Coverity found that the string return by 'object_get_canonical_path' was not
> being freed at two locations in the model (CID 1391294 and CID 1391293) and
> also that a memset was being called with a value greater than the max of a byte
> on the second argument (CID 1391286). This patch corrects this by adding the
> freeing of the strings and also changing to memset to zero instead on
> descriptor unaligned errors.

Perhaps this should have been two patches but in any case:

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> 
> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
> ---
>  hw/dma/xlnx-zdma.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
> index 14d86c254b..8eea757aff 100644
> --- a/hw/dma/xlnx-zdma.c
> +++ b/hw/dma/xlnx-zdma.c
> @@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "zdma: unaligned descriptor at %" PRIx64,
>                        addr);
> -        memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr));
> +        memset(buf, 0x0, sizeof(XlnxZDMADescr));
>          s->error = true;
>          return false;
>      }
> @@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, unsigned size)
>      RegisterInfo *r = &s->regs_info[addr / 4];
>  
>      if (!r->data) {
> +        gchar *path = object_get_canonical_path(OBJECT(s));
>          qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n",
> -                 object_get_canonical_path(OBJECT(s)),
> +                 path,
>                   addr);
> +        g_free(path);
>          ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
>          zdma_ch_imr_update_irq(s);
>          return 0;
> @@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, uint64_t value,
>      RegisterInfo *r = &s->regs_info[addr / 4];
>  
>      if (!r->data) {
> +        gchar *path = object_get_canonical_path(OBJECT(s));
>          qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n",
> -                 object_get_canonical_path(OBJECT(s)),
> +                 path,
>                   addr, value);
> +        g_free(path);
>          ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
>          zdma_ch_imr_update_irq(s);
>          return;
> -- 
> 2.11.0
> 

Re: [Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors

[Philippe Mathieu-Daudé]

On 05/28/2018 03:58 PM, Edgar E. Iglesias wrote:
> On Mon, May 28, 2018 at 08:48:59PM +0200, Francisco Iglesias wrote:
>> Coverity found that the string return by 'object_get_canonical_path' was not
>> being freed at two locations in the model (CID 1391294 and CID 1391293) and
>> also that a memset was being called with a value greater than the max of a byte
>> on the second argument (CID 1391286). This patch corrects this by adding the
>> freeing of the strings and also changing to memset to zero instead on
>> descriptor unaligned errors.
> 
> Perhaps this should have been two patches but in any case:
> 
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
>> ---
>>  hw/dma/xlnx-zdma.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
>> index 14d86c254b..8eea757aff 100644
>> --- a/hw/dma/xlnx-zdma.c
>> +++ b/hw/dma/xlnx-zdma.c
>> @@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf)
>>          qemu_log_mask(LOG_GUEST_ERROR,
>>                        "zdma: unaligned descriptor at %" PRIx64,
>>                        addr);
>> -        memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr));
>> +        memset(buf, 0x0, sizeof(XlnxZDMADescr));
>>          s->error = true;
>>          return false;
>>      }
>> @@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, unsigned size)
>>      RegisterInfo *r = &s->regs_info[addr / 4];
>>  
>>      if (!r->data) {
>> +        gchar *path = object_get_canonical_path(OBJECT(s));
>>          qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n",
>> -                 object_get_canonical_path(OBJECT(s)),
>> +                 path,
>>                   addr);
>> +        g_free(path);
>>          ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
>>          zdma_ch_imr_update_irq(s);
>>          return 0;
>> @@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, uint64_t value,
>>      RegisterInfo *r = &s->regs_info[addr / 4];
>>  
>>      if (!r->data) {
>> +        gchar *path = object_get_canonical_path(OBJECT(s));
>>          qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n",
>> -                 object_get_canonical_path(OBJECT(s)),
>> +                 path,
>>                   addr, value);
>> +        g_free(path);
>>          ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true);
>>          zdma_ch_imr_update_irq(s);
>>          return;
>> -- 
>> 2.11.0
>>
> 

Re: [Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors

[Peter Maydell]

On 28 May 2018 at 19:48, Francisco Iglesias <frasse.iglesias@gmail.com> wrote:
> Coverity found that the string return by 'object_get_canonical_path' was not
> being freed at two locations in the model (CID 1391294 and CID 1391293) and
> also that a memset was being called with a value greater than the max of a byte
> on the second argument (CID 1391286). This patch corrects this by adding the
> freeing of the strings and also changing to memset to zero instead on
> descriptor unaligned errors.
>
> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
> ---
>  hw/dma/xlnx-zdma.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)



Applied to target-arm.next, thanks.

-- PMM