On Mon, May 11, 2015 at 08:59:37PM +0800, Luyao Huang wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1220265

Pass the return value to an enum directly is not safe. When pass a invalid @ioeventfd and virTristateSwitchTypeFromString return -1 to def->msi.ioeventfd, and this value transform to 4294967295, so no error when the parse failed.

To fix this issue, folter the value using int and then assign it to virTristateSwitch as it's done.

Signed-off-by: Luyao Huang <lhuang@redhat.com> --- v2: a new way to fix this issue.

ACK, I reworded the commit message and pushed and I believe there is no need for a unit test. I wonder why we don't rework out FromString() helpers to parse the value into a parameter passed as a pointer and return 0/-1 properly. Probably nobody wanted to mess with half of libvirt code, I guess...

src/conf/domain_conf.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4cd36a1..04cce50 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10847,12 +10847,16 @@ virDomainShmemDefParseXML(xmlNodePtr node, } VIR_FREE(tmp);

- if ((tmp = virXMLPropString(msi, "ioeventfd")) && - (def->msi.ioeventfd = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid msi ioeventfd setting for shmem: '%s'"), - tmp); - goto cleanup; + if ((tmp = virXMLPropString(msi, "ioeventfd"))) { + int val; + + if ((val = virTristateSwitchTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid msi ioeventfd setting for shmem: '%s'"), + tmp); + goto cleanup; + } + def->msi.ioeventfd = val; } VIR_FREE(tmp); } -- 1.8.3.1