classification
Title: Dereference after NULL check in listobject.c merge_hi()
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3, Python 3.4
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, christian.heimes, tim.peters, vstinner
Priority: normal Keywords:

Created on 2013-07-28 13:20 by christian.heimes, last changed 2013-08-13 15:56 by christian.heimes. This issue is now closed.

Messages (3)
msg193825 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-07-28 13:20
Coverity Scan may have found an issue in listobject's merge code. I'm not familiar with the code so I don't know if ssb.value can be NULL here.

3. Condition "ssb.values != NULL", taking false branch
4. var_compare_op: Comparing "ssb.values" to null implies that "ssb.values" might be null.
1642    if (ssb.values != NULL)
1643        ssb.values = ms->a.values + nb - 1;

[...]

 
CID 715348 (#1 of 2): Dereference after null check (FORWARD_NULL)
18. var_deref_model: Passing "&ssb" to function "sortslice_copy_decr(sortslice *, sortslice *)", which dereferences null "ssb.values".
1711            sortslice_copy_decr(&dest, &ssb);

http://hg.python.org/cpython/file/tip/Objects/listobject.c#l1711
msg194518 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2013-08-05 22:59
According to this comment, ssb.values can be null:

/* A sortslice contains a pointer to an array of keys and a pointer to
 * an array of corresponding values.  In other words, keys[i]
 * corresponds with values[i].  If values == NULL, then the keys are
 * also the values.
 *
 * Several convenience routines are provided here, so that keys and
 * values are always moved in sync.
 */ <http://hg.python.org/cpython/file/tip/Objects/listobject.c#l969>


However, is ssb.values is null, dest.values must be null as well and sortslice_copy_decr() will not dereference either of the pointers.

Looks like a false positive to me, but I wonder if a strategically placed assert will silence coverity:

Py_LOCAL_INLINE(void)
sortslice_copy_decr(sortslice *dst, sortslice *src)
{
    *dst->keys-- = *src->keys--;
    if (dst->values != NULL) {
        assert(src->values != NULL);
        *dst->values-- = *src->values--;
    }
}
msg195082 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-08-13 15:56
Thanks! Your analysis is plausible. I have closed the issue as "false positive".
History
Date User Action Args
2013-08-13 15:56:58christian.heimessetstatus: open -> closed
resolution: not a bug
messages: + msg195082

stage: resolved
2013-08-05 22:59:51belopolskysetnosy: + belopolsky
messages: + msg194518
2013-07-28 13:20:05christian.heimescreate