From def2d2aebad1a6d1b149780181943e0cafa63bdc Mon Sep 17 00:00:00 2001 From: Mathieu Parent Date: Tue, 8 Mar 2016 16:56:50 +0100 Subject: [PATCH 1/3] createOutputFile: rename already existing file See https://bugs.debian.org/734688 Closes #23 Upstream-commit: fc1c3eff61edf8e9f0a4bfa980f3a6030a6b271f Signed-off-by: Kamil Dudka --- logrotate.c | 20 ++++++++++++++++++-- test/test | 26 ++++++++++++++++++++++++++ test/test-config.72.in | 7 +++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 test/test-config.72.in diff --git a/logrotate.c b/logrotate.c index f13d140..976210e 100644 --- a/logrotate.c +++ b/logrotate.c @@ -395,8 +395,24 @@ static int runScript(struct logInfo *log, char *logfn, char *script) int createOutputFile(char *fileName, int flags, struct stat *sb, acl_type acl, int force_mode) { int fd; - struct stat sb_create; - int acl_set = 0; + struct stat sb_create; + int acl_set = 0; + + if (stat(fileName, &sb_create) == 0) { + /* the destination file already exists, while it should not */ + struct tm now = *localtime(&nowSecs); + size_t fileName_size = strlen(fileName); + char* backupName = alloca(fileName_size + sizeof("-YYYYMMDDHH.backup")); + strncpy(backupName, fileName, fileName_size); + size_t date_size=strftime(backupName+fileName_size, 12, "-%Y%m%d%H", &now); + strncpy(backupName+fileName_size+date_size, ".backup\0", 8); + message(MESS_ERROR, "destination %s already exists, renaming to %s\n", fileName, backupName); + if (rename(fileName, backupName) != 0) { + message(MESS_ERROR, "error renaming already existing output file %s to %s: %s\n", + fileName, backupName, strerror(errno)); + return -1; + } + } fd = open(fileName, (flags | O_EXCL | O_NOFOLLOW), (S_IRUSR | S_IWUSR) & sb->st_mode); diff --git a/test/test b/test/test index bcdfe05..b47ac6a 100755 --- a/test/test +++ b/test/test @@ -1586,6 +1586,32 @@ EOF rm -rf testdir adir rm -rf testdir bdir +cleanup 72 + +# ------------------------------- Test 72 ------------------------------------ +preptest test.log 72 2 + +$RLR test-config.72 --force + +checkoutput < test.log.1.gz + +$RLR test-config.72 --force +dt="$(date +%Y%m%d%H)" + +checkoutput < Date: Mon, 17 Oct 2016 17:59:31 +0200 Subject: [PATCH 2/3] createOutputFile: eliminate stat/open TOCTOU race Upstream-commit: aff4a30807218a52b6b5f200c5aa0eea335547ba Signed-off-by: Kamil Dudka --- logrotate.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/logrotate.c b/logrotate.c index 976210e..fdc81ac 100644 --- a/logrotate.c +++ b/logrotate.c @@ -394,11 +394,18 @@ static int runScript(struct logInfo *log, char *logfn, char *script) int createOutputFile(char *fileName, int flags, struct stat *sb, acl_type acl, int force_mode) { - int fd; + int fd = -1; struct stat sb_create; int acl_set = 0; + int i; + + for (i = 0; i < 2; ++i) { + fd = open(fileName, (flags | O_EXCL | O_NOFOLLOW), + (S_IRUSR | S_IWUSR) & sb->st_mode); + + if ((fd >= 0) || (errno != EEXIST)) + break; - if (stat(fileName, &sb_create) == 0) { /* the destination file already exists, while it should not */ struct tm now = *localtime(&nowSecs); size_t fileName_size = strlen(fileName); @@ -412,11 +419,9 @@ int createOutputFile(char *fileName, int flags, struct stat *sb, acl_type acl, i fileName, backupName, strerror(errno)); return -1; } + /* existing file renamed, try it once again */ } - fd = open(fileName, (flags | O_EXCL | O_NOFOLLOW), - (S_IRUSR | S_IWUSR) & sb->st_mode); - if (fd < 0) { message(MESS_ERROR, "error creating output file %s: %s\n", fileName, strerror(errno)); -- 2.20.1 From 86d67f5769575104add7846b822069008c912b3c Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Mon, 17 Oct 2016 18:13:32 +0200 Subject: [PATCH 3/3] createOutputFile: improve code readability Upstream-commit: 5ecd908033a481c1e127ba583697d6662ffea4a3 Signed-off-by: Kamil Dudka --- logrotate.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/logrotate.c b/logrotate.c index fdc81ac..5b0df30 100644 --- a/logrotate.c +++ b/logrotate.c @@ -409,14 +409,24 @@ int createOutputFile(char *fileName, int flags, struct stat *sb, acl_type acl, i /* the destination file already exists, while it should not */ struct tm now = *localtime(&nowSecs); size_t fileName_size = strlen(fileName); - char* backupName = alloca(fileName_size + sizeof("-YYYYMMDDHH.backup")); - strncpy(backupName, fileName, fileName_size); - size_t date_size=strftime(backupName+fileName_size, 12, "-%Y%m%d%H", &now); - strncpy(backupName+fileName_size+date_size, ".backup\0", 8); - message(MESS_ERROR, "destination %s already exists, renaming to %s\n", fileName, backupName); + size_t buf_size = fileName_size + sizeof("-YYYYMMDDHH.backup"); + char *backupName = alloca(buf_size); + char *ptr = backupName; + + /* construct backupName starting with fileName */ + strcpy(ptr, fileName); + ptr += fileName_size; + buf_size -= fileName_size; + + /* append the -YYYYMMDDHH time stamp and the .backup suffix */ + ptr += strftime(ptr, buf_size, "-%Y%m%d%H", &now); + strcpy(ptr, ".backup"); + + message(MESS_ERROR, "destination %s already exists, renaming to %s\n", + fileName, backupName); if (rename(fileName, backupName) != 0) { - message(MESS_ERROR, "error renaming already existing output file %s to %s: %s\n", - fileName, backupName, strerror(errno)); + message(MESS_ERROR, "error renaming already existing output file" + " %s to %s: %s\n", fileName, backupName, strerror(errno)); return -1; } /* existing file renamed, try it once again */ -- 2.20.1