Patch for http proxying bug with some old gcc.

Vadim Panov vadim.v.panov at gmail.com
Fri May 13 05:04:40 UTC 2016


Hi Guys,

Could you please review and consider including the attached patch. It fixes
a bug which is only created by some gcc's, you will understand looking at
it.

I reproduce using nginx with openwrt on marvell platform, so I can't tell
what else might be affected.

How it works.
upstream do something like this
for(;;) {
still some data
   something->handle_header(some data)
}
and proxying module changes value of handle_header to some new function
inside handle_header. if we have enough data to still stay in this loop
some gcc would call a wrong pointer. (this bug is floating as if data goes
in slow we exit this for loop)

Proposed patch only fixes consequences as it is not an upstream bug, it is
a gcc bug. If there would be some trick adding volatile keyword the
resulting code would be too fragile to stay cross compilable and cross
platform. So checking one pointer seems a reasonable price to pay
considering some gcc can do this with a loop and using this kind of loop to
read remaining data is a very common pattern is the project design.

regards
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20160513/f79f8c6c/attachment.html>
-------------- next part --------------
# HG changeset patch
# User Vadim Panov <vadim.v.panov at gmail.com>
# Date 1463114406 -28800
#      Fri May 13 12:40:06 2016 +0800
# Node ID 66f5e8d930f7b51dea90ebda5aa90dcf7fa6fff9
# Parent  db699978a33fe19d786e95e00143eb2b6a1662c0
fix http proxying on version compiled with some old gcc's

diff -r db699978a33f -r 66f5e8d930f7 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c	Thu May 12 16:43:19 2016 +0300
+++ b/src/http/modules/ngx_http_proxy_module.c	Fri May 13 12:40:06 2016 +0800
@@ -1686,6 +1686,15 @@
 
     u = r->upstream;
 
+    /* Pointer to this function seems get cached in a calling loop with
+       some gcc versions. Even this function change the pointer in the end,
+       the caller may still call this function instead of a new one */
+    if (u->process_header != ngx_http_proxy_process_status_line) {
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                       "http proxy: wrong cached u->process_header call");
+        return u->process_header(r);
+    }
+
     rc = ngx_http_parse_status_line(r, &u->buffer, &ctx->status);
 
     if (rc == NGX_AGAIN) {


More information about the nginx-devel mailing list