[libvirt PATCH 4/7] nodedev: Add parser validation for node devices

At the moment, this is only for mediated devices. When a new mediated device is created or defined, the xml is expected specify the nodedev name of an existing device as its parent. We were not previously validating this and were simply accepting any string here. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/conf/node_device_conf.c | 30 +++++++++--- src/conf/node_device_conf.h | 20 +++++++- src/conf/virnodedeviceobj.h | 1 + src/hypervisor/domain_driver.c | 7 +-- src/node_device/node_device_driver.c | 68 +++++++++++++++++++++++++++- src/node_device/node_device_driver.h | 3 ++ src/node_device/node_device_udev.c | 2 + src/test/test_driver.c | 6 ++- tests/nodedevmdevctltest.c | 7 ++- tests/nodedevxml2xmltest.c | 3 +- 10 files changed, 129 insertions(+), 18 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 332b12f997..cd1c07092d 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2174,10 +2174,12 @@ static virNodeDeviceDef * virNodeDeviceDefParse(const char *str, const char *filename, int create, - const char *virt_type) + const char *virt_type, + virNodeDeviceDefParserCallbacks *parserCallbacks, + void *opaque) { xmlDocPtr xml; - virNodeDeviceDef *def = NULL; + g_autoptr(virNodeDeviceDef) def = NULL; if ((xml = virXMLParse(filename, str, _("(node_device_definition)")))) { def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), @@ -2185,25 +2187,39 @@ virNodeDeviceDefParse(const char *str, xmlFreeDoc(xml); } - return def; + if (parserCallbacks) { + int ret = 0; + /* validate definition */ + if (parserCallbacks->validate) { + ret = parserCallbacks->validate(def, opaque); + if (ret < 0) + return NULL; + } + } + + return g_steal_pointer(&def); } virNodeDeviceDef * virNodeDeviceDefParseString(const char *str, int create, - const char *virt_type) + const char *virt_type, + virNodeDeviceDefParserCallbacks *parserCallbacks, + void *opaque) { - return virNodeDeviceDefParse(str, NULL, create, virt_type); + return virNodeDeviceDefParse(str, NULL, create, virt_type, parserCallbacks, opaque); } virNodeDeviceDef * virNodeDeviceDefParseFile(const char *filename, int create, - const char *virt_type) + const char *virt_type, + virNodeDeviceDefParserCallbacks *parserCallbacks, + void *opaque) { - return virNodeDeviceDefParse(NULL, filename, create, virt_type); + return virNodeDeviceDefParse(NULL, filename, create, virt_type, parserCallbacks, opaque); } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 556878b9a8..786de85060 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -349,15 +349,31 @@ struct _virNodeDeviceDef { char * virNodeDeviceDefFormat(const virNodeDeviceDef *def); + +typedef int (*virNodeDeviceDefPostParseCallback)(virNodeDeviceDef *dev, + void *opaque); + +typedef int (*virNodeDeviceDefValidateCallback)(virNodeDeviceDef *dev, + void *opaque); + +typedef struct _virNodeDeviceDefParserCallbacks { + virNodeDeviceDefPostParseCallback postParse; + virNodeDeviceDefValidateCallback validate; +} virNodeDeviceDefParserCallbacks; + virNodeDeviceDef * virNodeDeviceDefParseString(const char *str, int create, - const char *virt_type); + const char *virt_type, + virNodeDeviceDefParserCallbacks *callbacks, + void *opaque); virNodeDeviceDef * virNodeDeviceDefParseFile(const char *filename, int create, - const char *virt_type); + const char *virt_type, + virNodeDeviceDefParserCallbacks *callbacks, + void *opaque); virNodeDeviceDef * virNodeDeviceDefParseNode(xmlDocPtr xml, diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h index 0cb78748a4..1fdd4f65da 100644 --- a/src/conf/virnodedeviceobj.h +++ b/src/conf/virnodedeviceobj.h @@ -47,6 +47,7 @@ struct _virNodeDeviceDriverState { /* Immutable pointer, self-locking APIs */ virObjectEventState *nodeDeviceEventState; + virNodeDeviceDefParserCallbacks parserCallbacks; }; void diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 29e11c0447..2969d55173 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -395,7 +395,8 @@ virDomainDriverNodeDeviceReset(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, + NULL, NULL); if (!def) return -1; @@ -440,7 +441,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL); if (!def) return -1; @@ -488,7 +489,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL); if (!def) return -1; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ad2ca2a614..8f39d79c68 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -896,7 +896,8 @@ nodeDeviceCreateXML(virConnectPtr conn, virt_type = virConnectGetType(conn); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type))) + if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type, + &driver->parserCallbacks, NULL))) return NULL; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -1376,7 +1377,8 @@ nodeDeviceDefineXML(virConnect *conn, virt_type = virConnectGetType(conn); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type))) + if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type, + &driver->parserCallbacks, NULL))) return NULL; if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0) @@ -1761,3 +1763,65 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst, return ret; } + + +/* validate that parent exists */ +static int nodeDeviceDefValidateMdev(virNodeDeviceDef *def, + G_GNUC_UNUSED virNodeDevCapMdev *mdev, + G_GNUC_UNUSED void *opaque) +{ + virNodeDeviceObj *obj = NULL; + if (!def->parent) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("missing parent device")); + return -1; + } + obj = virNodeDeviceObjListFindByName(driver->devs, def->parent); + if (!obj) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid parent device '%s'"), + def->parent); + return -1; + } + + virNodeDeviceObjEndAPI(&obj); + return 0; +} + +int nodeDeviceDefValidate(virNodeDeviceDef *def, + G_GNUC_UNUSED void *opaque) +{ + virNodeDevCapsDef *caps = NULL; + for (caps = def->caps; caps != NULL; caps = caps->next) { + switch (caps->data.type) { + case VIR_NODE_DEV_CAP_MDEV: + if (nodeDeviceDefValidateMdev(def, &caps->data.mdev, opaque) < 0) + return -1; + break; + + case VIR_NODE_DEV_CAP_SYSTEM: + case VIR_NODE_DEV_CAP_PCI_DEV: + case VIR_NODE_DEV_CAP_USB_DEV: + case VIR_NODE_DEV_CAP_USB_INTERFACE: + case VIR_NODE_DEV_CAP_NET: + case VIR_NODE_DEV_CAP_SCSI_HOST: + case VIR_NODE_DEV_CAP_SCSI_TARGET: + case VIR_NODE_DEV_CAP_SCSI: + case VIR_NODE_DEV_CAP_STORAGE: + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + case VIR_NODE_DEV_CAP_DRM: + case VIR_NODE_DEV_CAP_MDEV_TYPES: + case VIR_NODE_DEV_CAP_CCW_DEV: + case VIR_NODE_DEV_CAP_CSS_DEV: + case VIR_NODE_DEV_CAP_VDPA: + case VIR_NODE_DEV_CAP_AP_CARD: + case VIR_NODE_DEV_CAP_AP_QUEUE: + case VIR_NODE_DEV_CAP_AP_MATRIX: + case VIR_NODE_DEV_CAP_LAST: + break; + } + } + return 0; +} diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index edd763f0e4..d9b9b7a961 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -171,3 +171,6 @@ bool nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst, int nodeDeviceCreate(virNodeDevice *dev, unsigned int flags); + +int nodeDeviceDefValidate(virNodeDeviceDef *def, + void *opaque); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index e7db74b325..ccf94d8e7d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -2243,6 +2243,8 @@ nodeStateInitialize(bool privileged, driver->privateData = priv; driver->nodeDeviceEventState = virObjectEventStateNew(); + driver->parserCallbacks.validate = nodeDeviceDefValidate; + if (udevPCITranslateInit(privileged) < 0) goto unlock; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 892dc978f2..c2d642ff6b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7481,7 +7481,8 @@ testNodeDeviceMockCreateVport(testDriver *driver, if (!xml) goto cleanup; - if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL))) + if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, + NULL))) goto cleanup; VIR_FREE(def->name); @@ -7543,7 +7544,8 @@ testNodeDeviceCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL))) + if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL, + NULL, NULL))) goto cleanup; /* We run this simply for validation - it essentially validates that diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 7af89ab5f1..bf1d7b4984 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -12,6 +12,10 @@ #define VIRT_TYPE "QEMU" +static virNodeDeviceDefParserCallbacks parser_callbacks = { + .validate = nodeDeviceDefValidate +}; + struct TestInfo { const char *filename; virMdevctlCommand command; @@ -66,7 +70,8 @@ testMdevctlCmd(virMdevctlCommand cmd_type, return -1; } - if (!(def = virNodeDeviceDefParseFile(mdevxml, create, VIRT_TYPE))) + if (!(def = virNodeDeviceDefParseFile(mdevxml, create, VIRT_TYPE, + &parser_callbacks, NULL))) return -1; /* this function will set a stdin buffer containing the json configuration diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index e9716726d7..d8f81456ee 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -25,7 +25,8 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile) if (virTestLoadFile(xml, &xmlData) < 0) goto fail; - if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE, NULL))) + if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE, NULL, + NULL, NULL))) goto fail; /* Calculate some things that are not read in */ -- 2.31.1