[PATCH] Fastcgi: core dump was caused by duplicated request header

Simon Liu simohayha.bobo at gmail.com
Mon Jun 13 18:56:55 MSD 2011


Thank you very much!

this is my new patch, I use bitmask and ngx_list.

Index: src/http/modules/ngx_http_fastcgi_module.c
===================================================================
--- src/http/modules/ngx_http_fastcgi_module.c    (revision 3937)
+++ src/http/modules/ngx_http_fastcgi_module.c    (working copy)
@@ -691,11 +691,13 @@
     u_char                        ch, *pos, *lowcase_key;
     size_t                        size, len, key_len, val_len, padding,
                                   allocated;
-    ngx_uint_t                    i, n, next, hash, header_params;
+    uint64_t                     *bitmask;
+    ngx_uint_t                    i, k, n, next, hash, bm_index;
     ngx_buf_t                    *b;
+    ngx_list_t                    ignore_bitmask;
     ngx_chain_t                  *cl, *body;
-    ngx_list_part_t              *part;
-    ngx_table_elt_t              *header, **ignored;
+    ngx_list_part_t              *part, *bitmask_part;
+    ngx_table_elt_t              *header;
     ngx_http_script_code_pt       code;
     ngx_http_script_engine_t      e, le;
     ngx_http_fastcgi_header_t    *h;
@@ -703,8 +705,8 @@
     ngx_http_script_len_code_pt   lcode;

     len = 0;
-    header_params = 0;
-    ignored = NULL;
+    bm_index = 0;
+    bitmask = NULL;

     flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);

@@ -737,16 +739,24 @@
         lowcase_key = NULL;

         if (flcf->header_params) {
-            ignored = ngx_palloc(r->pool, flcf->header_params * sizeof(void
*));
-            if (ignored == NULL) {
+            if (ngx_list_init(&ignore_bitmask, r->pool, 3,
sizeof(uint64_t))
+                != NGX_OK)
+            {
                 return NGX_ERROR;
             }
+
+            bitmask = ngx_list_push(&ignore_bitmask);
+            if (bitmask == NULL) {
+                return NGX_ERROR;
+            }
+
+            *bitmask = 0;
         }

         part = &r->headers_in.headers.part;
         header = part->elts;

-        for (i = 0; /* void */; i++) {
+        for (i = 0, k = 0; /* void */; i++, k++) {

             if (i >= part->nelts) {
                 if (part->next == NULL) {
@@ -767,6 +777,16 @@
                     }
                 }

+                if (k >= sizeof(uint64_t) * 8) {
+                    bitmask = ngx_list_push(&ignore_bitmask);
+                    if (bitmask == NULL) {
+                        return NGX_ERROR;
+                    }
+
+                    *bitmask = 0;
+                    k = 0;
+                }
+
                 hash = 0;

                 for (n = 0; n < header[i].key.len; n++) {
@@ -784,7 +804,7 @@
                 }

                 if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
n)) {
-                    ignored[header_params++] = &header[i];
+                    *bitmask |= (uint64_t) 1 << k;
                     continue;
                 }

@@ -903,8 +923,13 @@
         part = &r->headers_in.headers.part;
         header = part->elts;

-        for (i = 0; /* void */; i++) {
+        if (flcf->header_params) {
+            bitmask_part = &ignore_bitmask.part;
+            bitmask = bitmask_part->elts;
+        }

+        for (i = 0, k = 0; /* void */; i++, k++) {
+
             if (i >= part->nelts) {
                 if (part->next == NULL) {
                     break;
@@ -915,10 +940,26 @@
                 i = 0;
             }

-            for (n = 0; n < header_params; n++) {
-                if (&header[i] == ignored[n]) {
-                    goto next;
+            if (flcf->header_params) {
+
+                if (k >= sizeof(uint64_t) * 8) {
+                    k = 0;
+                    bm_index++;
                 }
+
+                if (bm_index >= bitmask_part->nelts) {
+                    if (bitmask_part->next == NULL) {
+                        break;
+                    }
+
+                    bitmask_part = bitmask_part->next;
+                    bitmask = bitmask_part->elts;
+                    bm_index = 0;
+                }
+
+                if (bitmask[bm_index] & ((uint64_t) 1 << k)) {
+                    continue;
+                }
             }

             key_len = sizeof("HTTP_") - 1 + header[i].key.len;
@@ -964,9 +1005,6 @@
                            "fastcgi param: \"%*s: %*s\"",
                            key_len, b->last - (key_len + val_len),
                            val_len, b->last - val_len);
-        next:
-
-            continue;
         }
     }




On Sat, Jun 11, 2011 at 8:30 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Fri, Jun 10, 2011 at 04:55:55PM +0800, Simon Liu wrote:
>
> > Thanks!
> >
> > I refactor the prev patch.
> >
> > this is new patch:
>
> [...]
>
> > @@ -707,7 +769,10 @@
> >      ignored = NULL;
> >
> >      flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);
> > +    cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
> >
> > +    allow_underscores = cscf->underscores_in_headers;
> > +
>
> This is a layering violation.  This is also a good example of why
> layering violation is bad: this is incorrect as underscores may as
> well appear in headers with "ignore_invalid_headers off;", even if
> undescores_in_headers is off.
>
> >      if (flcf->params_len) {
> >          ngx_memzero(&le, sizeof(ngx_http_script_engine_t));
> >
> > @@ -784,6 +849,14 @@
> >                  }
> >
> >                  if (ngx_hash_find(&flcf->headers_hash, hash,
> lowcase_key,
> > n)) {
> > +
> > +                    if (header_params == flcf->header_params
> > +                       || ngx_http_fastcgi_ignored_header(ignored,
> > &header[i],
> > +                           header_params, allow_underscores) == NGX_OK)
> > +                    {
> > +                        continue;
> > +                    }
> > +
> >                      ignored[header_params++] = &header[i];
>
> I would suggest using bitmask sized after number of headers in
> request instead.
>
> Alternatively, nginx dynamically sized array or list may be used
> for ignored, this should be a bit closer to original code.  Though
> I personally think bitmask would be better.
>
> Maxim Dounin
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://nginx.org/mailman/listinfo/nginx-devel
>



-- 
博观约取

豆瓣:www.douban.com/people/mustang/

blog: www.pagefault.info

twitter: www.twitter.com/minibobo

sina 微博:  www.weibo.com/diaoliang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110613/49a3fb1a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fastcgi.patch
Type: text/x-patch
Size: 4471 bytes
Desc: not available
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110613/49a3fb1a/attachment.bin>


More information about the nginx-devel mailing list