diff options
author | Arjun <pkillarjun@protonmail.com> | 2024-05-29 13:13:24 +0530 |
---|---|---|
committer | Andrew Clayton <a.clayton@nginx.com> | 2024-06-14 15:08:44 +0100 |
commit | 04a24f61e069926a6546917ee049dc17fbaf1d03 (patch) | |
tree | 62ddd1dbe5f51fba95d17e1e929dff259620cc25 /src | |
parent | d7ec30c43aea185a8425e8c2ba3a6fbfdd24282b (diff) | |
download | unit-04a24f61e069926a6546917ee049dc17fbaf1d03.tar.gz unit-04a24f61e069926a6546917ee049dc17fbaf1d03.tar.bz2 |
http: fix use-of-uninitialized-value bug
This was found via MSan.
In nxt_http_fields_hash() we setup a nxt_lvlhsh_query_t structure and
initialise a couple of its members.
At some point we call
lhq->proto->alloc(lhq->pool, nxt_lvlhsh_bucket_size(lhq->proto));
Which in this case is
void *
nxt_lvlhsh_alloc(void *data, size_t size)
{
return nxt_memalign(size, size);
}
So even though lhq.ppol wasn't previously initialised we don't actually
use it in that particular function.
However MSan triggers on the fact that we are passing an uninitialised
value into that function.
Indeed, compilers will generally complain about such things, e.g
/* u.c */
struct t {
void *p;
int len;
};
static void test(void *p __attribute__((unused)), int len)
{
(void)len;
}
int main(void)
{
struct t t;
t.len = 42;
test(t.p, t.len);
return 0;
}
GCC and Clang will produce a -Wuninitialized warning.
But they won't catch the following...
/* u2.c */
struct t {
void *p;
int len;
};
static void _test(void *p __attribute__((unused)), int len)
{
(void)len;
}
static void test(struct t *t)
{
_test(t->p, t->len);
}
int main(void)
{
struct t t;
t.len = 42;
test(&t);
return 0;
}
Which is why we don't get a compiler warning about lhq.pool.
In this case initialising lhg.pool even though we don't use it here
seems like the right thing to do and maybe compilers will start being
able to catch these in the future.
Actually GCC with -fanalyzer does catch the above
$ gcc -Wall -Wextra -O0 -fanalyzer u2.c
u2.c: In function ‘test’:
u2.c:15:9: warning: use of uninitialized value ‘*t.p’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
15 | _test(t->p, t->len);
| ^~~~~~~~~~~~~~~~~~~
‘main’: events 1-3
|
| 18 | int main(void)
| | ^~~~
| | |
| | (1) entry to ‘main’
| 19 | {
| 20 | struct t t;
| | ~
| | |
| | (2) region created on stack here
|......
| 23 | test(&t);
| | ~~~~~~~~
| | |
| | (3) calling ‘test’ from ‘main’
|
+--> ‘test’: events 4-5
|
| 13 | static void test(struct t *t)
| | ^~~~
| | |
| | (4) entry to ‘test’
| 14 | {
| 15 | _test(t->p, t->len);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (5) use of uninitialized value ‘*t.p’ here
|
Signed-off-by: Arjun <pkillarjun@protonmail.com>
Link: <https://clang.llvm.org/docs/MemorySanitizer.html>
[ Commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/nxt_http_parse.c | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/src/nxt_http_parse.c b/src/nxt_http_parse.c index 48be5bdb..dd490e72 100644 --- a/src/nxt_http_parse.c +++ b/src/nxt_http_parse.c @@ -1182,6 +1182,7 @@ nxt_http_fields_hash(nxt_lvlhsh_t *hash, lhq.replace = 0; lhq.proto = &nxt_http_fields_hash_proto; + lhq.pool = NULL; for (i = 0; i < count; i++) { key = NXT_HTTP_FIELD_HASH_INIT; |