Project

General

Profile

Bug #14445

Possible race condition while updating credentials in the factory side

Added by Parag Mhashilkar about 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Parag Mhashilkar
Category:
-
Target version:
Start date:
11/08/2016
Due date:
% Done:

0%

Estimated time:
First Occurred:
Occurs In:
Stakeholders:
Duration:

Description

There is an extremely slight chance of race condition in creation/web_base/update_proxy.py. All the actions are atomic but first action should be copy instead of rename. We rename the current credentials to ".old" before renaming ".new" to current credentials. This should be in terms of micro seconds, nevertheless a possibility of missing credentials.

            # create new file
            fd = os.open(fname + ".new", os.O_CREAT|os.O_WRONLY, 0600)
            try:
                os.write(fd, credential_data)
            finally:
                os.close(fd)

            # move the old file to a tmp and the new one into the official name
            try:
                os.rename(fname, fname + ".old")
            except:
                pass # just protect

            os.rename(fname + ".new", fname)

History

#1 Updated by Parag Mhashilkar about 4 years ago

  • Status changed from New to Feedback
  • Assignee changed from Parag Mhashilkar to Dennis Box

#2 Updated by Dennis Box almost 4 years ago

The shutil.copy* family of functions are not atomic, so there appears to be a small chance that the credential.old file could be corrupted. This is an improvement over the existing small chance that the credential file is not there when some other process tries to read it.

I don't have a suggestion for improving this that would be worth the trouble and not have a high probability of introducing other more subtle race condition problems

#3 Updated by Dennis Box almost 4 years ago

  • Assignee changed from Dennis Box to Parag Mhashilkar

#4 Updated by Dennis Box almost 4 years ago

I thought about it some more, it looks like filling 'fname.new' at line 75 could fail for whatever reason, creating a corrupt credential that would then become the new credential by overwriting 'fname' at line 85.

May want to consider wrapping line 75 with try: except: finally: instead of try:finally: and avoid the overwrite at line 85 if an exception occurred.

#5 Updated by Parag Mhashilkar almost 4 years ago

  • Status changed from Feedback to Resolved

Talked to Dennis and his concern does not apply here. Merged to branch_v3_2

#6 Updated by Parag Mhashilkar almost 4 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF