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

Fixed the crash issue when a pointer is null. #24381

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ant-J
Copy link

@Ant-J Ant-J commented May 13, 2024

When using libcurl, at the time the process is about to exit, openssl will invoke the err_cleanup function, setting err_string_lock to NULL. If other threads are still calling libcurl at this moment, it will cause a crash in the int_err_get_item function.

当我使用libcrul,在进程要退出的时候,openssl会调用err_cleanup将err_string_lock赋值为NULL,此时其他线程还在调用libcurl,在int_err_get_item函数中会产生崩溃

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label May 13, 2024
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label May 13, 2024
@@ -188,6 +188,8 @@ static int err_string_data_cmp(const ERR_STRING_DATA *a,
static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)
{
ERR_STRING_DATA *p = NULL;
if(err_string_lock == NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a couple of minor issues with this edit: lack of blank line between variable declaration and beginning of code (which was there before this edit) and missing space after if before (.

A smaller change could be to change the old line 192 from

   if (!CRYPTO_THREAD_read_lock(err_string_lock))

to

   if (err_string_lock == NULL || !CRYPTO_THREAD_read_lock(err_string_lock))

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that this also does not fix all potential issues with the teardown. I'd instead recommend building openssl with no-atexit configure option.

@t8m t8m added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch hold: cla required The contributor needs to submit a license agreement severity: fips change The pull request changes FIPS provider sources triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants