Re: [libvirt] [PATCH] rng: restrict passthrough names to known-good files

On 03/04/13 23:49, Eric Blake wrote: > There is some controversy[1] on the qemu list on whether qemu should > have ever allowed arbitrary file name passthrough, or whether it > should be restricted to JUST /dev/random and /dev/hwrng. It is > always easier to add support for additional filenames than it is > to remove support for something once released, so this patch > restricts libvirt 1.0.3 (where the virtio-random backend was first > supported) to just the two uncontroversial names, letting us defer > to a later date any decision on whether supporting arbitrary files > makes sense. Additionally, since qemu 1.4 does NOT support > /dev/fdset/nnn fd passthrough for the backend, limiting to just > two known names means that we don't get tempted to try fd > passthrough where it won't work. > > [1]https://lists.gnu.org/archive/html/qemu-devel/2013-03/threads.html#00023 > > > * src/conf/domain_conf.c (virDomainRNGDefParseXML): Only allow > /dev/random and /dev/hwrng. > * docs/schemas/domaincommon.rng: Flag invalid files. > * docs/formatdomain.html.in (elementsRng): Document this. > * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args: > Update test to match. > * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml: > Likewise. > --- > > This needs to be acked before libvirt 1.0.3; otherwise we are > stuck supporting arbitrary name passthrough. Okay, this is fair enough for now. > > docs/formatdomain.html.in | 3 ++- > docs/schemas/domaincommon.rng | 5 ++++- > src/conf/domain_conf.c | 7 +++++++ > tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +- > tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml | 2 +- > 5 files changed, 15 insertions(+), 4 deletions(-) > ... > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 995cf0c..9c96cf1 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -7423,6 +7423,13 @@ virDomainRNGDefParseXML(const xmlNodePtr node, > switch ((enum virDomainRNGBackend) def->backend) { > case VIR_DOMAIN_RNG_BACKEND_RANDOM: > def->source.file = virXPathString("string(./backend)", ctxt); > + if (STRNEQ(def->source.file, "/dev/random") && > + STRNEQ(def->source.file, "/dev/hwrng")) { > + virReportError(VIR_ERR_XML_ERROR, > + _("file '%s' is not a supported random > source"), > + def->source.file); > + goto error; This is yet another part of the stuff that will need to be moved out of the parser when I finish the XML parsing callbacks.