Hello!

On Tue, Dec 17, 2013 at 09:05:16PM -0200, Wandenberg Peixoto wrote:

> Hi Maxim,
>
> sorry for the long delay. I hope you remember my issue.
> In attach is the new patch with the changes you suggest.
> Can you check it again? I hope it can be applied to nginx code now.
>
> About this point "2. There is probably no need to check both prev and
> next.",
> I check both pointers to avoid a segmentation fault, since in some
> situations the next can be NULL and the prev may point to pool->free.
> As far as I could follow the code seems to me that could happen one of this
> pointers, next or prev, point to a NULL.
> I just made a double check to protect.

As far as I see, all pages in the pool->free list are expected to
have both p->prev and p->next set. And all pages with type
NGX_SLAB_PAGE will be either on the pool->free list, or will have
p->next set to NULL.

[...]

> > > +{
> > > + ngx_slab_page_t *neighbour = &page[page->slab];
> >
> > Here "neighbour" may point outside of the allocated page
> > structures if we are freeing the last page in the pool.

It looks like you've tried to address this problem with the
following check:

> +static ngx_int_t
> +ngx_slab_merge_pages(ngx_slab_pool_t *pool, ngx_slab_page_t *page)
> +{
> + ngx_slab_page_t *prev, *p;
> +
> + p = &page[page->slab];
> + if ((u_char *) p >= pool->end) {
> + return NGX_DECLINED;
> + }

This looks wrong, as pool->end points to the end of the pool, not
the end of allocated page structures.

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel