All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] bdrv_img_create: Fix segfault
Date: Mon, 06 Jun 2011 12:00:00 +0200	[thread overview]
Message-ID: <4DECA520.9020109@redhat.com> (raw)
In-Reply-To: <BANLkTimW_uZVNOuQ-pW=S1dACymCCaaHBg@mail.gmail.com>

Am 02.06.2011 00:34, schrieb Stefan Hajnoczi:
> On Wed, Jun 1, 2011 at 1:05 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Block drivers that don't support creating images don't have a size option. Fail
>> gracefully instead of segfaulting when trying to access the option's value.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> A command-line to reproduce the crash would be nice.

qemu-img create -f bochs nbd:foo 32M

It doesn't happen with a file protocol any more since we merge the
create options of the protocol with those of the format (was introduced
with Sheepdog).

> I noticed this line above your fix:
> set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
> 
> If I follow correctly there should be an "Unknown option 'size'" error
> message before set_option_parameter_int() returns -1 (which we ignore)
> and then crash.

Right, this is what happens.

> Perhaps we should just catch the error when set_option_parameter_int() fails?

We could do that, but the segfault isn't really related to a failing
set_option_parameter_int() but to the failing get_option_parameter(). I
think it's good style not to rely on the error handling of an unrelated
action some lines above.

Adding some error handling there, too, wouldn't hurt, though.

Kevin

  reply	other threads:[~2011-06-06  9:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-01 12:05 [Qemu-devel] [PATCH] bdrv_img_create: Fix segfault Kevin Wolf
2011-06-01 22:34 ` Stefan Hajnoczi
2011-06-06 10:00   ` Kevin Wolf [this message]
2011-06-06 11:10     ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DECA520.9020109@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.