Memory leak in pg_hmac_final

On 1 Oct 2021, at 12:39, Sergey Shinderuk <s.shinderuk@postgrespro.ru> wrote:

Here is a patch fixing the subject.

Seems reasonable on a quick glance, the interim h buffer should be freed (this
is present since 14). I'll have another look at this in a bit and will take
care of it.

--
Daniel Gustafsson https://vmware.com/

On 01.10.2021 15:05, Daniel Gustafsson wrote:

On 1 Oct 2021, at 12:39, Sergey Shinderuk <s.shinderuk@postgrespro.ru> wrote:

Here is a patch fixing the subject.

Seems reasonable on a quick glance, the interim h buffer should be freed (this
is present since 14). I'll have another look at this in a bit and will take
care of it.

Thanks. I found it using the leaks tool on macOS.

Without the patch:

% MallocStackLogging=1 leaks -quiet -atExit -- psql -d 'dbname=postgres
user=alice password=secret' -XAtc 'select 1'
...
Process 91635: 4390 nodes malloced for 252 KB
Process 91635: 4103 leaks for 131296 total leaked bytes.
...

(User alice has a SCRAM-encrypted password.)

With the patch:

Process 98250: 290 nodes malloced for 124 KB
Process 98250: 3 leaks for 96 total leaked bytes.

The remaining leaks are expected and not worth fixing, I guess:

STACK OF 1 INSTANCE OF 'ROOT LEAK: malloc<32>':
4 libdyld.dylib 0x7fff68d80cc9 start + 1
3 psql 0x10938b9f9 main + 2393
startup.c:207
2 psql 0x1093ab5a5 pg_malloc + 21
fe_memutils.c:49
1 libsystem_malloc.dylib 0x7fff68f36cf5 malloc + 21
0 libsystem_malloc.dylib 0x7fff68f36d9e malloc_zone_malloc
+ 140
====
2 (48 bytes) ROOT LEAK: 0x7ffbb75040d0 [32]
1 (16 bytes) 0x7ffbb75040f0 [16] length: 8 "select 1"

STACK OF 1 INSTANCE OF 'ROOT LEAK: malloc<48>':
5 libdyld.dylib 0x7fff68d80cc9 start + 1
4 psql 0x10938b8b0 main + 2064
startup.c:207
3 psql 0x1093ab78e pg_strdup + 14
fe_memutils.c:96
2 libsystem_c.dylib 0x7fff68e26ce6 strdup + 32
1 libsystem_malloc.dylib 0x7fff68f36cf5 malloc + 21
0 libsystem_malloc.dylib 0x7fff68f36d9e malloc_zone_malloc
+ 140
====
1 (48 bytes) ROOT LEAK: 0x7ffbb75040a0 [48] length: 42
"dbname=postgres user=alice password=secret"

--
Sergey Shinderuk https://postgrespro.com/

On 1 Oct 2021, at 14:31, Sergey Shinderuk <s.shinderuk@postgrespro.ru> wrote:

On 01.10.2021 15:05, Daniel Gustafsson wrote:

On 1 Oct 2021, at 12:39, Sergey Shinderuk <s.shinderuk@postgrespro.ru> wrote:

Here is a patch fixing the subject.

Seems reasonable on a quick glance, the interim h buffer should be freed (this
is present since 14). I'll have another look at this in a bit and will take
care of it.

Patch pushed to master and 14.

Thanks. I found it using the leaks tool on macOS.

Nice, I hadn't heard of that before but it seems quite neat.

--
Daniel Gustafsson https://vmware.com/

On Fri, Oct 01, 2021 at 10:58:07PM +0200, Daniel Gustafsson wrote:

Nice, I hadn't heard of that before but it seems quite neat.

Thanks for the fix, it looks fine. I just saw the thread. Perhaps
the commit log should have said that this only impacts non-OpenSSL
builds. Worth noting that in ~13 we used a static buffer for "h" in
the SCRAM code, as its size was known thanks to SHA-256.
--
Michael