[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] poll: fix leak in ssh_poll_ctx_free


>> 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]);


-Jon

Follow-Ups:
Re: [PATCH] poll: fix leak in ssh_poll_ctx_freeAndreas Schneider <asn@xxxxxxxxxxxxxx>
References:
[PATCH] poll: fix leak in ssh_poll_ctx_freeJon Simons <jon@xxxxxxxxxxxxx>
Re: [PATCH] poll: fix leak in ssh_poll_ctx_freeAndreas Schneider <asn@xxxxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org