Skip to content

Nullify the hypertable cache pointer on invalidation - #9636

Merged
akuzm merged 2 commits into
mainfrom
oleksii/cache_ptr
May 8, 2026
Merged

Nullify the hypertable cache pointer on invalidation#9636
akuzm merged 2 commits into
mainfrom
oleksii/cache_ptr

Conversation

@alexeyklyukin

@alexeyklyukin alexeyklyukin commented Apr 22, 2026

Copy link
Copy Markdown
Member

Failure to do this results in the possible use after free if hypertable_cache_create fails due to out of memory.

Observed a segfault on the running instance under low memory condition (schema obfuscated):

[1034109]: [69d02db8.fc77d-15] [XX000] ERROR:  cache "(null)" is not initialized

[1034109]: [69d02db8.fc77d-16]  [XX000] STATEMENT:  INSERT INTO sample_table (a_id, timestamp, metric, b_text, c_text, d_text, created, updated, e_bool) VALUES ($1, $2, $3, $4, $5, $6, NOW(), NOW(), false) ON CONFLICT (a_id, timestamp, metric, b_text) DO NOTHING
[891]: [69c81ac3.37b-1174] 0 @,app= [00000] LOG:  server process (PID 1034109) was terminated by signal 11: Segmentation fault
[891]: [69c81ac3.37b-1175] 0 @,app= [00000] DETAIL:  Failed process was running: INSERT INTO sample_table (a_id, timestamp, metric, b_text, c_text, d_text, created, updated, e_bool) VALUES ($1, $2, $3, $4, $5, $6, NOW(), NOW(), false) ON CONFLICT (a_id, timestamp, metric, b_text) DO NOTHING

Disable-check: force-changelog-file

@github-actions

Copy link
Copy Markdown

@Poroma-Banerjee, @svenklemm: please review this pull request.

Powered by pull-review

Comment thread src/hypertable_cache.c Outdated

@akuzm akuzm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline that it makes more sense ts_cache_invalidate, because the core problem that it is supposed to release our pointer to the cache, but it doesn't actually nullify the pointer.

Failure to do this results in the possible use after free if
hypertable_cache_create fails due to out of memory.

Possibly (not reproducible, since there is no OOMs anymore) observed on the running instance with:
```
[1034109]: [69d02db8.fc77d-15] [XX000] ERROR:  cache "(null)" is not initialized

[1034109]: [69d02db8.fc77d-16] 0 tsdbadmin@tsdb,app=PostgreSQL JDBC Driver [XX000] STATEMENT:  INSERT INTO sample_table (a_id, timestamp, metric, b_text, c_text, d_text, created, updated, e_bool) VALUES ($1, $2, $3, $4, $5, $6, NOW(), NOW(), false) ON CONFLICT (a_id, timestamp, metric, b_text) DO NOTHING
[891]: [69c81ac3.37b-1174] 0 @,app= [00000] LOG:  server process (PID 1034109) was terminated by signal 11: Segmentation fault
[891]: [69c81ac3.37b-1175] 0 @,app= [00000] DETAIL:  Failed process was running: INSERT INTO sample_table (a_id, timestamp, metric, b_text, c_text, d_text, created, updated, e_bool) VALUES ($1, $2, $3, $4, $5, $6, NOW(), NOW(), false) ON CONFLICT (a_id, timestamp, metric, b_text) DO NOTHING
@akuzm

akuzm commented Apr 24, 2026

Copy link
Copy Markdown
Member

I could construct a test but it needed an error injection point: #9644

@svenklemm

svenklemm commented Apr 24, 2026

Copy link
Copy Markdown
Member

This seems similar to #9610 so might already be fixed in main

@alexeyklyukin

Copy link
Copy Markdown
Member Author

@svenklemm I think #9610 doesn't have path where cache is pinned correctly, i.e. for a global hyper table cache pointer, the NULL is assigned only when the cache is destroyed; however, if the old cache is pinned, but the global cache pointer is not assigned due to out of memory, it keeps pointing to the soon-to-be-released (at rollback) struct, causing SIGSEGV on access.

@akuzm
akuzm enabled auto-merge (squash) May 8, 2026 09:26
@akuzm
akuzm disabled auto-merge May 8, 2026 09:27
@akuzm
akuzm enabled auto-merge (squash) May 8, 2026 09:27
akuzm added a commit that referenced this pull request May 8, 2026
The bug is fixed by #9636

The test fails only under AddressSanitizer.
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cache.c 33.33% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@akuzm
akuzm merged commit 4f35213 into main May 8, 2026
64 of 68 checks passed
@akuzm
akuzm deleted the oleksii/cache_ptr branch May 8, 2026 09:40
@timescale-automation timescale-automation added the released-2.28.0 Released in 2.28.0 label Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released-2.28.0 Released in 2.28.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants