Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/stick_table.c: null pointer dereference suspected by coverity #2518

Open
chipitsine opened this issue Apr 4, 2024 · 1 comment
Open
Labels
type: code-report This issue describes a code report (like valgrind or coverity)

Comments

@chipitsine
Copy link
Member

Tool Name and Version

coverity

Code Report

*** CID 1542870:  Null pointer dereferences  (REVERSE_INULL)
/src/stick_table.c: 709 in stktable_get_entry()
703                     len = key->key_len + 1 < table->key_size ? key->key_len : table->key_size - 1;
704             else
705                     len = table->key_size;
706     
707             shard = stktable_calc_shard_num(table, key->key, len);
708     
>>>     CID 1542870:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "key" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
709             if (!key)
710                     return NULL;
711     
712             HA_RWLOCK_RDLOCK(STK_TABLE_LOCK, &table->shards[shard].sh_lock);
713             ts = __stktable_lookup_key(table, key, shard);
714             if (ts)

Additional Information

No response

Output of haproxy -vv

no
@chipitsine chipitsine added the type: code-report This issue describes a code report (like valgrind or coverity) label Apr 4, 2024
haproxy-mirror pushed a commit that referenced this issue Apr 4, 2024
This bug arrived with this commit:
     MAJOR: stktable: split the keys across multiple shards to reduce contention

At this time, there are no callers which call stktable_get_entry() without checking
the nullity of <key> passed as parameter. But the documentation of this function
says it supports this case where the <key> passed as parameter could be null.

Move the nullity test on <key> at first statement of this function.

Thanks to @chipitsine for having reported this issue in GH #2518.
@haproxyFred
Copy link
Contributor

I have just pushed a fix for this. That said, as mentioned in the commit log, there are no callers of stktable_get_entry() which do not test the nullity of passed as parameter to this function (no crashes were possible), even if this is useless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: code-report This issue describes a code report (like valgrind or coverity)
Projects
None yet
Development

No branches or pull requests

2 participants