[libvirt] [PATCH] conf: fix crash when hotplug a channel chr device with no target

https://bugzilla.redhat.com/show_bug.cgi?id=1181408 when we try to hotplug a channel chr device with no target, we will get success(which should fail) in virDomainChrDefParseXML, because we use goto cleanup this place and return def.then cause a big problem in virDomainChrEquals(touch a shouldn't happend place). Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 57e99e6..3cbb93d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8700,7 +8700,7 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt, if (!seenTarget && ((def->targetType = virDomainChrDefaultTargetType(def->deviceType)) < 0)) - goto cleanup; + goto error; if (virDomainChrSourceDefParseXML(&def->source, node->children, flags, def, ctxt, vmSeclabels, nvmSeclabels) < 0) -- 1.8.3.1

On 01/13/2015 09:41 AM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1181408
when we try to hotplug a channel chr device with no target, we will get success(which should fail) in virDomainChrDefParseXML, because we use goto cleanup this place and return def.then cause a big problem in virDomainChrEquals(touch a shouldn't happend place).
The problem is that ChrEquals matches according to the target name, so after we add the device to the domain definition, we cannot remove it after failure, leaving a stale pointer there.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK I have added the explanation to the commit message and pushed the patch. Jan

On 01/13/2015 05:34 PM, Ján Tomko wrote:
On 01/13/2015 09:41 AM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1181408
when we try to hotplug a channel chr device with no target, we will get success(which should fail) in virDomainChrDefParseXML, because we use goto cleanup this place and return def.then cause a big problem in virDomainChrEquals(touch a shouldn't happend place). The problem is that ChrEquals matches according to the target name, so after we add the device to the domain definition, we cannot remove it after failure, leaving a stale pointer there.
yes, thanks a lot for improving the explanation.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK
I have added the explanation to the commit message and pushed the patch.
Jan
Luyao
participants (3)
-
Ján Tomko
-
lhuang
-
Luyao Huang