On 07/12/2018 09:10 AM, Simon Kobyda wrote:
XML shmem name will not include character '/', and will not be equal to strings "." or "..", as shmem name is used in a path.
Validate that the provided XML shmem name is not directory specific "." or ".." names as well as ensuring that there is no path separator '/' in the name.
https://bugzilla.redhat.com/show_bug.cgi?id=1192400 ---
Changes in V2 - Added error reports - Error situation will happen only if shmem name is equal to "." or "..", however their occurence in a name compromised of more characters is allowed.
src/conf/domain_conf.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
I believe this actually belongs in virDomainDeviceDefValidateInternal for case VIR_DOMAIN_DEVICE_SHMEM. Also, should the docs/schemas/domaincommon.rng be modified? Currently it has: <define name="shmem"> <element name="shmem"> <attribute name="name"> <data type="string"> <param name="pattern">[^/]*</param> </data> Consider how other names are limited in their scope. The basictypes.rng has a number of examples. Naturally, the problem with changing it is that someone somewhere will complain, but libvirt used to accept this other format. Right now I would think the scope a bit too broad. If we are to limit the name we should also document in docs/formatdomain.html.in that the shmem name is "limited" in name to avoid the '/' character, ".", and "..". BTW: My regex isn't that good, but it would seem '/' is an invalid character by XML standards even though the code never checked for it. Using virt-xml-validate <file> <schema> would "validate" whether someone provides valid XML. John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7ab2953d83..6b34c17de4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def) static int virDomainDefValidateInternal(const virDomainDef *def) { + size_t i; + if (virDomainDefCheckDuplicateDiskInfo(def) < 0) return -1;
@@ -6136,6 +6138,26 @@ virDomainDefValidateInternal(const virDomainDef *def) return -1; }
+ for (i = 0; i < def->nshmems; i++) { + if (strchr(def->shmems[i]->name, '/')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem name cannot include '/' character")); + return -1; + } + + if (STREQ(def->shmems[i]->name, ".")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem name cannot be equal to '.'")); + return -1; + } + + if (STREQ(def->shmems[i]->name, "..")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem name cannot be equal to '..'")); + return -1; + } + } + if (virDomainDefLifecycleActionValidate(def) < 0) return -1;