Re: BUG #17920: Incorrect memory access in array_position(s) is detected (or not)

From: Alexander Lakhin <exclusion(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org, Richard Guo <guofenglinux(at)gmail(dot)com>
Subject: Re: BUG #17920: Incorrect memory access in array_position(s) is detected (or not)
Date: 2023-05-05 03:00:00
Message-ID: 2006c427-e070-b58a-e6d5-6fefa9f1a268@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

04.05.2023 18:53, Tom Lane wrote:
>> ==00:00:00:04.955 1823064== at 0x5EDBD0: array_positions
>> (array_userfuncs.c:806)
> Fixed, thanks for the report!

Thank you for the fix!

>> I was surprised by the fact, that Valgrind doesn't complain for the single
>> query execution:
>> SELECT array_positions('{}'::int[], 1)
>> IIUC, this is explained by the lack of red zones around palloc'ed memory
>> areas.
>> sed "s/VALGRIND_CREATE_MEMPOOL(\(.*\), 0, false);/VALGRIND_CREATE_MEMPOOL(\1, 16, false);/" -i
>> src/backend/utils/mmgr/mcxt.c
>> on master (after 414d66220) helps Valgrind to detect the invalid read on
>> the single query execution too:
> I didn't try valgrind'ing this on HEAD, but I think this situation
> might have been improved by 414d66220.

Exactly. On 414d66220~1, with the redzones added
(VALGRIND_CREATE_MEMPOOL(..., 16, ...) [1]) even initdb fails:
running bootstrap script ... ==00:00:00:00.782 2770024== Invalid read of size 4
==00:00:00:00.782 2770024==    at 0x7729F8: GetMemoryChunkMethodID (mcxt.c:195)
==00:00:00:00.782 2770024==    by 0x7729F8: pfree (mcxt.c:1439)
==00:00:00:00.782 2770024==    by 0x3D7D8D: check_temp_tablespaces (tablespace.c:1304)
==00:00:00:00.782 2770024==    by 0x757492: call_string_check_hook (guc.c:6838)
==00:00:00:00.782 2770024==    by 0x7595B6: InitializeOneGUCOption (guc.c:1689)
==00:00:00:00.782 2770024==    by 0x75BC10: InitializeGUCOptions (guc.c:1520)
==00:00:00:00.782 2770024==    by 0x2CE4DD: BootstrapModeMain (bootstrap.c:215)
==00:00:00:00.782 2770024==    by 0x4694F3: main (main.c:189)
==00:00:00:00.782 2770024==  Address 0x73b7fd0 is 8 bytes before a block of size 1 client-defined
==00:00:00:00.782 2770024==    at 0x77186D: MemoryContextAlloc (mcxt.c:1035)
==00:00:00:00.782 2770024==    by 0x7731FB: MemoryContextStrdup (mcxt.c:1616)
==00:00:00:00.782 2770024==    by 0x773225: pstrdup (mcxt.c:1626)
==00:00:00:00.782 2770024==    by 0x3D7B46: check_temp_tablespaces (tablespace.c:1210)
==00:00:00:00.782 2770024==    by 0x757492: call_string_check_hook (guc.c:6838)
==00:00:00:00.782 2770024==    by 0x7595B6: InitializeOneGUCOption (guc.c:1689)
==00:00:00:00.782 2770024==    by 0x75BC10: InitializeGUCOptions (guc.c:1520)
==00:00:00:00.782 2770024==    by 0x2CE4DD: BootstrapModeMain (bootstrap.c:215)
==00:00:00:00.782 2770024==    by 0x4694F3: main (main.c:189)
==00:00:00:00.782 2770024==

But on HEAD `make check` passes for me.
So may be it makes sense to add the redzones for v16?/v17 to catch
such errors more reliably?
Though it's not clear to me, how many previously missed cases could be
detected with the redzones. Anyway, I'm going to perform tests with
this additional protection and share my findings.

[1] https://valgrind.org/docs/manual/mc-manual.html#mc-manual.mempools

Best regards,
Alexander

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2023-05-05 07:12:15 BUG #17921: Packages missing on reposync
Previous Message Stan S 2023-05-04 16:24:03 Re: BUG #17914: walsenders taking up all memory