[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qed: check for header size overflow
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qed: check for header size overflow |
Date: |
Wed, 14 Jan 2015 15:00:16 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 12.01.2015 um 13:31 hat Stefan Hajnoczi geschrieben:
> Header size is denoted in clusters. The maximum cluster size is 64 MB
> but there is no limit on header size. Check for uint32_t overflow in
> case the header size field has a whacky value.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> block/qed.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/block/qed.c b/block/qed.c
> index 80f18d8..d2c6045 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -440,6 +440,12 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict
> *options, int flags,
> s->l2_mask = s->table_nelems - 1;
> s->l1_shift = s->l2_shift + ffs(s->table_nelems) - 1;
>
> + /* Header size calculation must not overflow uint32_t */
> + if ((uint64_t)s->header.cluster_size * s->header.header_size !=
> + s->header.cluster_size * s->header.header_size) {
Generally, I find checks that don't exercise the integer overflow easier
to read, i.e. in this case:
if (s->header.header_size > UINT32_MAX / s->header.cluster_size)
Or even just INT_MAX to be on the safe side. But I'll admit that it's a
matter of taste.
> + return -EINVAL;
> + }
> +
> if ((s->header.features & QED_F_BACKING_FILE)) {
> if ((uint64_t)s->header.backing_filename_offset +
> s->header.backing_filename_size >
Series: Reviewed-by: Kevin Wolf <address@hidden>