Re: [PATCH 1/8] conf: Report an error when default TPM model is provided

On 8/1/22 13:10, Peter Krempa wrote: > On Mon, Jul 18, 2022 at 11:30:43 +0200, Michal Privoznik wrote: >> When "default" model of a TPM was provided, our parses accepts it >> happily even though the value is forbidden by our RNG and not >> documented as accepted value. This is because of < 0 vs <= 0 >> comparison of virDomainTPMModelTypeFromString() retval. >> >> Make the parser error out explicitly in this case. Users can >> always chose to not specify the attribute in which case we pick a >> sane default (in qemuDomainTPMDefPostParse()). >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/conf/domain_conf.c | 2 +- >> src/conf/domain_conf.h | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 4c7a5a044c..b7147945da 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -10360,7 +10360,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, >> >> model = virXMLPropString(node, "model"); >> if (model != NULL && >> - (def->model = virDomainTPMModelTypeFromString(model)) < 0) { >> + (def->model = virDomainTPMModelTypeFromString(model)) <= 0) { >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >> _("Unknown TPM frontend model '%s'"), model); >> goto error; > > 'virDomainTPMDefFormat' happily formats 'default' as supported type: > > virBufferAsprintf(&attrBuf, " model='%s'", > virDomainTPMModelTypeToString(def->model)); > > Is there any other code path which would forbid 'default'? Couple of them, actually. The first one is in qemuValidateDomainDeviceDefTPM() where virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, &tpmCaps); is called followed by if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.model, tpm->model)) {}. And the second is qemuDomainTPMDefPostParse() which overwrites the _DEFAULT to either _TIS or _SPAPR (alright, this is not a check that forbids 'default' per se).