| Message ID | 1382795083-28591-1-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 26, 2013 at 9:44 PM, Max Reitz <mreitz@redhat.com> wrote: > > bdrv_open_backing_file() tries to copy the backing file name using > pstrcpy directly after calling bdrv_open() to open the backing file > without checking whether that was actually successful. If it was not, > ps->backing_hd->file will probably be NULL and qemu will crash. > > Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. Reviewed-by: Amos Kong <kongjianjun@gmail.com> > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index 4474012..61795fe 100644 > --- a/block.c > +++ b/block.c > @@ -1005,8 +1005,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > ret = bdrv_open(bs->backing_hd, > *backing_filename ? backing_filename : NULL, options, > back_flags, back_drv, &local_err); > - pstrcpy(bs->backing_file, sizeof(bs->backing_file), > - bs->backing_hd->file->filename); > if (ret < 0) { > bdrv_unref(bs->backing_hd); > bs->backing_hd = NULL; > @@ -1014,6 +1012,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > error_propagate(errp, local_err); > return ret; > } > + pstrcpy(bs->backing_file, sizeof(bs->backing_file), > + bs->backing_hd->file->filename); > return 0; > } > > -- > 1.8.4.1 > >
Le Saturday 26 Oct 2013 à 15:44:43 (+0200), Max Reitz a écrit : > bdrv_open_backing_file() tries to copy the backing file name using > pstrcpy directly after calling bdrv_open() to open the backing file > without checking whether that was actually successful. If it was not, > ps->backing_hd->file will probably be NULL and qemu will crash. > > Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index 4474012..61795fe 100644 > --- a/block.c > +++ b/block.c > @@ -1005,8 +1005,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > ret = bdrv_open(bs->backing_hd, > *backing_filename ? backing_filename : NULL, options, > back_flags, back_drv, &local_err); > - pstrcpy(bs->backing_file, sizeof(bs->backing_file), > - bs->backing_hd->file->filename); > if (ret < 0) { > bdrv_unref(bs->backing_hd); > bs->backing_hd = NULL; > @@ -1014,6 +1012,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) > error_propagate(errp, local_err); > return ret; > } > + pstrcpy(bs->backing_file, sizeof(bs->backing_file), > + bs->backing_hd->file->filename); > return 0; > } > > -- > 1.8.4.1 > > Reviewed-by: Benoit Canet <benoit@irqsave.net>
Am 26.10.2013 um 15:44 hat Max Reitz geschrieben: > bdrv_open_backing_file() tries to copy the backing file name using > pstrcpy directly after calling bdrv_open() to open the backing file > without checking whether that was actually successful. If it was not, > ps->backing_hd->file will probably be NULL and qemu will crash. > > Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Thanks, applied. A test case wouldn't hurt, though. Kevin
diff --git a/block.c b/block.c index 4474012..61795fe 100644 --- a/block.c +++ b/block.c @@ -1005,8 +1005,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) ret = bdrv_open(bs->backing_hd, *backing_filename ? backing_filename : NULL, options, back_flags, back_drv, &local_err); - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - bs->backing_hd->file->filename); if (ret < 0) { bdrv_unref(bs->backing_hd); bs->backing_hd = NULL; @@ -1014,6 +1012,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) error_propagate(errp, local_err); return ret; } + pstrcpy(bs->backing_file, sizeof(bs->backing_file), + bs->backing_hd->file->filename); return 0; }
bdrv_open_backing_file() tries to copy the backing file name using pstrcpy directly after calling bdrv_open() to open the backing file without checking whether that was actually successful. If it was not, ps->backing_hd->file will probably be NULL and qemu will crash. Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)