Hello Maxim,

did you have opportunity to take a look on this last patch?


Regards,
Wandenberg


On Thu, Dec 26, 2013 at 10:12 PM, Wandenberg Peixoto
<wandenberg@gmail.com>wrote:

> Hello Maxim,
>
> I changed the patch to check only the p->next pointer.
> And checking if the page is in an address less than the (pool->pages +
> pages).
>
> + ngx_slab_page_t *prev, *p;
> + ngx_uint_t pages;
> + size_t size;
> +
> + size = pool->end - (u_char *) pool - sizeof(ngx_slab_pool_t);
> + pages = (ngx_uint_t) (size / (ngx_pagesize +
> sizeof(ngx_slab_page_t)));
> +
> + p = &page[page->slab];
> +
> + if ((p < pool->pages + pages) &&
> + (p->next != NULL) &&
> + ((p->prev & NGX_SLAB_PAGE_MASK) == NGX_SLAB_PAGE)) {
>
>
> I hope that now I did the right checks.
>
> Regards,
> Wandenberg
>
>
> On Fri, Dec 20, 2013 at 2:49 PM, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
>> 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
>>
>
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel