Skip to content

prevent possible segfault when creating seek table#4201

Merged
Cyan4973 merged 1 commit into
facebook:devfrom
rorosen:seek-table-create-null-check
Nov 27, 2024
Merged

prevent possible segfault when creating seek table#4201
Cyan4973 merged 1 commit into
facebook:devfrom
rorosen:seek-table-create-null-check

Conversation

@rorosen

@rorosen rorosen commented Nov 24, 2024

Copy link
Copy Markdown
Contributor

Add a check in ZSTD_seekTable_create_fromSeekable whether the seek table of a ZSTD_seekable is initialized before creating a new seek table from it. Formerly, the function created a segfault when the seek table of ZSTD_seekablewas not initialized.

Fixes #4200


ZSTD_seekTable* ZSTD_seekTable_create_fromSeekable(const ZSTD_seekable* zs)
{
if (zs->seekTable.entries == NULL) return NULL;

@Cyan4973 Cyan4973 Nov 25, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we are dereferencing zs here, I would prefer an assert(zs != NULL); just before this statement.

Arguably, it should have been added a few lines below too, but wasn't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I added the assert

@Cyan4973 Cyan4973 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indeed, given that ZSTD_seekTable is essentially a limited extract from ZSTD_seekable, it involves copying some content from one side to the other.
And it's a problem if the source doesn't exist (is not initialized).
This simple check is a good fix.

@Cyan4973 Cyan4973 self-assigned this Nov 25, 2024
Add a check whether the seek table of a `ZSTD_seekable` is initialized
before creating a new seek table from it. Return `NULL`, if the check
fails.
@rorosen
rorosen force-pushed the seek-table-create-null-check branch from affa711 to b683c0d Compare November 25, 2024 07:57
@Cyan4973
Cyan4973 merged commit 82d4705 into facebook:dev Nov 27, 2024
@rorosen
rorosen deleted the seek-table-create-null-check branch January 10, 2025 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZSTD_seekTable_create_fromSeekable can cause segmentation faults

3 participants