RE: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM

[Tong Ho]

Hi Peter,

I will follow up with patches to fix the memory leaks.

Where can I get a copy of the Coverity reports that have the 10 issues you 
indicated?

Thanks,
Tong

-----Original Message-----
From: Peter Maydell <peter.mayd...@linaro.org> 
Sent: Saturday, October 2, 2021 3:28 AM
To: Tong Ho <to...@xilinx.com>
Cc: qemu-arm <qemu-...@nongnu.org>; QEMU Developers <qemu-devel@nongnu.org>; 
Alistair Francis <alist...@alistair23.me>; Edgar E. Iglesias 
<edgar.igles...@gmail.com>
Subject: Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM

On Fri, 17 Sept 2021 at 06:24, Tong Ho <tong...@xilinx.com> wrote:
>
> This series implements the Xilinx eFUSE and BBRAM devices for the 
> Versal and ZynqMP product families.
>
> Furthermore, both new devices are connected to the xlnx-versal-virt 
> board and the xlnx-zcu102 board.
>
> See changes in docs/system/arm/xlnx-versal-virt.rst for detail.

Hi -- now this has landed upstream, Coverity points out a lot of memory leaks 
on error or logging paths, where the code does things like:

*** CID 1464071:  Resource leaks  (RESOURCE_LEAK)
/qemu/hw/nvram/xlnx-versal-efuse-ctrl.c: 628 in efuse_ctrl_reg_write()
622         dev = reg_array->mem.owner;
623         assert(dev);
624
625         s = XLNX_VERSAL_EFUSE_CTRL(dev);
626
627         if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
>>>     CID 1464071:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by 
>>> "object_get_canonical_path((Object *)s)" leaks it.
628             qemu_log_mask(LOG_GUEST_ERROR,
629                           "%s[reg_0x%02lx]: Attempt to write
locked register.\n",
630                           object_get_canonical_path(OBJECT(s)), (long)addr);
631         } else {
632             register_write_memory(opaque, addr, data, size);
633         }

You need to free the memory here. A good pattern is how it's done in 
xlnx-zynqmp-can.c with g_autofree:

    if (ARRAY_FIELD_EX32(s->regs, SOFTWARE_RESET_REGISTER, SRST)) {
        g_autofree char *path = object_get_canonical_path(OBJECT(s));

        qemu_log_mask(LOG_GUEST_ERROR, "%s: Attempting to transfer data while"
                      " data while controller is in reset mode.\n",
                      path);
        return false;
    }

Could somebody send some followup patches that fix all of these, please? (There 
are 10 coverity issues, covering probably all of the places where 
object_get_canonical_path() is used in this series.)

thanks
-- PMM