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

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 permitted).

> > 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.