nuttx/drivers/syslog/syslog_write.c
Marco Casaroli 5a7f1b5005
Some checks are pending
Build Documentation / build-html (push) Waiting to run
MemBrowse Memory Report / changes-filter (push) Waiting to run
MemBrowse Memory Report / load-targets (push) Waiting to run
MemBrowse Memory Report / identical (push) Blocked by required conditions
MemBrowse Memory Report / analyze (push) Blocked by required conditions
drivers/syslog: fix syslog_write() returning -EIO on every write
syslog_write_foreach() compares an unsigned count against a signed
accumulator:

  size_t  nwritten     = 0;
  ssize_t nwritten_max = -EIO;
  ...
  if (nwritten > nwritten_max)
    {
      nwritten_max = nwritten;
    }
  return nwritten_max;

The usual arithmetic conversions promote nwritten_max to size_t, so -EIO
becomes 4294967291 on a 32-bit target, and the comparison is never true.
nwritten_max keeps its initial value and the function returns -EIO no
matter how many bytes actually went out.  Observed under gdb on a running
target: nwritten == 64, nwritten_max == -5, (nwritten > nwritten_max) == 0.

Most callers discard the result -- syslog() itself returns void -- so this
is normally invisible.  It becomes fatal when /dev/console is backed by
syslog_console_write(), because then stdio acts on it.
lib_fflush_unlocked() sees a negative return, sets __FS_FLAG_ERROR and
returns early, before resetting fs_bufpos.  The bytes have already been
emitted, but the buffer is never cleared, so every subsequent stdio call
re-flushes the same CONFIG_STDIO_BUFFER_SIZE bytes.  The console fills
with one repeated fragment and the system makes no further progress.

Reaching that state needs CONFIG_CONSOLE_SYSLOG=y together with no driver
claiming /dev/console ahead of syslog_console_init().  Three in-tree
defconfigs qualify: x86/qemu-i486:ostest, renesas/skp16c26:ostest and
x86_64/qemu-intel64:earlyfb.  The other 56 CONSOLE_SYSLOG configurations
have a serial console that registers /dev/console first, which is why this
has gone unnoticed.

Introduced by 1685e8ff7b ("syslog: avoid an infinite loop if one channel
fails"), which changed nwritten_max from size_t to ssize_t = -EIO so that
an all-channels-failed case could be reported.  Give nwritten the same type
so the comparison is signed, which preserves that intent: nwritten_max
stays -EIO only when no channel wrote anything.  nwritten is never negative,
so the remaining comparisons against buflen are unaffected.

Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 07:57:29 +02:00

264 lines
7.1 KiB
C

/****************************************************************************
* drivers/syslog/syslog_write.c
*
* SPDX-License-Identifier: Apache-2.0
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#include <sys/types.h>
#include <assert.h>
#include <nuttx/arch.h>
#include <nuttx/sched.h>
#include <nuttx/syslog/syslog.h>
#include "syslog.h"
/****************************************************************************
* Private Functions
****************************************************************************/
/****************************************************************************
* Name: syslog_safe_to_block
*
* Description:
* Check if it is safe to block for write. If not, the write defaults to a
* non-blocking method.
*
* Input Parameters:
* None.
*
* Returned Value:
* true if it is safe to block; false otherwise.
*
****************************************************************************/
static bool syslog_safe_to_block(void)
{
#ifdef CONFIG_ENABLE_ALL_SIGNALS
FAR const struct tcb_s *rtcb;
#endif
/* It's not safe to block in interrupts or when executing the idle loop */
if (up_interrupt_context() || sched_idletask())
{
return false;
}
/* It's not safe to block if a signal is being delivered */
#ifdef CONFIG_ENABLE_ALL_SIGNALS
rtcb = nxsched_self();
if (rtcb->sigdeliver != NULL)
{
return false;
}
#endif
return true;
}
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* Name: syslog_write_foreach
*
* Description:
* This provides a default write method for syslog devices that do not
* support multiple byte writes This functions simply loops, outputting
* one character at a time.
*
* Input Parameters:
* buffer - The buffer containing the data to be output
* buflen - The number of bytes in the buffer
*
* Returned Value:
* On success, the number of characters written is returned. A negated
* errno value is returned on any failure.
*
****************************************************************************/
ssize_t syslog_write_foreach(FAR const char *buffer,
size_t buflen, bool force)
{
syslog_write_t write;
syslog_putc_t putc;
ssize_t nwritten = 0;
ssize_t nwritten_max = -EIO;
ssize_t ret;
int i;
for (i = 0; i < CONFIG_SYSLOG_MAX_CHANNELS; i++)
{
FAR syslog_channel_t *channel = g_syslog_channel[i];
if (channel == NULL)
{
break;
}
#ifdef CONFIG_SYSLOG_IOCTL
if (channel->sc_state & SYSLOG_CHANNEL_DISABLE)
{
continue;
}
#endif
write = !force ? channel->sc_ops->sc_write :
channel->sc_ops->sc_write_force;
if (write != NULL)
{
nwritten = 0;
#ifdef CONFIG_SYSLOG_CRLF
if (!(channel->sc_state & SYSLOG_CHANNEL_DISABLE_CRLF))
{
size_t head;
for (head = 0; head < buflen; head++)
{
if (buffer[head] != '\n')
{
continue;
}
ret = write(channel, buffer + nwritten, head - nwritten);
if (ret >= 0)
{
ret = write(channel, "\r\n", 2);
}
if (ret < 0)
{
continue;
}
nwritten = head + 1;
}
}
#endif
if (nwritten < buflen)
{
ret = write(channel, buffer + nwritten, buflen - nwritten);
if (ret < 0)
{
continue;
}
else
{
nwritten += ret;
}
}
}
else
{
putc = !force ? channel->sc_ops->sc_putc :
channel->sc_ops->sc_force;
if (putc == NULL)
{
continue;
}
#ifdef CONFIG_SYSLOG_CRLF
if (channel->sc_state & SYSLOG_CHANNEL_DISABLE_CRLF)
#endif
{
for (nwritten = 0; nwritten < buflen; nwritten++)
{
putc(channel, buffer[nwritten]);
}
}
#ifdef CONFIG_SYSLOG_CRLF
else
{
for (nwritten = 0; nwritten < buflen; nwritten++)
{
if (buffer[nwritten] == '\n')
{
/* Add CR */
putc(channel, '\r');
}
putc(channel, buffer[nwritten]);
}
}
#endif
}
/* Instead of returning the number of bytes
* written to the last channel, returns the maximum
* number of bytes written to any existing channel.
*/
if (nwritten > nwritten_max)
{
nwritten_max = nwritten;
}
}
return nwritten_max;
}
/****************************************************************************
* Name: syslog_write
*
* Description:
* This is the low-level, multiple character, system logging interface.
*
* Input Parameters:
* buffer - The buffer containing the data to be output
* buflen - The number of bytes in the buffer
*
* Returned Value:
* On success, the number of characters written is returned. A negated
* errno value is returned on any failure.
*
****************************************************************************/
ssize_t syslog_write(FAR const char *buffer, size_t buflen)
{
bool force = !syslog_safe_to_block();
#ifdef CONFIG_SYSLOG_INTBUFFER
if (force)
{
syslog_add_intbuffer(buffer, buflen);
return buflen;
}
else
{
/* Flush any characters that may have been added to the interrupt
* buffer.
*/
syslog_flush_intbuffer(false);
}
#endif
return syslog_write_foreach(buffer, buflen, force);
}