<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/15265">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix: vty crash by logging to killed telnet session<br><br>When a telnet session dies (e.g. killall telnet) and also has logging enabled,<br>the closing of the telnet session logs to the killed telnet session and<br>segfaults: the vty->obuf is already NULL.<br><br>In vty_out(), guard against this situation by not composing an output if<br>vty->obuf is NULL.<br><br>Also guard all buffer_*() functions against a NULL buffer argument, which<br>should catch all other hypothetical code paths trying to add to a closed<br>vty->obuf.<br><br>Related: OS#4164<br>Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41<br>---<br>M src/vty/buffer.c<br>M src/vty/vty.c<br>2 files changed, 35 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/15265/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/vty/buffer.c b/src/vty/buffer.c</span><br><span>index e68e3a2..486aafb 100644</span><br><span>--- a/src/vty/buffer.c</span><br><span>+++ b/src/vty/buffer.c</span><br><span>@@ -92,6 +92,8 @@</span><br><span> /* Free buffer. */</span><br><span> void buffer_free(struct buffer *b)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> buffer_reset(b);</span><br><span> talloc_free(b);</span><br><span> }</span><br><span>@@ -104,6 +106,9 @@</span><br><span> char *s;</span><br><span> char *p;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> for (data = b->head; data; data = data->next)</span><br><span> totlen += data->cp - data->sp;</span><br><span> if (!(s = _talloc_zero(tall_vty_ctx, (totlen + 1), "buffer_getstr")))</span><br><span>@@ -120,7 +125,7 @@</span><br><span> /* Return 1 if buffer is empty. */</span><br><span> int buffer_empty(struct buffer *b)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- return (b->head == NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ return (!b || b->head == NULL);</span><br><span> }</span><br><span> </span><br><span> /* Clear and free all allocated data. */</span><br><span>@@ -129,6 +134,9 @@</span><br><span> struct buffer_data *data;</span><br><span> struct buffer_data *next;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> for (data = b->head; data; data = next) {</span><br><span> next = data->next;</span><br><span> BUFFER_DATA_FREE(data);</span><br><span>@@ -141,6 +149,9 @@</span><br><span> {</span><br><span> struct buffer_data *d;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> d = _talloc_zero(b,</span><br><span> offsetof(struct buffer_data, data[b->size]),</span><br><span> "buffer_add");</span><br><span>@@ -161,9 +172,14 @@</span><br><span> /* Write data to buffer. */</span><br><span> void buffer_put(struct buffer *b, const void *p, size_t size)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct buffer_data *data = b->tail;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct buffer_data *data;</span><br><span> const char *ptr = p;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ data = b->tail;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* We use even last one byte of data buffer. */</span><br><span> while (size) {</span><br><span> size_t chunk;</span><br><span>@@ -185,12 +201,16 @@</span><br><span> /* Insert character into the buffer. */</span><br><span> void buffer_putc(struct buffer *b, unsigned char c)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> buffer_put(b, &c, 1);</span><br><span> }</span><br><span> </span><br><span> /* Put string to the buffer. */</span><br><span> void buffer_putstr(struct buffer *b, const char *c)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> buffer_put(b, c, strlen(c));</span><br><span> }</span><br><span> </span><br><span>@@ -202,7 +222,7 @@</span><br><span> struct buffer_data *head;</span><br><span> size_t head_sp;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!b->head)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b || !b->head)</span><br><span> return BUFFER_EMPTY;</span><br><span> head_sp = (head = b->head)->sp;</span><br><span> /* Flush all data. */</span><br><span>@@ -395,6 +415,9 @@</span><br><span> size_t iovcnt = 0;</span><br><span> size_t nbyte = 0;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return BUFFER_EMPTY;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> for (d = b->head; d && (iovcnt < MAX_CHUNKS) && (nbyte < MAX_FLUSH);</span><br><span> d = d->next, iovcnt++) {</span><br><span> iov[iovcnt].iov_base = d->data + d->sp;</span><br><span>@@ -440,6 +463,8 @@</span><br><span> {</span><br><span> ssize_t nbytes;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (!b)</span><br><span style="color: hsl(120, 100%, 40%);">+ return BUFFER_ERROR;</span><br><span> #if 0</span><br><span> /* 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... */</span><br><span> </span><br><span>diff --git a/src/vty/vty.c b/src/vty/vty.c</span><br><span>index a96d86c..27e35fe 100644</span><br><span>--- a/src/vty/vty.c</span><br><span>+++ b/src/vty/vty.c</span><br><span>@@ -260,6 +260,13 @@</span><br><span> vprintf(format, ap);</span><br><span> } else {</span><br><span> va_list args;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!vty->obuf) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* There is no output buffer. This can happen from logging to a telnet session, during cleanup</span><br><span style="color: hsl(120, 100%, 40%);">+ * of this same (killed) telnet session. See OS#4146. */</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Try to write to initial buffer. */</span><br><span> va_copy(args, ap);</span><br><span> len = vsnprintf(buf, sizeof buf, format, args);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/15265">change 15265</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/15265"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41 </div>
<div style="display:none"> Gerrit-Change-Number: 15265 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>