-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[fix] Add check on failed allocation in legacy/zstd_v06 #4050
Conversation
I'm trying to follow the same coding style used in: |
4f71e74
to
30546c1
Compare
lib/legacy/zstd_v06.c
Outdated
@@ -3919,6 +3919,7 @@ ZBUFFv06_DCtx* ZBUFFv06_createDCtx(void) | |||
if (zbd==NULL) return NULL; | |||
memset(zbd, 0, sizeof(*zbd)); | |||
zbd->zd = ZSTDv06_createDCtx(); | |||
if (zbd->zd==NULL) return NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning immediately here would result in a memory leak, because zbd
has been allocated at that point.
Free zbd
, using ZBUFFv06_freeDCtx()
, before returning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, indeed. Thanks a lot for the review, I will fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
30546c1
to
0e37337
Compare
lib/legacy/zstd_v06.c
Outdated
@@ -3919,6 +3919,7 @@ ZBUFFv06_DCtx* ZBUFFv06_createDCtx(void) | |||
if (zbd==NULL) return NULL; | |||
memset(zbd, 0, sizeof(*zbd)); | |||
zbd->zd = ZSTDv06_createDCtx(); | |||
if (zbd->zd == NULL) return NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty spaces around ==.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
lib/legacy/zstd_v06.c
Outdated
@@ -3919,6 +3919,7 @@ ZBUFFv06_DCtx* ZBUFFv06_createDCtx(void) | |||
if (zbd==NULL) return NULL; | |||
memset(zbd, 0, sizeof(*zbd)); | |||
zbd->zd = ZSTDv06_createDCtx(); | |||
if (zbd->zd==NULL) return NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, indeed. Thanks a lot for the review, I will fix that.
lib/legacy/zstd_v06.c
Outdated
@@ -3919,6 +3919,7 @@ ZBUFFv06_DCtx* ZBUFFv06_createDCtx(void) | |||
if (zbd==NULL) return NULL; | |||
memset(zbd, 0, sizeof(*zbd)); | |||
zbd->zd = ZSTDv06_createDCtx(); | |||
if (zbd->zd==NULL) return NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
As reported by Ben Hawkes in facebook#4026, a failure to allocate a zstd context would lead to a dereference of a NULL pointer due to a missing check on the returned result of ZSTDv06_createDCtx(). This patch fix the issue by adding a check for valid returned pointer.
0e37337
to
1872688
Compare
@Cyan4973 anything else you like to see changed in the patch? |
Nope, it's good! |
As reported by Ben Hawkes in #4026, a failure to allocate a zstd context would lead to a dereference of a NULL pointer due to a missing check on the returned result of ZSTDv06_createDCtx().
This patch fix the issue by adding a check for valid returned pointer.