[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 7/8 v8] qemu: Set unpriv_sgio when attaching disk

On 2012年12月19日 02:03, Eric Blake wrote:
On 12/16/2012 12:32 PM, Osier Yang wrote:
Again, does this do the right thing if some other domain already had
cdbfilter='no' (aka kernel allows SG_IO) but this domain omitted the
cdbfilter attribute (which defaults to cdbfilter='yes' but does not
satisfy the if(disk->cdbfilter) condition)?

I think it does the right thing, because not specifying "cdbfilter"
should mean it doesn't care about what the kernel setting is, not
means cdbfiler="yes" instead.

Unfortunately, that's the wrong approach.  We should be secure by
default, and therefore we MUST treat an omitted attribute as though it
meant the secure setting, and not that we don't care about the setting.
  In other words, if guest A uses cdbfilter='no' and guest B omits the
cdbfilter attribute, then we must refuse to start guest B (ignoring the
issue would mean that guest B could do a raw SG_IO command that would
negatively impact guest A, and the premise of sVirt is that guest B
cannot impact guest A unless it was explicitly documented as being


Of course, this is based on we use a separate XML tag like cdbfilter,
it won't stand anymore if we finally use a tri-state rawio.

I know you reposted a newer version of this series, and that based on
IRC conversations we talked about using a naming of
sgio='filtered|unfiltered' instead of cdbfilter='yes|no'; I'll have to
review that series.  But I wanted to make sure that I left a comment in
this thread (and not just IRC) why secure-by-default is the only safe
way to behave when the new attribute is not present.

Instead of defaulting the sgio to "filtered" when parsing conf. The
new version defaults it in qemu driver. To distinguish the user
explicit requested "filtered" and default "filtered", for the
default "filtered", we will ignore it if the kernel doesn't
support it, but for explictly requestd "filtered", we have to
error out anyway. Otherwise we might drop into regression on
the platform kernel is old enough doesn't support unpriv_sgio.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]