qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	famz@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com,
	Ian Main <imain@redhat.com>,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH V6 1/3] Implement sync modes for drive-backup.
Date: Thu, 25 Jul 2013 18:59:07 +0200	[thread overview]
Message-ID: <51F1595B.6050106@redhat.com> (raw)
In-Reply-To: <51F158E8.8040907@redhat.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 25/07/2013 18:57, Eric Blake ha scritto:
> On 07/25/2013 08:36 AM, Paolo Bonzini wrote:
> 
>>> Furthermore, I'm proposing that for 1.6, we should make the
>>> format argument mandatory for drive-backup.  We made it
>>> optional for drive-mirror, to allow for probing, but there have
>>> been CVEs in the past due to probing of a raw file gone wrong.
>>> We can always relax a mandatory argument into an optional one
>>> in 1.7, if we decide that probing can be done safely, but we
>>> can never turn an optional argument into a mandatory one once
>>> the initial release bakes in the option.  It would make the
>>> code a lot simpler to just have a mandatory format argument,
>>> instead of having to bake in and document hueristics on which
>>> format is picked when the caller doesn't provide one.
>> 
>> Probing is unsafe by definition, on the other hand we should
>> allow it for consistency with the rest of the API.
> 
> Shouldn't security trump consistency?

If the image can be trusted, disallowing probing makes things more
complex for the user for no reason.

Also it's only on raw images that probing is unsafe.  If all
user-writable images are non-raw, probing is actually safe.

Paolo

  My argument is that if probing
> can be abused, it is better to have 1.6 be conservative and mandate
> a format argument always; if we can later argue that some forms of
> probing are safe, then for 1.7 we can relax the argument to
> optional in the qapi-schema.json file and add code checks that
> still mandate the argument in the instances where it is needed, but
> allow probing in the remaining cases.  But why are we so concerned
> about allowing probing in the first place?  Libvirt will ALWAYS be
> passing a format, because that is the only way to avoid a security
> bug; it is easier to design the format to be mandatory than it is
> to reason about when probing might be safe, and there's no need to
> be consistent with the security holes that are permanently baked
> into older commands.
> 
>> 
>> Making the format mandatory for mode != 'existing' is fine,
>> though. We can relax it later.
>> 
>> For mode = 'existing' we should allow both probing, and using an 
>> explicit format.
> 
> But if you insist on allowing probing for mode='existing', then 
> qapi-schema.json must leave it as '*format':'str', and we are
> stuck implementing the code for when format is mandatory in the C
> code.
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR8VlaAAoJEBvWZb6bTYbyTcYP/jo6xYvm2kXQ8IdDlTGu8d0I
2h+ySv2TUg7b7KSRPlDc/pAAItbfzkxRl9lrk8m50iMtX+IwFkkFYLPv0hqRBKu2
5QA6DBmXmNkCl+2pABbTaVSwZgyKqxXExuHEMyLczX7Zutx5H9RQeo7hIU3TvhcU
5yuLyO6wgjsiWspvNlax2PZiN0ZMjil5BdQsTG4G2dxP3z9bDDjH9ZFUBStcrUTO
qPuwmWWk1FvQptbK19U+pMybPX1MsDjJRVLd6QwbOTWlSn89zl5MDCXAdqJagAYj
yN4cxuN5mlV4FyKTrnhb1eIfu5Wm+EwCo9YKcSc6EbvmKPCXXNh73xZKbk/1CngT
WOhaHn+QLM0s0rQOIWPZtn+zZU2TqXW9kvRXL0nKF1cXqw/+Aoz6sjP/uJILcEh5
O9vkyeSYL7eM8EmozIRqL//R4puBjaqdKBCxD1yQQYC2dtizIBiLRVU4+w4mbnIk
d561F2MR3UUEAfkGMPMMejL+rNyywffKvle4isT4raEHCppbb5OptK6RtllJ9jPr
mLLJ+758JhMD5H5u1MiKTzE/eQtHaP0MTBrAaQJf4omKvWOfhv/jDD9lsLP0k3Ms
WmivfZdQI+bGwi9vysEhbfAuAYdC6SSeTtZCaY6m8cKsJI/7CF5RhPMXVZElC4Q5
K2Fz6vhiU+HacN6pP2F6
=TyCJ
-----END PGP SIGNATURE-----

  reply	other threads:[~2013-07-25 16:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-22 22:09 [Qemu-devel] [PATCH V6 0/3] Implement sync modes for drive-backup Ian Main
2013-07-22 22:09 ` [Qemu-devel] [PATCH V6 1/3] " Ian Main
2013-07-24 10:55   ` Kevin Wolf
2013-07-24 17:57     ` Ian Main
2013-07-24 20:32     ` Eric Blake
2013-07-24 22:01       ` Ian Main
2013-07-25 11:23         ` Kevin Wolf
2013-07-25 14:36       ` Paolo Bonzini
2013-07-25 16:57         ` Eric Blake
2013-07-25 16:59           ` Paolo Bonzini [this message]
2013-07-22 22:09 ` [Qemu-devel] [PATCH V6 2/3] Add tests for sync modes 'TOP' and 'NONE' Ian Main
2013-07-24 11:19   ` Kevin Wolf
2013-07-24 18:02     ` Ian Main
2013-07-22 22:09 ` [Qemu-devel] [PATCH V6 3/3] Add backing drive while performing backup Ian Main
2013-07-24 11:28   ` Kevin Wolf
2013-07-23 11:53 ` [Qemu-devel] [PATCH V6 0/3] Implement sync modes for drive-backup Stefan Hajnoczi
2013-07-23 19:55   ` Ian Main
2013-07-24  0:55     ` Fam Zheng

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=51F1595B.6050106@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=imain@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=stefanha@redhat.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 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).