qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Tomáš Golembiovský" <tgolembi@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-nbd: Add --image-size option
Date: Tue, 20 Sep 2016 13:35:29 +0200	[thread overview]
Message-ID: <20160920133529.05889b97@fiorina> (raw)
In-Reply-To: <30916fa0-a4b2-fa59-a594-38b172099fdb@redhat.com>

On Tue, 20 Sep 2016 11:59:28 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 20/09/2016 11:41, Tomáš Golembiovský wrote:
> > When image is part of the file it makes sense to limit the length of the
> > image in the file. Otherwise it is assumed that the image spans to the
> > end of the file. This assumption may lead to reads/writes outside of the
> > image and thus lead to errors or data corruption.
> > 
> > To limit the assumed image size new option is introduced.  
> 
> The patch makes sense, but I think the commit message is incorrect
> because this bug is already fixed by patch 1.  Also, the option in the

I agree that the wording is not completely clear. The patches solve two
different but related problems. The first patch solves situation where
you have file containing:

    || some data ; image ||

In this case qemu-nbd tried to access data outside the file. But there
is nothing there, because the file is shorter.

The second patch tries to solve the situation when you have file:

    || some data ; image ; some more data ||

In this case there is no way to say where the image ends and client may
also access content in the "some more data" area. Thus corrupting the
data outside the image.


What about something like this:

    Normally qemu-nbd assumes that the image spans from the beginning of
    the file (or from position specified by --offset) to the end of the
    file. If the image is embedded inside the file and there are some
    other data after the image this may lead to reads/writes outside the
    image and data corruption.

    This patch adds new command line argument --size that limits the
    assumed device size. This way the user can specify that the image
    ends sooner than at the end of the file.

> help is --device-size, not --image-size; I would just call it --size.

Ok. I will change it to --size.

> 
> Thanks,
> 
> Paolo
> 

  parent reply	other threads:[~2016-09-20 11:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-20  9:41 [Qemu-devel] [PATCH 2/2] qemu-nbd: Add --image-size option Tomáš Golembiovský
2016-09-20  9:59 ` Paolo Bonzini
2016-09-20 11:12   ` Daniel P. Berrange
2016-09-20 11:45     ` Paolo Bonzini
2016-10-02 19:33       ` Tomáš Golembiovský
2016-09-20 11:35   ` Tomáš Golembiovský [this message]
2016-09-20 12:45     ` Richard W.M. Jones

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=20160920133529.05889b97@fiorina \
    --to=tgolembi@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).