[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] qcow2: avoid memcpy(dst, NULL, len)
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH] qcow2: avoid memcpy(dst, NULL, len) |
Date: |
Tue, 13 Sep 2016 15:14:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 13/09/2016 11:15, Stefan Hajnoczi wrote:
> Section "7.1.4 Use of library functions" in the C99 standard says:
>
> If an argument to a function has an invalid value (such as [...]
> a null pointer [...]) [...] the behavior is undefined.
>
> Additionally the "searching and sorting" functions are specified as
> requiring valid pointer values as described in 7.1.4.
>
> This patch fixes the following static analyzer errors:
>
> block/qcow2.c:1807:41: runtime error: null pointer passed as argument 2,
> which is declared to never be null
> block/qcow2-cluster.c:86:26: runtime error: null pointer passed as argument
> 2, which is declared to never be null
>
> Reported-by: Peter Maydell <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> block/qcow2-cluster.c | 4 +++-
> block/qcow2.c | 5 ++++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index f941835..ab0dcdc 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -83,7 +83,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
> min_size,
> }
> memset(new_l1_table, 0, align_offset(new_l1_size2, 512));
>
> - memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
> + if (s->l1_table) {
> + memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
> + }
>
> /* write new table (align to cluster) */
> BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index c079aa8..758a997 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1804,7 +1804,10 @@ static size_t header_ext_add(char *buf, uint32_t
> magic, const void *s,
> .magic = cpu_to_be32(magic),
> .len = cpu_to_be32(len),
> };
> - memcpy(buf + sizeof(QCowExtension), s, len);
> +
> + if (s) {
Should they be "if (len)" and "if (s->l1_size)"?
I think a SIGSEGV is the right thing to do if s->l1_table is NULL but
s->l1_size is non-zero, or if s is NULL but len is non-zero.
Paolo
> + memcpy(buf + sizeof(QCowExtension), s, len);
> + }
>
> return ext_len;
> }
>
- [Qemu-block] [PATCH] qcow2: avoid memcpy(dst, NULL, len), Stefan Hajnoczi, 2016/09/13
- Re: [Qemu-block] [PATCH] qcow2: avoid memcpy(dst, NULL, len), Kevin Wolf, 2016/09/13
- Re: [Qemu-block] [PATCH] qcow2: avoid memcpy(dst, NULL, len),
Paolo Bonzini <=
- Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: avoid memcpy(dst, NULL, len), Eric Blake, 2016/09/13