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)

scan-build.tar.gz (241.2 KB ) - added by Jeffrey Walton 11 years ago.
Clang 3.3 scan-build results

Download all attachments as: .zip

Change History (3)

by Jeffrey Walton, 11 years ago

Attachment: scan-build.tar.gz added

Clang 3.3 scan-build results

comment:1 by Ruslan Ermilov, 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 Ruslan Ermilov, 11 years ago

Resolution: invalid
Status: newclosed
Note: See TracTickets for help on using tickets.