[libvirt] [PATCH 2/7] conf: prevent crash with no uuid in cephx auth secret
Ján Tomko
jtomko at redhat.com
Thu Nov 29 16:21:56 UTC 2012
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
More information about the libvir-list
mailing list