Re: [libvirt] [PATCH 2/7] conf: prevent crash with no uuid in cephx auth secret
On 11/28/12 15:31, Osier Yang wrote:
On 2012年11月28日 21:34, Ján Tomko wrote:
> Also remove the pointles check for NULL in auth.cephx.secret.uuid,
> since this is a static array.
It's nice if there is log of coverity.
Error: FORWARD_NULL (CWE-476):
libvirt-0.10.2/src/conf/storage_conf.c:447: cond_false: Condition
"auth->username == NULL", taking false branch
libvirt-0.10.2/src/conf/storage_conf.c:451: if_end: End of if statement
libvirt-0.10.2/src/conf/storage_conf.c:455: cond_true: Condition "uuid
== NULL", taking true branch
libvirt-0.10.2/src/conf/storage_conf.c:455: var_compare_op: Comparing
"uuid" to null implies that "uuid" might be null.
libvirt-0.10.2/src/conf/storage_conf.c:455: cond_false: Condition
"auth->secret.usage == NULL", taking false branch
libvirt-0.10.2/src/conf/storage_conf.c:459: if_end: End of if statement
libvirt-0.10.2/src/conf/storage_conf.c:461: var_deref_model: Passing
null pointer "uuid" to function "virUUIDParse(char const *, unsigned
char *)", which dereferences it. (The dereference is assumed on the
basis of the 'nonnull' parameter attribute.)
Error: NO_EFFECT (CWE-398):
libvirt-0.10.2/src/conf/storage_conf.c:979: array_null: Comparing an
array to null is not useful: "src->auth.cephx.secret.uuid != NULL".
> ...
This change forces both of "uuid" and "usage" to be specified.
But from the RNG schema:
<define name='sourceinfoauthsecret'>
<element name='secret'>
<choice>
<attribute name='uuid'>
<text/>
</attribute>
<attribute name='usage'>
<text/>
</attribute>
</choice>
</element>
</define>
Means that it allows only one of them specified.
Hm, from the schema, it should error out if both of them are
specified too. So either there is problem of either the schema
or the codes.
I think we have to figure out if the schema is correct first.
Looking at the code in storage_backend_rbd.c it looks like if both are
specified, only usage is taken into account:
if (pool->def->source.auth.cephx.secret.uuid != NULL) {
virUUIDFormat(pool->def->source.auth.cephx.secret.uuid, secretUuid);
VIR_DEBUG("Looking up secret by UUID: %s", secretUuid);
secret = virSecretLookupByUUIDString(conn, secretUuid);
}
if (pool->def->source.auth.cephx.secret.usage != NULL) {
VIR_DEBUG("Looking up secret by usage: %s",
pool->def->source.auth.cephx.secret.usage);
secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH,
pool->def->source.auth.cephx.secret.usage);
}
I'll send another version shortly.
Jan