Opened 5 years ago

Closed 2 years ago

#686 closed defect (wontfix)

With some condition,ngx_palloc() function will alloc a illegal memory address

Reported by: Deming Sun Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.7.x
Keywords: illegal memory address, Cc:
uname -a:
nginx -V: nginx version: nginx/1.7.8

Description

in ngx_palloc.c the function ngx_palloc:

void * ngx_palloc(ngx_pool_t *pool, size_t size)
{
    u_char      *m;
    ngx_pool_t  *p;

    if (size <= pool->max) {

        p = pool->current;

        do {
            m = ngx_align_ptr(p->d.last, NGX_ALIGNMENT);

            if ((size_t) (p->d.end - m) >= size) {
                p->d.last = m + size;

                return m;
            }

            p = p->d.next;

        } while (p);

        return ngx_palloc_block(pool, size);
    }

    return ngx_palloc_large(pool, size);
}

at this line

m = ngx_align_ptr(p->d.last, NGX_ALIGNMENT);
}
at sometimes the value of (p->d.end - p->d.last) may less than align coefficient,then ngx_align_ptr make m larger than p->d.end,after this the "if" compare the value of (p->d.end - m) and size with a type cast,
when m > p->d.end , (p->d.end - m) will get a negative numbera(e.g: -1、-2、-3) .
Underflow happend here and then p->d.last write a address out of p->d.end.
I'm debuging a segmant fault in these days,at last I found 
p->d.last==0x83e96c 
p->d.end==0x83e96f
after ngx_align_ptr
m ==0x83e970
then (size_t) (p->d.end - m)==18446744073709551615 greate larger than size,the p->d.last got a illegl address.
after this ngx_palloc will always alloc illegl address.

there must add a check of p->d.end and m

Attachments (1)

ngx_palloc.c (8.0 KB ) - added by Deming Sun 5 years ago.

Download all attachments as: .zip

Change History (10)

by Deming Sun, 5 years ago

Attachment: ngx_palloc.c added

comment:1 by Ruslan Ermilov, 5 years ago

Status: newaccepted

comment:2 by Ruslan Ermilov, 5 years ago

The problem can only be seen if pool size is not a multiple of sizeof(long). This is not possible with the stock nginx code - all pool sizes are at least 16-bytes aligned (the NGX_POOL_ALIGNMENT macro).

Are you developing your own module or using some 3rd-party modules for nginx?

in reply to:  2 comment:3 by Deming Sun, 5 years ago

Replying to Ruslan Ermilov:

The problem can only be seen if pool size is not a multiple of sizeof(long). This is not possible with the stock nginx code - all pool sizes are at least 16-bytes aligned (the NGX_POOL_ALIGNMENT macro).

Are you developing your own module or using some 3rd-party modules for nginx?

I'm using ngx_lua and pcre.(I do a little modify on ngx_lua,It's using my own pool.).It's happend when ngx_lua call pcre_compile function.pcre use a callback to malloc memory from ngx_pool of the my own pool.
I think if the pool size or alloc size isn't aligned should only decrease performance not cause memory problem.

Last edited 5 years ago by Deming Sun (previous) (diff)

comment:4 by Deming Sun, 5 years ago

I'm confirmd.When the problem happend the poolsize is not aligned with 16-bytes definitely.But i think it's better to don't alloc illegl address when the poolsize or alloc size not aligned.
For example , In the conf file if "connection_pool_size" or "request_pool_size" configured to a number not aligned with 16-bytes may be cause memory corruption.

comment:5 by Maxim Dounin, 5 years ago

You can't set connection_pool_size or request_pool_size to an unaligned value, nginx will complain during configuration parsing, see ngx_http_core_pool_size() function.

in reply to:  5 comment:6 by Deming Sun, 5 years ago

Replying to Maxim Dounin:

You can't set connection_pool_size or request_pool_size to an unaligned value, nginx will complain during configuration parsing, see ngx_http_core_pool_size() function.

ok,I know.

comment:7 by Deming Sun, 5 years ago

Alought nginx stock code has no chance to tigger this problem, but I think it's a "trap" for 3rd-party modules developer.Generally considered the size is not aligned only affect memory performance without causing memory corruption.

comment:8 by Maxim Dounin, 2 years ago

See also #1454.

comment:9 by Maxim Dounin, 2 years ago

Resolution: wontfix
Status: acceptedclosed

The requirements of ngx_create_pool() are currently documented (see f29bd40e9a62):

ngx_create_pool(size, log) — Create a pool with specified block size. The pool object returned is allocated in the pool as well. The size should be at least NGX_MIN_POOL_SIZE and a multiple of NGX_POOL_ALIGNMENT.

This is believed to be enough, no further changes are planned.

Note: See TracTickets for help on using tickets.