Re: BUG #17920: Incorrect memory access in array_position(s) is detected (or not) - Mailing list pgsql-bugs

From Alexander Lakhin
Subject Re: BUG #17920: Incorrect memory access in array_position(s) is detected (or not)
Date
Msg-id 2006c427-e070-b58a-e6d5-6fefa9f1a268@gmail.com
Whole thread Raw
In response to Re: BUG #17920: Incorrect memory access in array_position(s) is detected (or not)  (Tom Lane <tgl@sss.pgh.pa.us>)
List 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



pgsql-bugs by date:

Previous
From: Stan S
Date:
Subject: Re: BUG #17914: walsenders taking up all memory
Next
From: PG Bug reporting form
Date:
Subject: BUG #17921: Packages missing on reposync