Thread: Memory leak in pg_hmac_final
Hi, Here is a patch fixing the subject. Regards, -- Sergey Shinderuk https://postgrespro.com/
Attachment
> 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