diff options
author | Rainer Gerhards <rgerhards@adiscon.com> | 2010-03-25 07:50:55 +0100 |
---|---|---|
committer | Rainer Gerhards <rgerhards@adiscon.com> | 2010-03-25 07:50:55 +0100 |
commit | 9e5b31fc44136dbcc1e443cfe7714e9daf97d844 (patch) | |
tree | 4e31876b298b652181e776bb63de8efce26f5d85 | |
parent | 5d58774813d4ecd4fc9f8230f8d5446457eb2ed5 (diff) | |
download | rsyslog-9e5b31fc44136dbcc1e443cfe7714e9daf97d844.tar.gz rsyslog-9e5b31fc44136dbcc1e443cfe7714e9daf97d844.tar.xz rsyslog-9e5b31fc44136dbcc1e443cfe7714e9daf97d844.zip |
bugfix: race condition during directory creation
If multiple files try to create a directory at (almost) the same time,
some of them may fail. This is a data race and also exists with other
processes that may create the same directory. We do now check for this
condition and gracefully handle it.
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | runtime/srutils.c | 32 |
2 files changed, 31 insertions, 6 deletions
@@ -2,6 +2,11 @@ Version 4.6.2 [v4-stable] (rgerhards), 2010-03-?? - new feature: "." action type added to support writing files to relative pathes (this is primarily meant as a debug aid) +- bugfix: race condition during directory creation + If multiple files try to create a directory at (almost) the same time, + some of them may fail. This is a data race and also exists with other + processes that may create the same directory. We do now check for this + condition and gracefully handle it. - bugfix: potential re-use of free()ed file stream object in omfile when dynaCache is enabled, the cache is full, a new entry needs to be allocated, thus the LRU discarded, then a new entry is opend and that diff --git a/runtime/srutils.c b/runtime/srutils.c index c403b312..1452c9b7 100644 --- a/runtime/srutils.c +++ b/runtime/srutils.c @@ -166,10 +166,22 @@ uchar *srUtilStrDup(uchar *pOld, size_t len) /* creates a path recursively - * Return 0 on success, -1 otherwise. On failure, errno - * hold the last OS error. - * Param "mode" holds the mode that all non-existing directories - * are to be created with. + * Return 0 on success, -1 otherwise. On failure, errno * hold the last OS error. + * Param "mode" holds the mode that all non-existing directories are to be + * created with. + * Note that we have a potential race inside that code, a race that even exists + * outside of the rsyslog process (if multiple instances run, or other programs + * generate directories): If the directory does not exist, a context switch happens, + * at that moment another process creates it, then our creation on the context + * switch back fails. This actually happened in practice, and depending on the + * configuration it is even likely to happen. We can not solve this situation + * with a mutex, as that works only within out process space. So the solution + * is that we take the optimistic approach, try the creation, and if it fails + * with "already exists" we go back and do one retry of the check/create + * sequence. That should then succeed. If the directory is still not found but + * the creation fails in the similar way, we return an error on that second + * try because otherwise we would potentially run into an endless loop. + * loop. -- rgerhards, 2010-03-25 */ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, uid_t uid, gid_t gid, int bFailOnChownFail) @@ -177,6 +189,8 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, uchar *p; uchar *pszWork; size_t len; + int err; + int iTry = 0; int bErr = 0; assert(szFile != NULL); @@ -190,8 +204,9 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, if(*p == '/') { /* temporarily terminate string, create dir and go on */ *p = '\0'; +again: if(access((char*)pszWork, F_OK)) { - if(mkdir((char*)pszWork, mode) == 0) { + if((err = mkdir((char*)pszWork, mode)) == 0) { if(uid != (uid_t) -1 || gid != (gid_t) -1) { /* we need to set owner/group */ if(chown((char*)pszWork, uid, gid) != 0) @@ -201,8 +216,13 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, * to do so. */ } - } else + } else { + if(err == EEXIST && iTry == 0) { + iTry = 1; + goto again; + } bErr = 1; + } if(bErr) { int eSave = errno; free(pszWork); |