From def2d2aebad1a6d1b149780181943e0cafa63bdc Mon Sep 17 00:00:00 2001

From: Mathieu Parent <Mathieu.PARENT@nantesmetropole.fr>

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 <kdudka@redhat.com>

---

 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 0

+test.log.1 0 zero

+test.log.2.gz 1 first

+EOF

+

+echo 'unexpected' > test.log.1.gz

+

+$RLR test-config.72 --force

+dt="$(date +%Y%m%d%H)"

+

+checkoutput <

+test.log 0

+test.log.1 0

+test.log.1.gz-$dt.backup 0 unexpected

+test.log.2.gz 1 zero

+test.log.3.gz 1 first

+EOF

+

 cleanup 73

 # ------------------------------- Test 73 ------------------------------------

diff --git a/test/test-config.72.in b/test/test-config.72.in

new file mode 100644

index 0000000..9fe50a2

--- /dev/null

+++ b/test/test-config.72.in

@@ -0,0 +1,7 @@

+&DIR&/test.log {

+    daily

+    rotate 3

+    compress

+    delaycompress

+    create

+}

-- 

2.17.2

From 1fa3ff8477d3766f0cc39631ace8c63ee99a1974 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

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 <kdudka@redhat.com>

---

 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 <kdudka@redhat.com>

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 <kdudka@redhat.com>

---

 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