[PATCH ustream] ustream-openssl: fix bio memory leak

[Eneas U de Queiroz]

On Wed, Dec 9, 2020 at 1:45 PM Petr Štetiar <ynezz at true.cz> wrote:
>
> Eneas U de Queiroz <cotequeiroz at gmail.com> [2020-12-09 13:06:45]:
>
> Hi,
>
> > Using the patch by Pan Chen as inspiration, this avoids a memory leak by
> > using a global BIO_METHOD pointer that doesn't ordinarily need to be
> > freed.
>
> this sounds weird, how is global pointer avoiding memory leaks? :-)


BIO_METHOD was made opaque by openssl 1.1.0.  It's just a table of
methods, and it does change.  Before that, one would just fill the
struct without having to make any calls.
I am the one responsible for introducing the bug in 34b0b80
[ustream-ssl: add openssl-1.1.0 compatibility].  The old, openssl 1.0
code was just:

static BIO_METHOD methods_ustream = {
       100 | BIO_TYPE_SOURCE_SINK,
       "ustream",
       s_ustream_write,
       s_ustream_read,
       s_ustream_puts,
       s_ustream_gets,
       s_ustream_ctrl,
       s_ustream_new,
       s_ustream_free,
       NULL,
};

So the answer to your question is because you only allocate the table
if methods_ustream is NULL, and it will point to the created table
then.  The table won't change during the lifetime of the process, and
will get freed only when the process ends.

We could free it in s_ustream_free, but only to have to create it
again with the same data the next time ustream_bio_new is called. I
wouldn't do it, but if you'd rather, I can add it in a v2.

>
> > CC: Pan Chen <serial115200 at outlook.com>
> >
> > Signed-off-by: Eneas U de Queiroz <cotequeiroz at gmail.com>
> >
> > ---
> > Run-tested with a WRT-3200ACM, running uclient_fetch and uhttpd.
> > I have not run it with valgrind or any other debugger.
>
> how do you otherwise verify the correctness? :-) FYI this is my work in progress[1].
>
> 1. https://gitlab.com/ynezz/openwrt-ustream-ssl/-/commit/807ce1de752e021802a563783dfa580950746a0c

As for testing I don't have valgrind running, so I wasn't able to do
it; but someone else can.  That's why I made sure to point it out.  As
for the WIP, you're perhaps doing too much work.

Cheers,

Eneas