On Mon, Mar 27, 2017 at 08:58:28PM +0200, Markus Armbruster wrote: > Markus Armbruster <arm...@redhat.com> writes: > > > Max Reitz <mre...@redhat.com> writes: > > > >> On 27.03.2017 18:10, Max Reitz wrote: > >>> On 27.03.2017 15:26, Markus Armbruster wrote: > >>>> qemu_rbd_open() neglects to check pool and image are present. > >>>> Reproducer: > >>>> > >>>> $ qemu-system-x86_64 -nodefaults -drive if=none,driver=rbd,pool=p > >>>> Segmentation fault (core dumped) > >>>> $ qemu-system-x86_64 -nodefaults -drive if=none,driver=rbd,image=i > >>>> qemu-system-x86_64: -drive if=none,driver=rbd,image=i: error opening > >>>> pool (null) > >>>> > >>>> Doesn't affect -drive with file=..., because qemu_rbd_parse_filename() > >>>> always sets both pool and image. > >>>> > >>>> Doesn't affect -blockdev, because pool and image are mandatory in the > >>>> QAPI schema. > >>>> > >>>> Fix by adding the missing checks. > >>>> > >>>> Signed-off-by: Markus Armbruster <arm...@redhat.com> > >>>> Reviewed-by: Eric Blake <ebl...@redhat.com> > >>>> --- > >>>> block/rbd.c | 10 +++++++--- > >>>> 1 file changed, 7 insertions(+), 3 deletions(-) > >>> > >>> Reviewed-by: Max Reitz <mre...@redhat.com> > >> > >> That said, don't we have a similar issue with qemu_rbd_create()? It too > >> doesn't check whether those options are given but I guess they're just > >> as mandatory. > > > > Looks like it. I'll try to stick a fix into v4. > > Hmm, ignorant question: how can I reach qemu_rbd_create() without going > through qemu_rbd_parse_filename()?
You can't -- commit c7cacb3e7 forces qemu_rbd_create() to call qemu_rbd_parse_filename(). And in qemu_rbd_parse_filename(), it will complain if pool is not provided (and that is what causes the abort, not the missing image parameter). So I think we are safe, but a nicer error message for a missing 'image' parameter might be nice anyway.
