Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Markov <imarkov@tarantool.org>
To: georgy@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [log 1/1] log: Fix logging large objects
Date: Wed, 28 Mar 2018 13:35:14 +0300	[thread overview]
Message-ID: <5644fb6343e31f6e136071bb5659f893ac176400.1522233290.git.imarkov@tarantool.org> (raw)

The bug was that logging we passed to function write
number of bytes which may be more than size of buffer.
This may happen because formatting log string we use vsnprintf which
returns number of bytes would be written to buffer, not the actual
number.

Fix this with limiting number of bytes passing to write function.

Close #3248
---
 src/say.c                    | 24 ++++++++++++++++++------
 test/app-tap/logger.test.lua |  9 +++++++--
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/src/say.c b/src/say.c
index b92514d..8a8d46a 100644
--- a/src/say.c
+++ b/src/say.c
@@ -842,6 +842,18 @@ say_format_syslog(struct log *log, char *buf, int len, int level, const char *fi
 enum { SAY_BUF_LEN_MAX = 16 * 1024 };
 static __thread char buf[SAY_BUF_LEN_MAX];
 
+/**
+ * Wrapper over write which ensures, that writes not more than buffer size.
+ */
+static ssize_t
+safe_write(int fd, const char *buf, int size)
+{
+	/* Writes at most SAY_BUF_LEN_MAX - 1
+	 * (1 byte was taken for 0 byte in vsnprintf).
+	 */
+	return write(fd, buf, MIN(size, SAY_BUF_LEN_MAX - 1));
+}
+
 static void
 say_default(int level, const char *filename, int line, const char *error,
 	    const char *format, ...)
@@ -852,7 +864,7 @@ say_default(int level, const char *filename, int line, const char *error,
 	int total = log_vsay(log_default, level, filename,
 			     line, error, format, ap);
 	if (level == S_FATAL && log_default->fd != STDERR_FILENO) {
-		ssize_t r = write(STDERR_FILENO, buf, total);
+		ssize_t r = safe_write(STDERR_FILENO, buf, total);
 		(void) r;                       /* silence gcc warning */
 	}
 
@@ -870,7 +882,7 @@ write_to_file(struct log *log, int total)
 	       log->type == SAY_LOGGER_PIPE ||
 	       log->type == SAY_LOGGER_STDERR);
 	assert(total >= 0);
-	ssize_t r = write(log->fd, buf, total);
+	ssize_t r = safe_write(log->fd, buf, total);
 	(void) r;                               /* silence gcc warning */
 }
 
@@ -882,7 +894,7 @@ write_to_syslog(struct log *log, int total)
 {
 	assert(log->type == SAY_LOGGER_SYSLOG);
 	assert(total >= 0);
-	if (log->fd < 0 || write(log->fd, buf, total) <= 0) {
+	if (log->fd < 0 || safe_write(log->fd, buf, total) <= 0) {
 		/*
 		 * Try to reconnect, if write to syslog has
 		 * failed. Syslog write can fail, if, for example,
@@ -899,7 +911,7 @@ write_to_syslog(struct log *log, int total)
 			 * it would block thread. Try to reconnect
 			 * on next vsay().
 			 */
-			ssize_t r = write(log->fd, buf, total);
+			ssize_t r = safe_write(log->fd, buf, total);
 			(void) r;               /* silence gcc warning */
 		}
 	}
@@ -1041,11 +1053,11 @@ log_vsay(struct log *log, int level, const char *filename, int line,
 	case SAY_LOGGER_SYSLOG:
 		write_to_syslog(log, total);
 		if (level == S_FATAL && log->fd != STDERR_FILENO)
-			(void) write(STDERR_FILENO, buf, total);
+			(void) safe_write(STDERR_FILENO, buf, total);
 		break;
 	case SAY_LOGGER_BOOT:
 	{
-		ssize_t r = write(STDERR_FILENO, buf, total);
+		ssize_t r = safe_write(STDERR_FILENO, buf, total);
 		(void) r;                       /* silence gcc warning */
 		break;
 	}
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 72d13d3..85f6166 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 
 local test = require('tap').test('log')
-test:plan(23)
+test:plan(24)
 
 --
 -- Check that Tarantool creates ADMIN session for #! script
@@ -91,6 +91,11 @@ local line = file:read()
 message = json.decode(line)
 test:is(message.message, "this is \"", "check message with escaped character")
 
+-- gh-3248 trash in log file with logging large objects
+log.info(string.rep('a', 32000))
+line = file:read()
+test:ok(line:len() < 20000, "big line truncated")
+
 log.info("json")
 local line = file:read()
 message = json.decode(line)
@@ -100,7 +105,7 @@ log.info("hello")
 line = file:read()
 test:ok(not line:match("{"), "log change format")
 s, e = pcall(log.log_format, "non_format")
-test:ok(not s)
+test:ok(not s, "bad format")
 file:close()
 
 log.log_format("json")
-- 
2.7.4

             reply	other threads:[~2018-03-28 10:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-28 10:35 Ilya Markov [this message]
2018-03-29  7:40 ` [tarantool-patches] " Georgy Kirichenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5644fb6343e31f6e136071bb5659f893ac176400.1522233290.git.imarkov@tarantool.org \
    --to=imarkov@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [log 1/1] log: Fix logging large objects' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox