Description:
In kernel builds, any unprivileged process running on the NuttX device
can open /dev/efuse and attempt to read/write fuse content. Reading the
fuses may provide valuable information to an attacker controlling the user
process. The write operation, in extreme cases where the fuse blocks are
not locked, may brick the device.
This is part of https://github.com/apache/nuttx/issues/19410
Compiles ok.
Signed-off-by: Catalin Visinescu <catalin_visinescu@yahoo.com>
Align the NuttX open(2) flag constants with the Linux asm-generic
values so that the FUSE wire protocol and other cross-platform
interfaces work without conversion.
All code that used '(flags & O_RDONLY)' as a bitmask check (always 0
now that O_RDONLY=0) has been updated to use '(flags & O_ACCMODE)'
comparisons.
The NUTTX_O_* constants in include/nuttx/fs/hostfs.h are updated to
match, and the sim hostfs open flag mapping is fixed.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
O_RDOK and O_WROK are non-standard aliases for O_RDONLY and O_WRONLY
respectively. Having two names for the same flag creates confusion,
especially when aligning the flag values with Linux. Remove the
aliases and replace all uses with the standard O_RDONLY/O_WRONLY.
No functional change — O_RDOK was defined as O_RDONLY and O_WROK as
O_WRONLY, so the replacement is a pure text substitution.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
PR https://github.com/apache/nuttx/pull/19139 addresses the issue, but there
is one minor problem. In the for loop the element `i+1` is written which
means there can still be an overflow by one element (uint32_t or 4 bytes).
Addressing here with this PR.
Tested locally, builds fine.
Signed-off-by: Catalin Visinescu <catalin_visinescu@yahoo.com>
An unchecked integer assignment in the CAN driver may result in a division-by-zero which would result in a kernel crash (denial of service).
Tested locally, builds fine.
An unchecked integer assignment in the CAN driver may result in a division-by-zero which would result in a kernel crash (denial of service).
The CAN driver `can_ioctl` function, shown below (drivers/can/can.c), receives commands from user processes. Its received arguments `cmd` and content of `arg` are under attacker's control. In the CAN driver, some `ioctl` commands are hardware specific and are processed by calling `dev_ioctl()`. Subsequently, depending on the hardware (STM32 or AT32), this function calls `fdcan_ioctl` or `at32can_ioctl`, as shown in the snippets that follow.
```c
static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
{
FAR struct inode *inode = filep->f_inode;
FAR struct can_dev_s *dev = inode->i_private;
FAR struct can_reader_s *reader = filep->f_priv;
...
flags = enter_critical_section();
/* Handle built-in ioctl commands */
switch (cmd)
{
...
/* Not a "built-in" ioctl command.. perhaps it is unique to this
* lower-half, device driver. */
default:
{
ret = dev_ioctl(dev, cmd, arg);
}
break;
}
leave_critical_section(flags);
return ret;
}
```
There are a few instances where the user can trigger a kernel crash, caused by a division by zero on `CANIOC_SET_BITTIMING` command. On STM32 platforms (arch/arm/src/stm32/stm32_fdcan.c), while there is a `DEBUGASSERT` assertion for `bt->bt_baud`, the check does not cover value `0`.
```c
static const struct can_ops_s g_fdcanops =
{
...
.co_ioctl = fdcan_ioctl,
...
};
static int fdcan_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg)
{
...
switch (cmd)
{
...
case CANIOC_SET_BITTIMING:
{
const struct canioc_bittiming_s *bt = (const struct canioc_bittiming_s *)arg;
uint32_t nbrp;
uint32_t ntseg1;
uint32_t ntseg2;
uint32_t nsjw;
uint32_t ie;
uint8_t state;
DEBUGASSERT(bt != NULL);
DEBUGASSERT(bt->bt_baud < STM32_FDCANCLK_FREQUENCY); // <- not valid
DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 16);
DEBUGASSERT(bt->bt_tseg1 > 1 && bt->bt_tseg1 <= 64);
DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 16);
/* Extract bit timing data */
ntseg1 = bt->bt_tseg1 - 1;
ntseg2 = bt->bt_tseg2 - 1;
nsjw = bt->bt_sjw - 1;
nbrp = (uint32_t)
( ((float) STM32_FDCANCLK_FREQUENCY /
((float)(ntseg1 + ntseg2 + 3) * (float)bt->bt_baud)) - 1 ); // <- div by 0
```
Similarly, another division by zero was found in Artery Technology AT32 (arch/arm/src/stm32/stm32_can.c) driver:
```c
static const struct can_ops_s g_canops =
{
...
.co_ioctl = at32can_ioctl,
...
};
static int at32can_ioctl(struct can_dev_s *dev, int cmd, unsigned long arg)
{
...
/* Handle the command */
switch (cmd)
{
...
case CANIOC_SET_BITTIMING:
{
const struct canioc_bittiming_s *bt = (const struct canioc_bittiming_s *)arg;
...
uint32_t tmp;
uint32_t regval;
DEBUGASSERT(bt != NULL);
DEBUGASSERT(bt->bt_baud < AT32_PCLK1_FREQUENCY); // <- not valid
DEBUGASSERT(bt->bt_sjw > 0 && bt->bt_sjw <= 4);
DEBUGASSERT(bt->bt_tseg1 > 0 && bt->bt_tseg1 <= 16);
DEBUGASSERT(bt->bt_tseg2 > 0 && bt->bt_tseg2 <= 8);
regval = at32can_getreg(priv, AT32_CAN_BTMG_OFFSET);
/* Extract bit timing data tmp is in clocks per bit time */
tmp = AT32_PCLK1_FREQUENCY / bt->bt_baud; // <- div by 0
```
Instances found are listed in the *Location* section below. They are not shown in detail to reduce the length of the issue.
Ensure the attacker-controlled data is properly validated before use, to stop division by zero situations. For instance:
```c
DEBUGASSERT(bt->bt_baud > 0 && bt->bt_baud < AT32_PCLK1_FREQUENCY);
```
* arch/arm/src/stm32/stm32_fdcan.c
* arch/arm/src/stm32/stm32_can.c
* arch/arm/src/sama5/sam_mcan.c
* arch/arm/src/at32/at32_can.c
* arch/arm/src/samv7/sam_mcan.c
* arch/arm/src/stm32f0l0g0/stm32_fdcan.c
* arch/arm/src/stm32f7/stm32_can.c
* arch/arm/src/stm32h5/stm32_fdcan.c
* arch/arm/src/stm32l4/stm32l4_can.c
* arch/arm/src/tiva/common/tiva_can.c
* drivers/can/mcp2515.c
Signed-off-by: Catalin Visinescu <catalin_visinescu@yahoo.com>
A malformed packet can trigger memory corruption in the kernel leading to a
system crash or potentially arbitrary code execution in the kernel.
The CAN driver for the CTU CAN FD IP Core connected to the NuttX device
via a PCI / PCI Express (PCIe) bus shows a lack of consideration for
malformed data, assuming the CAN frames are always correct.
Ensure `frame->fmt.rwcnt` is 21 or less before it is used in the `for` loop.
A similar change was done in ctucanfd_sock_recv().
Tested locally, builds fine.
Signed-off-by: Catalin Visinescu <catalin_visinescu@yahoo.com>
Always allocate per-file can_reader on open so write-only descriptors get msgalign / ioctl state; free that context on close when it was never linked into cd_readers (fixes leak).
Cap each drain loop (20×500 ms) so close() cannot block forever when bxCAN retries without ACK or dev_txempty never clears.
Fix stm32can_vputreg debug log to print the written value (correct parameter name).
Signed-off-by: Alexey Matveev <tippet@yandex.ru>
debug.h is a NuttX-specific, non-POSIX header. Placing it in the
top-level include/ directory creates naming conflicts with external
projects that define their own debug.h.
This commit moves the canonical header to include/nuttx/debug.h,
following the NuttX convention for non-POSIX/non-standard headers,
and updates all in-tree references.
A backward-compatibility shim is left at include/debug.h that
emits a deprecation #warning and re-includes <nuttx/debug.h>,
allowing out-of-tree code to continue building while migrating.
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
since the judgment for network card selection was changed from IS_UP to
IS_RUNNING, drivers that lack carrier_on need to add the carrier_on
operation; otherwise, network access issues will occur.
Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
The rwcnt (read word count) field in ctucanfd_frame_fmt_s was previously
limited to 4 bits, allowing a maximum value of 15. This is insufficient
to correctly represent the frame size (excluding the FRAME_FORMAT word)
for larger CAN FD frames as required by the CTU CAN FD hardware.
Increase the rwcnt bitfield width to support a larger range and adjust
the structure layout so that ctucanfd_frame_fmt_s has a size that is a
multiple of 4 bytes, as required by the hardware interface.
This change improves correctness and robustness when handling larger
CAN FD frames without affecting existing users.
Signed-off-by: xucheng5 <xucheng5@xiaomi.com>
Replace enter_critical_section() with spinlock-based protection to
avoid sleeping in atomic or interrupt contexts.
Signed-off-by: xucheng5 <xucheng5@xiaomi.com>
The CAN message header was not fully initialized in
ctucanfd_chardev_receive(), which could result in uninitialized
fields and incorrect message contents being delivered to
upper-layer applications.
Signed-off-by: zhaohaiyang1 <zhaohaiyang1@xiaomi.com>
Introduce a single configuration option to provide strict transmit
priority ordering based on CAN ID.
The intention is to expose the user-visible behavior (strict TX
priority ordering) rather than the underlying implementation details.
Strict priority ordering is only meaningful when hardware transmit
buffer cancellation is available, so splitting this functionality into
separate configuration options was misleading and could result in
partially effective or incorrect configurations.
Signed-off-by: zhaohaiyang1 <zhaohaiyang1@xiaomi.com>
O_NONBLOCK open mode was broken since
https://github.com/apache/nuttx/pull/17360
MIN() comapres signed value (int) with unsigned value (size_t) which causes
an unexpected return value when ret is negative
Signed-off-by: raiden00pl <raiden00@railab.me>
This adds ability for read and write operations to work with messages
aligned to configured number of bytes. This has few different use
cases.
The alignment is specified as unsigned integer and can be changed with
ioctl command CANIOC_SET_MSGALIGN. The current value can be queried by
CANIOC_GET_MSGALIGN command.
The default value for the message alignment is 1. This will provide
behavior consistent with current one. Thus messages are placed to the
buffer right after data of the previous one. The same applies for
writes.
The special alignment value 0 disables read and write of multiple frames. Thus
read will always return at most one message and write will always write
only one message even if larger buffer size is provided.
Another use case is if message alignment is set to exactly message
representation size (`sizeof(struct can_msg_s)`). This allows writing
and reading arrays of messages.
Other values provide even more advanced and specialized use cases, such
as optimizations if architecture has to emulate some non-aligned
accesses, there alignment of for example 4 bytes could provide
performance boost.
The original motivation behind this is was compatibility with socket
CAN. It is easier to port applications to NuttX's CAN driver if only one
frame is provided at the time. This solution was suggested by Pavel Pisa
<pisa@fel.cvut.cz> as a more versatile variant of plain boolean
disabling the multiple frame retrieval.
Signed-off-by: Karel Kočí <kkoci@elektroline.cz>
The empty FIFO can happen only in two cases. Either there is some
inconsistency that made rx_sem ready but buffer is empty (that might
happen if multiple reads are blocked in parallel, or just due to the bug
in the logic), or if close is used. Until now the 0 would be returned
and thus EOF. The error is not EOF and actually it is still possible to
read afterwards. Thus we for sure should not signal EOF for the first
cause. The second cause for close matches the behavior that is expressed
even on glibc for some file descriptors (the waiting read will commonly
get error EINVAL there). We can't decide on the real cause of this
situation and thus we always report EIO error and thus generic
input/output error.
Signed-off-by: Karel Kočí <kkoci@elektroline.cz>
The previous version of the code was posting the semaphore every time
the value was less or equal to the zero. In case there is read already
waiting on the semaphore the value will be -1. The first frame will post
the value to 0 and thus unblocking the waiting read. The read will take
frames out of FIFO. During that time if next frame is received it will
be placed to the FIFO but because rx_sem has value 0 at that time it
will be posted to value 1. Once read finishes the execution it will
check if there is something still in the FIFO and posts semaphore back
(expecting that the value is 0) thus increasing value to 2. Thus we end
up with read being unblocked once more triggering 'RX FIFO sem posted
but FIFO is empty.' error branch and returning zero (signaling EOF?).
The intuitive fix is to check the value of the semaphore before posting
it in the read's tail. This could have race condition case when
interrupt is delivered between nxsem_get_value and nxsem_post and
can_receive is called.
This fix instead uses the same condition to detect if semaphore must be
posted as read does. Thus we check if previously the FIFO was empty. We
post the semaphore on in case FIFO was initially empty, otherwise we
expect semaphore to be fully managed by can_read.
Signed-off-by: Karel Kočí <cynerd@email.cz>
The message passed to the write can have `cm.hdr.ch_rtr` set and if
nonzero DLC is specified in the frame then invalid number of bytes is
removed from the buffer as RTR frames do not have any data even when DLC
is nonzero.
This was tested on SAMv7 (mcan). With this change a custom RTR can be
sent.
Signed-off-by: Karel Kočí <cynerd@email.cz>
The message size is being calculated from the message itself. If
application sets value cm_hdr.ch_dlc greater than buflen (that is
size_t) then calculation in while condition underflows and multiple
messages are attempted to be sent.
This check prevents that by verifying that the message size that was
encoded in the dlc is not greater than indicated size of the buffer.
Signed-off-by: Karel Kočí <cynerd@email.cz>
Nuttx currently has 2 types of sleep interfaces:
1. Signal-scheduled sleep: nxsig_sleep() / nxsig_usleep() / nxsig_nanosleep()
Weaknesses:
a. Signal-dependent: The signal-scheduled sleep method is bound to the signal framework, while some driver sleep operations do not depend on signals.
b. Timespec conversion: Signal-scheduled sleep involves timespec conversion, which has a significant impact on performance.
2. Busy sleep: up_mdelay() / up_udelay()
Weaknesses:
a. Does not actively trigger scheduling, occupy the CPU loading.
3. New interfaces: Scheduled sleep: nxsched_sleep() / nxsched_usleep() / nxsched_msleep() / nxsched_ticksleep()
Strengths:
a. Does not depend on the signal framework.
b. Tick-based, without additional computational overhead.
Currently, the Nuttx driver framework extensively uses nxsig_* interfaces. However, the driver does not need to rely on signals or timespec conversion.
Therefore, a new set of APIs is added to reduce dependencies on other modules.
(This PR also aims to make signals optional, further reducing the code size of Nuttx.)
Signed-off-by: chao an <anchao.archer@bytedance.com>
Refactor kvaser_pci.c to use netdev_upperhalf for SocketCAN interface.
The reason for this change is to simplify the driver code and get rid of
duplicate code that may be shared between other network devices.
Signed-off-by: p-szafonimateusz <p-szafonimateusz@xiaomi.com>
Configure number of passes in interrupt handler logic to avoid losing RX frames
in QEMU environment.
Signed-off-by: p-szafonimateusz <p-szafonimateusz@xiaomi.com>
Fix frame reception when CANFD is not enabled and there are many message in
the RX buffer.
The previous implementation assumed that the size of the data in the RX buffer
is constant, which is not true. Fortunately, the amount of
data in the buffer can be easily read from frame->fmt.rwcnt.
Signed-off-by: p-szafonimateusz <p-szafonimateusz@xiaomi.com>
This revises the post-IRQ interruption control logic so that to balance
the disable/enable operations for both chardev and socketcan cases.
Checked with chardev/socketcan on qemu-intel64.
Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
fix an unaligned pointer error for arm32 build:
ctucanfd_pci.c:1361:7: warning: converting a packed 'struct ctucanfd_frame_s'
pointer (alignment 1) to a 'uint32_t' {aka 'long unsigned int'} pointer (alignment 4)
may result in an unaligned pointer value [-Waddress-of-packed-member]
1361 | ptr = (FAR uint32_t *)&rxframe;
Signed-off-by: p-szafonimateusz <p-szafonimateusz@xiaomi.com>
Move can_bytes2dlc() and can_dlc2bytes() to a common CAN file
that can be shared between socketCAN implementation and CAN
character driver.
This is the first step to simplifying the logic repeated in
many CAN drivers.
Signed-off-by: raiden00pl <raiden00@railab.me>
For clearing some variables corresponding with the pollfds of the felip in can_close API, we modify poll logic by binding can_reader_s and pollfd.
Signed-off-by: zhaohaiyang1 <zhaohaiyang1@xiaomi.com>
Corrected CAN FD messages sending in character driver mode.
Assign CAN FD format flag in reception of CAN FD messages.
Corrected some defines mismatches.
The code has been tested in QEMU
qemu-system-x86_64 -m 2G -enable-kvm -smp 1 \
-cpu host,+pcid,+x2apic,+tsc-deadline,+xsave,+rdrand \
-kernel nuttx/nuttx \
-nographic -serial mon:stdio \
-object can-bus,id=canbus0-bus \
-object can-host-socketcan,if=can0,canbus=canbus0-bus,id=canbus0-socketcan \
-device ctucan_pci,canbus0=canbus0-bus,canbus1=canbus0-bus
The overall state of this third party CTU CAN FD driver in NuttX
is far from ideal. It would worth to consult and follow more
closely our Linux kernel driver and even better RTEMS CAN/CAN FD
stack design
https://canbus.pages.fel.cvut.cz/#cancan-fd-subsystem-and-drivers-for-rtems
Signed-off-by: Pavel Pisa <pisa@fel.cvut.cz>
The feature of preventing the same ID frame rotation should be done in the lowerhalf, not in the upperhalf.
Signed-off-by: gaohedong <gaohedong@xiaomi.com>
Most tools used for compliance and SBOM generation use SPDX identifiers
This change brings us a step closer to an easy SBOM generation.
Signed-off-by: Alin Jerpelea <alin.jerpelea@sony.com>
Some macro definitions have already been defined in other header files, redundant macro definitions have been removed in include/nuttx/can.h. Align some code. Remove no use struct (can_response_s) and some variables.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
