Opened 11 years ago
Closed 11 years ago
#460 closed task (invalid)
Clang reports use-after-free in core/ngx_resolver.c
Reported by: | Jeffrey Walton | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-core | Version: | |
Keywords: | Clang use-after-free | Cc: | |
uname -a: |
$ uname -a
Darwin riemann.home.pvt 12.5.0 Darwin Kernel Version 12.5.0: Sun Sep 29 13:33:47 PDT 2013; root:xnu-2050.48.12~1/RELEASE_X86_64 x86_64 |
||
nginx -V: |
$ ./objs/nginx -V
nginx version: nginx/1.4.4 configure arguments: |
Description
Clang 3.3 flagged two use-after-free's in core/ngx_resolver.c.
I'm not sure its a valid finding, but I'm reporting it just in case. I logged it as a task rather than a defect.
src/core/ngx_resolver.c:854:20: warning: Use of memory after it is freed if (now <= rn->expire) { ^~~~~~~~~~ src/core/ngx_resolver.c:972:19: warning: Use of memory after it is freed if (now < rn->expire) {
Both issues appear to be centered on the calls to the following (and the subsequent jump to the top of the loop):
ngx_rbtree_delete(tree, &rn->node); ngx_resolver_free_node(r, rn);
The attached is a copy of the results from scan-build. Its a graphical call graph to show how Clang determined the use-after-free path. In the attached, navigate to index.html. The click the link for issue.
Finally, for those who use Clang 3.3 and scan-build, the command to duplicate is:
$ export CC="/usr/local/bin/clang" $ export CXX="/usr/local/bin/clang++" $ /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang ./configure $ /usr/local/bin/scan-build/scan-build --use-analyzer=/usr/local/bin/clang make
Attachments (1)
Change History (3)
by , 11 years ago
Attachment: | scan-build.tar.gz added |
---|
comment:1 by , 11 years ago
An existing cache entry (rn) can be in one of two queues: resend queue, or expire queue. When the node is removed and its memory is freed by ngx_resolver_free_node(), it should be first removed from the cache by ngx_rbtree_delete() and from a queue by ngx_queue_remove(). The current code is correct, from what I can tell.
comment:2 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Clang 3.3 scan-build results