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;
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list