19.03.2014 12:19, Deepak Kathayat wrote: > Signed-off-by: Deepak Kathayat <deepak.m...@gmail.com> > --- > The len variable is a signed integer whereas the backing file name > length in the image header is unsigned. Therefore, it may > overflow. Furthermore, backing file name length cannot be > zero. These two cases must be handled explicitly. > block/qcow2.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 945c9d6..7b6f65c 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -625,6 +625,11 @@ static int qcow2_open(BlockDriverState *bs, QDict > *options, int flags, > /* read the backing file name */ > if (header.backing_file_offset != 0) { > len = header.backing_file_size; > + if (len <= 0) { > + error_setg(errp, "Invalid backing file name length: %d", len); > + ret = -EINVAL; > + goto fail; > + } > if (len > 1023) { > len = 1023; > }
A better fix has been implemented meanwhile, as a part of input format validation series: commit 6d33e8e7dc9d40ea105feed4b39caa3e641569e8 Author: Kevin Wolf <kw...@redhat.com> Date: Wed Mar 26 13:05:47 2014 +0100 qcow2: Fix backing file name length check len could become negative and would pass the check then. Nothing bad happened because bdrv_pread() happens to return an error for negative length values, but make variables for sizes unsigned anyway. This patch also changes the behaviour to error out on invalid lengths instead of silently truncating it to 1023. Thanks, /mjt