Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unchecked mallocs in parallel.c #41

Closed
LennartNoordsij opened this issue Jan 10, 2019 · 3 comments
Closed

Unchecked mallocs in parallel.c #41

LennartNoordsij opened this issue Jan 10, 2019 · 3 comments

Comments

@LennartNoordsij
Copy link
Contributor

The function compress_init_par in the file parallel.c contains the following unchecked mallocs

bs = (bitstream**)malloc(chunks * sizeof(bitstream*));

void* buffer = copy ? malloc(size) : (uchar*)stream_data(stream->stream) + stream_size(stream->stream) + block * stream->maxbits / CHAR_BIT;

I uploaded a fix in #40

@LennartNoordsij
Copy link
Contributor Author

Bitstream.c also contains unchecked mallocs for the stream_open and stream_clone functions. If this malloc fails, stream_open/clone returns NULL. In parallel.c, stream_open is called repeatedly (l.55) without verifying if the return value is not NULL. This causes crashes upon memory access or freeing. In order to fix this the mallocs have to be checked.

On failed malloc, would it be best to return an error in bitstream.c or to check for NULL returns in the caller?

@lindstro
Copy link
Member

lindstro commented Feb 5, 2019

Thanks for letting us know. Really the only mallocs in bitstream.c are for the small, 40-byte bitstream struct, and the mallocs themselves are checked. The calling code should check for NULL return values from stream_open and stream_clone, however.

If this malloc fails, you probably have larger problems to worry about, so having a dedicated error code for such a rare occurrence does not make much sense to me. You can't really take any reasonable action in this case other than abort.

We'll add fixes to check the return values.

@lindstro
Copy link
Member

lindstro commented May 7, 2019

The missing malloc tests (and related tests for failure) have been added to the recent 0.5.5 release and are now on the master branch. I'm closing this issue.

@lindstro lindstro closed this as completed May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants