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

[agw][mme] Make ctime call thread safe #4216

Merged
merged 1 commit into from Dec 31, 2020

Conversation

ssanadhya
Copy link
Collaborator

Signed-off-by: Shruti Sanadhya ssanadhya@fb.com

Summary

MME service encountered seg faults due to memory corruption while calling ctime. This change replaces ctime with ctime_r

Test Plan

  • Unable to reproduce the original memory corruption
  • Sanity check with make integ_test

Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
@ssanadhya ssanadhya added component: agw Access gateway-related issue apply-v1.3 Needs to be applied to v1.3 release branch as well labels Dec 30, 2020
@ssanadhya ssanadhya self-assigned this Dec 30, 2020
@magmabot magmabot added the component: cwag CWAG related issues label Dec 30, 2020
Copy link
Contributor

@ulaskozat ulaskozat left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@pshelar pshelar left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -563,7 +563,8 @@ static char* log_get_readable_cur_time(time_t* cur_time) {
// get the current local time
*cur_time = time(NULL);
// get the current local time in readable string format
return (strtok(ctime(cur_time), "\n"));
char buf[26];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is exact buf required, I would gone with larger buffer than required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pshelar missed this comment before merging this PR. Could you please suggest an alternative size for the buffer? I can fix it in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

32 bytes.

@ssanadhya ssanadhya merged commit cb595fd into magma:master Dec 31, 2020
tmdzk pushed a commit that referenced this pull request Jan 1, 2021
Signed-off-by: Shruti Sanadhya <ssanadhya@fb.com>
@tmdzk
Copy link
Collaborator

tmdzk commented Jan 1, 2021

backported

@ssanadhya ssanadhya added the backported-v1.3 Has been backported to v1.3 release branch label Jan 1, 2021
@ssanadhya ssanadhya deleted the thread_safe_ctime branch December 6, 2021 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apply-v1.3 Needs to be applied to v1.3 release branch as well backported-v1.3 Has been backported to v1.3 release branch component: agw Access gateway-related issue component: cwag CWAG related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants