Re: [PATCH] poll: fix leak in ssh_poll_ctx_free

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Thursday 31 October 2013 01:18:54 Jon Simons wrote:
> >> Attached is a patch which I believe should address the memory
> >> leak reported here: https://red.libssh.org/issues/128.
> > 
> > I think you're right about the free, but the loop will not work,
> > cause it only frees the first element. Check the attached patch.
> 
> The loop in the original patch is correct, I believe: the codepath
> 'ssh_poll_free' -> 'ssh_poll_ctx_remove' has the side-effect that
> it will always ensure to refill the slot that was made empty by
> the removal, if the slot was not the last one:
> 
>   547 void ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p) {
>   548   size_t i;
>   549
>   550   i = p->x.idx;
>   551   p->x.fd = ctx->pollfds[i].fd;
>   552   p->ctx = NULL;
>   553
>   554   ctx->polls_used--;
>   555
>   556   /* fill the empty poll slot with the last one */
>   557   if (ctx->polls_used > 0 && ctx->polls_used != i) {
>   558     ctx->pollfds[i] = ctx->pollfds[ctx->polls_used];
>   559     ctx->pollptrs[i] = ctx->pollptrs[ctx->polls_used];
>   560     ctx->pollptrs[i]->x.idx = i;
>   561   }
> 
> So this loop should ensure to remove and free all entries:
> 
>   while (ctx->polls_used > 0)
>     ssh_poll_free(ctx->pollptrs[0]);

These two lines of code are an infinite loop :)

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx