Nginx 0.8.41 segfaults in ngx_list_push

Hi,

I’ve found few coredumps in /tmp/ from nginx.
#0 0x00000000004055b8 in ngx_list_push (l=0xdf18e98) at
src/core/ngx_list.c:45
45 if (last->nelts == l->nalloc) {
(gdb) bt
#0 0x00000000004055b8 in ngx_list_push (l=0xdf18e98) at
src/core/ngx_list.c:45
#1 0x0000000000459643 in ngx_http_header_add (r=0xdf18e30, key=0x4686b1
“X-AF-Serial”, value=…)
at /usr/src/redhat/SOURCES/af-headers/ngx_af_headers_module.c:370
#2 0x0000000000459789 in ngx_http_af_header_filter (r=0xdf18e30)
at /usr/src/redhat/SOURCES/af-headers/ngx_af_headers_module.c:422
#3 0x000000000042536c in ngx_http_core_generic_phase (r=0xdf18e98,
ph=0x4686b1)
at src/http/ngx_http_core_module.c:874




(gdb) print last
$2 = (ngx_list_part_t *) 0x0
(gdb) list
40 void *elt;
41 ngx_list_part_t last;
42
43 last = l->last;
44
45 if (last->nelts == l->nalloc) {
46
47 /
the last part is full, allocate a new list part */
48
49 last = ngx_palloc(l->pool, sizeof(ngx_list_part_t));
seems like in this function sometimes l->last could be NULL.
Is it will help if I’ll send patch to fix this problem?

Hi,

seems like in this function sometimes l->last could be NULL.

Not likely, but you’re probably misusing ngx_list.

Are you sure that your “AF headers” module uses either ngx_list_create()
or
ngx_list_init()?

Best regards,
Piotr S. < [email protected] >

Hi,

I’m not using any of this functions and as I can see mod_gzip not
calling this functions too but is uses ngx_list_push.

here is parts of my code:

 static ngx_int_t ngx_http_header_add(ngx_http_request_t *r, char

*key, ngx_str_t value)
{
ngx_table_elt_t *h;

 if (!key || value.len==0)
     return -1;

 h = ngx_list_push(&r->headers_in.headers); <==
 if (h == NULL) {
     return -1;
 }




static ngx_int_t ngx_http_af_header_filter(ngx_http_request_t *r)
{
ngx_http_af_headers_loc_conf_t *afcf =
ngx_http_get_module_loc_conf( r, ngx_http_af_headers_module );
if (!afcf->headers_enabled)
return NGX_OK;

 if (afcf->path && r->connection->sockaddr->sa_family == AF_INET) {
     struct sockaddr_in *sin = (struct sockaddr_in *)

r->connection->sockaddr;
af_struct_t *ret=af_struct_get(r, afcf->path,
sin->sin_addr.s_addr);
if (!ret)
return NGX_OK;
ngx_http_header_add(r, “AF”, ret->af);



static ngx_int_t ngx_http_af_headers_init(ngx_conf_t *cf)
{
ngx_http_core_main_conf_t *cmcf =
ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module);

ngx_http_handler_pt *h =
ngx_array_push(&cmcf->phases[NGX_HTTP_REWRITE_PHASE].handlers);
if (h == NULL) {
return NGX_ERROR;
}

*h = ngx_http_af_header_filter;

ngx_http_next_header_filter = ngx_http_top_header_filter;
ngx_http_top_header_filter = ngx_http_sub_header_filter;

ngx_http_next_body_filter = ngx_http_top_body_filter;
ngx_http_top_body_filter = ngx_http_af_filter;

return NGX_OK;
}

Hello!

On Mon, Jun 21, 2010 at 10:57:06AM -0700, Roman V. wrote:

ngx_table_elt_t *h;

if (!key || value.len==0)
    return -1;

h = ngx_list_push(&r->headers_in.headers); <==

You shouldn’t attempt to modify r->headers_in, it’s not safe
operation to do.

And r->headers_in are known to sometimes be in an inconsistent
state - e.g. in subrequests, where they are partially copied from
parent request, but members needed for manipulations aren’t
correctly initialized (as request headers were already parsed and
no further manipulations expected).

Further reading:

http://nginx.org/pipermail/nginx-devel/2010-February/000132.html
http://nginx.org/pipermail/nginx-devel/2010-February/000133.html

Maxim D.

Hello Maxim,

my module is generating some extra headers for PHP scripts. If I’m
registering input headers generator not correctly how better to do that?

PS: I’ve took this code from mod_rewrite.

Hello!

On Mon, Jun 21, 2010 at 11:34:33AM -0700, Roman V. wrote:

my module is generating some extra headers for PHP scripts. If I’m
registering input headers generator not correctly how better to do
that?

There is no such thing as “input headers generator”. Input
headers are parsed from client’s request. If you want to pass to
backend something different from what you got from client - you
have to do it by means provided by appropriate backend module.
It’s proxy_set_header for proxy, fastcgi_param for fastcgi,
uwsgi_param for uwsgi, scgi_param for fastcgi.

Please follow thread I’ve already linked:

http://nginx.org/pipermail/nginx-devel/2010-February/000135.html

PS: I’ve took this code from mod_rewrite.

There are no r->headers_in manipulations in nginx rewrite
module.

Maxim D.

p.s. Please do not top post. Thank you.

Thank you Maxim for extended description.

Seems like I need some extra advice :slight_smile:
Let me describe my architecture and may be you will see better solution.
I have openvpn server which storing some information at login time to
some temp file and sending throw DHCP local IP address to client.
All 80 port transparently goes throw nginx as forward proxy, results
from nginx in comparing with apache even without keep-alive support on
highly loaded server MUCH better. No problems with memory usage, cpu
utilization, and we have 10% faster browsing. Thank you guys for perfect
web/proxy server.
So… temp file is simple comma separated fields. From nginx part I’m
getting this file by file name which is equals to local IP address,
parsing it and providing to some scripts like input headers. If this
method is unsafe could you recommend me best way to give this fast
changeable session data to my scripts?

Thank you again for fast and full answer.

Hi,

patch below are fixing this problem:
diff -Naur nginx-0.8.42/src/core/ngx_list.c
nginx-0.8.42.new/src/core/ngx_list.c
— nginx-0.8.42/src/core/ngx_list.c 2006-10-11 05:47:11.000000000
-0700
+++ nginx-0.8.42.new/src/core/ngx_list.c 2010-06-21
11:11:04.000000000 -0700
@@ -42,7 +42,7 @@

  last = l->last;
  • if (last->nelts == l->nalloc) {
  • if (last && (last->nelts == l->nalloc)) {

     /* the last part is full, allocate a new list part */

Sounds good,

just one extra question about nginx variables. I’m using for storage of
6 variables 1 file and then splitting this data by strsep. All what I
can find about variable callbacks working revers direction:

  1. I’m requesting some variable;
  2. Callback making some calculations and filling memory with variable

In my case I have to open this file 6 times and make splitting 6 times.
Is it possible to define 6 variables at one step or is possible to save
parsed data in some kind of cache?

Hello!

On Mon, Jun 21, 2010 at 05:32:00PM -0700, Roman V. wrote:

save parsed data in some kind of cache?
Just cache parsed data in module context once first variable
request comes, it’s fairy trivial.

Maxim D.

Hello!

On Mon, Jun 21, 2010 at 02:44:17PM -0700, Roman V. wrote:

Thank you Maxim for extended description.

Seems like I need some extra advice :slight_smile:
Let me describe my architecture and may be you will see better solution.
I have openvpn server which storing some information at login time
to some temp file and sending throw DHCP local IP address to client.
All 80 port transparently goes throw nginx as forward proxy, results

First of all - it’s not a good idea to use nginx as forward proxy.
It was never designed to be used as forward proxy and issues
including security ones are expected.

from nginx in comparing with apache even without keep-alive support
on highly loaded server MUCH better. No problems with memory usage,
cpu utilization, and we have 10% faster browsing. Thank you guys for
perfect web/proxy server.
So… temp file is simple comma separated fields. From nginx part
I’m getting this file by file name which is equals to local IP
address, parsing it and providing to some scripts like input
headers. If this method is unsafe could you recommend me best way to
give this fast changeable session data to my scripts?

The unsafe part is direct modification of r->headers_in. As long
as you’ll use something like

proxy_set_header X-Your-Header $variable_from_your_module;

instead - you are on the safe side.

Maxim D.

On 06/21/2010 06:39 PM, Maxim D. wrote:

  1. I’m requesting some variable;
  2. Callback making some calculations and filling memory with variable

In my case I have to open this file 6 times and make splitting 6 times.
Is it possible to define 6 variables at one step or is possible to
save parsed data in some kind of cache?

Just cache parsed data in module context once first variable
request comes, it’s fairy trivial.

Can I cache some data in request or connection context?