ngx_chain_get_free_buf crashes with free=0 after return next_body_filter

[Ruslan Khusnullin]

On Mon, Dec 3, 2012 at 10:09 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Note: returning NGX_HTTP_INTERNAL_SERVER_ERROR is completely wrong
> here, and it will result in undefined behaviour if ever returned.
>
> You should either return NGX_ERROR here (probably logging an alert
> message) or just assume it should never happen unless process
> memory is corrupted and just dereference the pointer (this way
> you'll get proper segmentation fault instead of undefined
> behviour).

Thank you for this, I have replaced all NGX_HTTP_INTERNAL_SERVER_ERROR
with NGX_ERROR in body_filter and now it doesn't crash.

So is returning NGX_HTTP_*_ERROR valid only in a handler function?

>> Here is what I've catched from gdb:
>> (gdb) bt
>> #0  ngx_chain_get_free_buf (p=0x80f5020, free=0x0) at src/core/ngx_buf.c:160
>> #1  0x08089fbe in ngx_http_chunked_body_filter (r=0x80de458, in=0xbfffe798)
>> at src/http/modules/ngx_http_chunked_filter_module.c:150
>
> This part of the backtrace suggests the r->chunked flag was
> somehow set outside of chunked filter, with chunked filter context
> not set (== NULL).

Strange, I didn't touch other modules' contexts but may be it is
really related to invallid returning above.

Could you please also comment below piece of code? I've got it from
previous coders and I'm not sure it needs to be there. It is a version
of ngx_chain_add_copy that really copies data instead of copying
pointers. Does it make sense? The module copies all chunks from
upstream cache with this function and then when meeting buf->last_buf,
it composes whole response in one memory block and modifies it. I
guess that all the chunks remain in memory for all request time or I
really need to make a copy of data in my context?

static ngx_int_t
chain_add_copy (ngx_pool_t * pool, ngx_chain_t ** chain, ngx_chain_t *
in, size_t * data_size) {
    ngx_chain_t * cl, ** ll;
    size_t len;

    ll = chain;
    for (cl = * chain; cl != NULL; cl = cl->next) {
        ll = & cl->next;
    }

    while (in) {
        len = in->buf->last - in->buf->pos;

        cl = ngx_alloc_chain_link (pool);
        if (cl == NULL) {
            return NGX_ERROR;
        }

        cl->buf = ngx_create_temp_buf (pool, len);
        if (cl->buf == NULL) {
            return NGX_ERROR;
        }

        cl->buf->last = ngx_copy (cl->buf->last, in->buf->pos, len);
        * data_size += len;

        * ll = cl;
        ll = & cl->next;
        in = in->next;
    }

    * ll = NULL;

    return NGX_OK;
}

Thanks in advance.