[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input

[Eric Blake]

According to Jim Meyering on 3/1/2010 1:21 PM:
> Clang warned about the potential NULL-dereference
> via the STREQ/strcmp below.  models[i] could be NULL.
> Even "models" could be NULL, and the "allowed = ..." test
> makes that appear to be deliberately allowed.

This same function was also listed by coverity, but only for models, not
models[i].

> The change below is the least invasive and cleanest
> I could come up with, assuming there is no need to diagnose
> (e.g., by returning -1) the condition of a NULL models[i] pointer.
> 
>      while (candidate != NULL) {
>          bool allowed = (models == NULL);
> 
>          for (i = 0; i < candidate->ncpuid; i++) {
>              cpuid = x86DataCpuid(data, candidate->cpuid[i].function);
>              if (cpuid == NULL
>                  || !x86cpuidMatchMasked(cpuid, candidate->cpuid + i))
>                  goto next;
>          }
> 
>          for (i = 0; i < nmodels; i++) {
> -            if (STREQ(models[i], candidate->name)) {
> +            if (models && models[i] && STREQ(models[i], candidate->name)) {

Isn't the intent that (models==NULL) iff (nmodels==0)?  In which case,
this code is unreachable if models is NULL.  But your patch certainly is
the least-invasive possible, and while it is only a false positive for
well-formed arguments, I didn't spend time checking all clients of
x86Decode to see if there is ever a possibility of bad arguments.

ACK

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 320 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100301/c764f333/attachment-0001.sig>