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

Petr Štetiar ynezz at true.cz
Wed Dec 9 16:58:56 EST 2020


Eneas U de Queiroz <cotequeiroz at gmail.com> [2020-12-09 14:39:06]:

Hi,

> 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. 

I was referencing the missing freeing of allocated resources.

> 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.

Is this micro optimization worth it? You're adding global variable in the
library, you're breaking API layer etc. I'm not supposed to study how is it
implemented _now_, because it will likely change with the next release (either
OpenSSL or wolfSSL) and it might be source of regressions. The API boundary is
given so I'm just trying to use it as designed and as seen in the
docs/examples/tests etc. And there is always new/free combo.

> As for the WIP, you're perhaps doing too much work.

I'm spending time on this mainly because of FS#3465, perhaps mbedTLS has
similar issues[1]. In the end I would like to have uclient/ustream-ssl CI
tested (all 3 SSL libs combinations), with static analyzers, various
sanitizers and Valgrind. So I have to fix all the issues those tools expose.

Maybe it's too much work, but given the constraints (no globals, follow API),
it's currently simplest working solution, but not fully tested yet.

BTW I'm not discouraging you from v2, I've rejected the v1 patch, because it
doesn't fix the memory leak as advertised in the subject :-) Thanks!

1. https://patchwork.ozlabs.org/project/openwrt/patch/trinity-0c56705d-7e2c-482a-a0b5-a3230d3e75b2-1533383113431@3c-app-gmx-bs62/

Cheers,

Petr



More information about the openwrt-devel mailing list