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 <[email protected]>
Gerrit-MessageType: newchange
