Horton

Thread

Commits

  1. Fix memory leak in pg_hmac

  1. Memory leak in pg_hmac_final

    Sergey Shinderuk <s.shinderuk@postgrespro.ru> — 2021-10-01T10:39:40Z

    Hi,

Here is a patch fixing the subject.

Regards,

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

  2. Re: Memory leak in pg_hmac_final

    Daniel Gustafsson <daniel@yesql.se> — 2021-10-01T12:05:05Z

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

  3. Re: Memory leak in pg_hmac_final

    Sergey Shinderuk <s.shinderuk@postgrespro.ru> — 2021-10-01T12:31:23Z

    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/

  4. Re: Memory leak in pg_hmac_final

    Daniel Gustafsson <daniel@yesql.se> — 2021-10-01T20:58:07Z

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

  5. Re: Memory leak in pg_hmac_final

    Michael Paquier <michael@paquier.xyz> — 2021-10-02T05:25:44Z

    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