[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] fw_cfg: assertion to detect memory leak whe
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] fw_cfg: assertion to detect memory leak when adding new data blob |
Date: |
Mon, 16 Mar 2015 20:12:34 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 03/16/15 15:15, Gabriel L. Somlo wrote:
> Currently, fw_cfg_add_bytes_read_callback() does not deal with
> the possibility that the data pointer at the requested key position
> has previously been set, and assumes it will be called exactly once
> for each key value.
>
> This patch introduces an assertion to codify this assumption, and
> insure the data pointer about to be set is NULL at the time the
> function is called, which will prevent the inadvertent leaking of
> data blobs by erroneous multiple calls using the same key value.
>
> Signed-off-by: Gabriel Somlo <address@hidden>
> ---
> hw/nvram/fw_cfg.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 86090f3..5501a97 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -399,6 +399,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s,
> uint16_t key,
> key &= FW_CFG_ENTRY_MASK;
>
> assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX);
> + assert(s->entries[arch][key].data == NULL); /* prevent memory leak */
>
> s->entries[arch][key].data = data;
> s->entries[arch][key].len = (uint32_t)len;
>
I think I agree with the patch (the assert itself), but the comment
could be more precise. I'd simply say "avoid selector key conflict" or
some such. The predicate we want to assert here primarily is "single
assignment of selector key", right? Calling
fw_cfg_add_bytes_read_callback() with the same key is an error
regardless of any leaks (you could call it with a pointer to a static
storage duration object, and it would remain an error just the same).
Thanks
Laszlo
- [Qemu-devel] [PATCH 0/6] fw-cfg: documentation, cleanup, and proposed feature, Gabriel L. Somlo, 2015/03/16
- [Qemu-devel] [PATCH 1/6] fw_cfg: add documentation file (docs/specs/fw_cfg.txt), Gabriel L. Somlo, 2015/03/16
- [Qemu-devel] [PATCH 4/6] fw_cfg: exit with error when dupe fw_cfg file name inserted, Gabriel L. Somlo, 2015/03/16
- [Qemu-devel] [PATCH 5/6] fw_cfg: insert fw_cfg file blobs via qemu cmdline, Gabriel L. Somlo, 2015/03/16
- [Qemu-devel] [PATCH 6/6] qga: RFC: guest-side retrieval of fw_cfg file, Gabriel L. Somlo, 2015/03/16
- [Qemu-devel] [PATCH 3/6] fw_cfg: assertion to detect memory leak when adding new data blob, Gabriel L. Somlo, 2015/03/16
- Re: [Qemu-devel] [PATCH 3/6] fw_cfg: assertion to detect memory leak when adding new data blob,
Laszlo Ersek <=
- [Qemu-devel] [PATCH 2/6] fw_cfg: remove support for guest-side data writes, Gabriel L. Somlo, 2015/03/16
- Re: [Qemu-devel] [PATCH 0/6] fw-cfg: documentation, cleanup, and proposed feature, Patchew Tool, 2015/03/16