Change in ...libosmocore[master]: fix: vty crash by logging to killed telnet session

[neels]

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/15265


Change subject: fix: vty crash by logging to killed telnet session
......................................................................

fix: vty crash by logging to killed telnet session

When a telnet session dies (e.g. killall telnet) and also has logging enabled,
the closing of the telnet session logs to the killed telnet session and
segfaults: the vty->obuf is already NULL.

In vty_out(), guard against this situation by not composing an output if
vty->obuf is NULL.

Also guard all buffer_*() functions against a NULL buffer argument, which
should catch all other hypothetical code paths trying to add to a closed
vty->obuf.

Related: OS#4164
Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41
---
M src/vty/buffer.c
M src/vty/vty.c
2 files changed, 35 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/15265/1

diff --git a/src/vty/buffer.c b/src/vty/buffer.c
index e68e3a2..486aafb 100644
--- a/src/vty/buffer.c
+++ b/src/vty/buffer.c
@@ -92,6 +92,8 @@
 /* Free buffer. */
 void buffer_free(struct buffer *b)
 {
+	if (!b)
+		return;
 	buffer_reset(b);
 	talloc_free(b);
 }
@@ -104,6 +106,9 @@
 	char *s;
 	char *p;
 
+	if (!b)
+		return NULL;
+
 	for (data = b->head; data; data = data->next)
 		totlen += data->cp - data->sp;
 	if (!(s = _talloc_zero(tall_vty_ctx, (totlen + 1), "buffer_getstr")))
@@ -120,7 +125,7 @@
 /* Return 1 if buffer is empty. */
 int buffer_empty(struct buffer *b)
 {
-	return (b->head == NULL);
+	return (!b || b->head == NULL);
 }
 
 /* Clear and free all allocated data. */
@@ -129,6 +134,9 @@
 	struct buffer_data *data;
 	struct buffer_data *next;
 
+	if (!b)
+		return;
+
 	for (data = b->head; data; data = next) {
 		next = data->next;
 		BUFFER_DATA_FREE(data);
@@ -141,6 +149,9 @@
 {
 	struct buffer_data *d;
 
+	if (!b)
+		return NULL;
+
 	d = _talloc_zero(b,
 			 offsetof(struct buffer_data, data[b->size]),
 			 "buffer_add");
@@ -161,9 +172,14 @@
 /* Write data to buffer. */
 void buffer_put(struct buffer *b, const void *p, size_t size)
 {
-	struct buffer_data *data = b->tail;
+	struct buffer_data *data;
 	const char *ptr = p;
 
+	if (!b)
+		return;
+
+	data = b->tail;
+
 	/* We use even last one byte of data buffer. */
 	while (size) {
 		size_t chunk;
@@ -185,12 +201,16 @@
 /* Insert character into the buffer. */
 void buffer_putc(struct buffer *b, unsigned char c)
 {
+	if (!b)
+		return;
 	buffer_put(b, &c, 1);
 }
 
 /* Put string to the buffer. */
 void buffer_putstr(struct buffer *b, const char *c)
 {
+	if (!b)
+		return;
 	buffer_put(b, c, strlen(c));
 }
 
@@ -202,7 +222,7 @@
 	struct buffer_data *head;
 	size_t head_sp;
 
-	if (!b->head)
+	if (!b || !b->head)
 		return BUFFER_EMPTY;
 	head_sp = (head = b->head)->sp;
 	/* Flush all data. */
@@ -395,6 +415,9 @@
 	size_t iovcnt = 0;
 	size_t nbyte = 0;
 
+	if (!b)
+		return BUFFER_EMPTY;
+
 	for (d = b->head; d && (iovcnt < MAX_CHUNKS) && (nbyte < MAX_FLUSH);
 	     d = d->next, iovcnt++) {
 		iov[iovcnt].iov_base = d->data + d->sp;
@@ -440,6 +463,8 @@
 {
 	ssize_t nbytes;
 
+	if (!b)
+		return BUFFER_ERROR;
 #if 0
 	/* Should we attempt to drain any previously buffered data?  This could help reduce latency in pushing out the data if we are stuck in a long-running thread that is preventing the main select loop from calling the flush thread... */
 
diff --git a/src/vty/vty.c b/src/vty/vty.c
index a96d86c..27e35fe 100644
--- a/src/vty/vty.c
+++ b/src/vty/vty.c
@@ -260,6 +260,13 @@
 		vprintf(format, ap);
 	} else {
 		va_list args;
+
+		if (!vty->obuf) {
+			/* There is no output buffer. This can happen from logging to a telnet session, during cleanup
+			 * of this same (killed) telnet session. See OS#4146. */
+			return 0;
+		}
+
 		/* Try to write to initial buffer.  */
 		va_copy(args, ap);
 		len = vsnprintf(buf, sizeof buf, format, args);

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15265
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41
Gerrit-Change-Number: 15265
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190821/06a1d3b2/attachment.htm>