"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: leak in got_fetch_pack
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 13 Feb 2022 23:37:54 +0100

Download raw body.

Thread
On Sun, Feb 13, 2022 at 10:49:05PM +0100, Omar Polo wrote:
> Sure, but I fail to see how it's leaking if imsg_clear is called outside
> of the loop.
> 
> If I'm reading correctly imsg{,-buf}.c all it does is to dequeue pending
> ibufs and file descriptors and frees/closes them, so while I agree that
> doing it inside the loop (possibly) frees these imsgbuf internal queues
> more quickly, I don't see how it avoids leaking memory.
> 
> I'm clearly missing something obvious here, sorry, I just don't see it.

This API is not very intuitive, and the imsg_init(3) man page does
not clearly explain when imsg_clear(3) is intended to be used.

Looking at the code it seems that only the writing side of the imsg
channel is supposed to call imsg_clear() ever?

The reader receives the data in a single buffer (ibuf->r.buf) which
is pre-allocated as part of struct ibuf, so we don't need to free it
since it lives on the stack.

And any file descriptors that arrive in an ibuf are copied into struct
imsg by imsg_get() and in such cases we close imsg->fd ourselves or
copy its value elsewhere.

So imsg_clear() at the receiving end looks like a no-op?