Use of uninitialized memory in pg_bigm

[robert-pang]

In my environment, I ran pg_bigm's regression tests with memory sanitization checks turned on and got a warning about a use of uninitialized memory below:

==2630==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5595cb3cdc43 in ginFillScanEntry postgres/src/backend/access/gin/ginscan.c:72:6
#1 0x5595cb3cdc43 in ginFillScanKey postgres/src/backend/access/gin/ginscan.c:221:23
#2 0x5595cb3cd045 in ginNewScanKey postgres/src/backend/access/gin/ginscan.c:372:3
#3 0x5595cb3ce63d in gingetbitmap postgres/src/backend/access/gin/ginget.c:1827:2
#4 0x5595cb2024ef in index_getbitmap postgres/src/backend/access/index/indexam.c:600:10
#5 0x5595cb309512 in MultiExecBitmapIndexScan postgres/src/backend/executor/nodeBitmapIndexscan.c:89:23
#6 0x5595cb2c6b5d in BitmapHeapNext postgres/src/backend/executor/nodeBitmapHeapscan.c:104:23
#7 0x5595cb2c8687 in ExecScan postgres/src/backend/executor/execScan.c:180:10
#8 0x5595cb2b8e84 in ExecProcNode postgres/src/backend/executor/execProcnode.c:437:13
#9 0x5595cb205914 in ExecutePlan postgres/src/backend/executor/execMain.c:1567:10
#10 0x5595cb205914 in standard_ExecutorRun postgres/src/backend/executor/execMain.c:339:3
#11 0x5595cb66bffe in PortalRunSelect postgres/src/backend/tcop/pquery.c:948:4
#12 0x5595cb66b556 in PortalRun postgres/src/backend/tcop/pquery.c:789:18
#13 0x5595cb664bbe in exec_simple_query postgres/src/backend/tcop/postgres.c:1114:10
#14 0x5595cb664bbe in PostgresMain postgres/src/backend/tcop/postgres.c:4185:7
#15 0x5595cb56d7b9 in BackendRun postgres/src/backend/postmaster/postmaster.c:4430:2
#16 0x5595cb56c094 in BackendStartup postgres/src/backend/postmaster/postmaster.c:4096:3
#17 0x5595cb56c094 in ServerLoop postgres/src/backend/postmaster/postmaster.c:1759:7
#18 0x5595cb5669e4 in PostmasterMain postgres/src/backend/postmaster/postmaster.c:1367:11
#19 0x5595cb1662ca in main postgres/src/backend/main/main.c:228:3

LOG: server process (PID 2630) exited with exit code 66
DETAIL: Failed process was running: SELECT col1 FROM test_bigm WHERE col1 LIKE likequery('%');
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory.

I looked into it and recognized that pg_bigm allocates a bool and stores it extra_data in gin_extract_query_bigm. However, Postgres' GIN index expects the extractQuery implementation to store an array of *nkeys pointers (doc) in extra_data. As Postgres tries to copy the array pointed to by extra_data, it results in the warning.

Do we think it is a real issue and if and how it can be resolved? Thanks.

[MasaoFujii]

@robert-pang
Thanks for the report!

At first I'd like to reproduce this issue on my laptop. But I'm not sure how to use the memory sanitizer when building PostgreSQL extension. Could you tell me the steps to do that on macOS? Just build PostgreSQL as usual but with CFLAGS="-fsanitize=memory" even on macOS?

[robert-pang]

To use MemorySanitizer in Postgres, we will need to map Postgres' Valgrind calls in memdebug.h to MemorySanitizer's counterparts. Instead, you can use Postgres' existing support for Valgrind and I believe you should get the same result.

[howladr]

Hi,

We have also encountered the issue and have identified the problem.

There is a bug in the pg_bigm code that can cause segmentation faults in the system when it interacts with the GIN index. This is due to trying to access memory that has not been allocated.

The gin index code strictly states that if extra_data is initialized, then we'll want to iterate through an array of Pointers, refer to ginFillScanKey in ginscan.c. However pg_bigm only allocates memory for sizeof(bool) i.e. 1 byte, code

On rare occasions, the bug would cause segmentation faults as a result of attempting to access unallocated memory. The rarity in nature is due to the way we allocate memory in Postgres; palloc will always allocate you a “chunk of memory” from the memory context. If lucky, we may end up accessing memory, still within the memory context provided by the system, although, this would be garbage data that we are reading in.

A proposed fix is to allocate *extra_data with an array of Pointers which matches the cardinality that is expected by Postgres/GIN index.

[MasahikoSawada]

I agree with your analysis. The documentation of extractQuery() interface says:

To use it, extractQuery must allocate an array of *nkeys pointers and store its address at *extra_data, then store whatever it wants to into the individual pointers.

And as howladr referred to ginscan.c code, the gin codes passes an element of the extra_data array.

@howladr could you send a PR to fix this issue?

[MasaoFujii]

@howladr @MasahikoSawada
I've opened a PR to fix this memory allocation bug:
PR #23

I haven’t fully tested the patch yet, so I’d appreciate any reviews and testing.

[howladr]

Hi @MasaoFujii @MasahikoSawada,
Thank you for opening up a PR, I've reviewed the fix and it LGTM upon first look. I'll also do some tests from my side and follow up on this thread later next week with my findings.

[robert-pang]

Thanks for the fix. I gave it a test and confirmed that the uninitialized memory access is resolved.

[robert-pang]

Now that this issue is resolved, another array-bound-read issue is reported and I have PR #26 to address it. I will appreciate your review and feedback. Thank you.

[howladr]

Thank you for the fix @MasaoFujii! I've also confirmed from my side that the memory bug issue is resolved and installchecks are still passing.

[MasaoFujii]

@robert-pang @howladr @MasahikoSawada
Thanks for reviewing the PR! I’ve pushed the fix.

@robert-pang
Regarding the new array-bound-read issue you mentioned — thanks for catching that! I’ll review PR #26 shortly.