Skip to content

[components][serial] Harden RT-Smart serial ioctl arguments - #11733

Open
BernardXiong wants to merge 3 commits into
RT-Thread:masterfrom
BernardXiong:fix/pr-11453-serial-ioctl
Open

[components][serial] Harden RT-Smart serial ioctl arguments#11733
BernardXiong wants to merge 3 commits into
RT-Thread:masterfrom
BernardXiong:fix/pr-11453-serial-ioctl

Conversation

@BernardXiong

Copy link
Copy Markdown
Member

Description

Follow up on #11453 to harden the remaining RT-Smart serial ioctl paths.

  • Marshal known serial ioctl payloads at the v1/v2 file boundary and copy results back to user space.
  • Protect v2 RT_SERIAL_CTRL_GET_UNREAD_BYTES_COUNT and use the correct struct termios buffer for v2 termios commands.
  • Reject invalid baud rates before imx6ull UART register updates and commit serial configuration only after hardware configuration succeeds.

Validation

  • arm-none-eabi-gcc -fsyntax-only for v1/v2 serial sources with LWP disabled.
  • arm-linux-musleabi-gcc -fsyntax-only for v1/v2 serial sources and imx6ull UART driver with a temporary Musl/LWP configuration.
  • Full imx6ull-smart SCons build is blocked by the existing Newlib/LWP configuration error in components/lwp/lwp.h.

Changes

  • components/drivers/serial/dev_serial.c
  • components/drivers/serial/dev_serial_v2.c
  • bsp/nxp/imx/imx6ull-smart/drivers/drv_uart.c

在 v1/v2 串口文件边界复制已知用户参数,补齐 v2 unread count 和 termios 缓冲区保护。

在 imx6ull UART 编程前校验波特率,并在硬件配置成功后提交串口状态。
@BernardXiong
BernardXiong requested a review from Rbb666 as a code owner August 23, 2026 02:11
@github-actions github-actions Bot added the BSP: NXP Code related with NXP label Aug 23, 2026
@github-actions

Copy link
Copy Markdown

👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).


🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • Use workflow from 保持默认分支(通常为 master
    Keep the default branch (usually master) in Use workflow from
  • branch 输入框填写 PR 分支 fix/pr-11453-serial-ioctl
    Enter PR branch fix/pr-11453-serial-ioctl in the branch field
  • 设置需排除的文件/目录(目录请以"/"结尾)
    Set files/directories to exclude (directories should end with "/")
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将作为独立提交推送至你的分支。
    The formatting changes will be pushed to your branch as a separate commit.

完成后,提交将自动更新至 fix/pr-11453-serial-ioctl 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the fix/pr-11453-serial-ioctl branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们,再次感谢您的贡献!💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

📌 Code Review Assignment

🏷️ Tag: components

Reviewers: @Maihuanyi

Changed Files (Click to expand)
  • components/drivers/serial/dev_serial.c
  • components/drivers/serial/dev_serial_v2.c

🏷️ Tag: components_driver_serial_v2

Reviewers: @Ryan-CW-Code

Changed Files (Click to expand)
  • components/drivers/serial/dev_serial_v2.c

📊 Current Review Status (Last Updated: 2026-08-23 11:32 CST)


📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

at32f415-start

  • ROM: .text +32 B (+0.0%, 65,400 B / 262,144 B, total: 25% used)

esp32-c3

  • iram0_2_seg: .flash.text +16 B (+0.0%, 229,594 B / 8,388,576 B, total: 3% used)

gd32105r-start

  • CODE: .text +32 B (+0.0%, 69,316 B / 262,144 B, total: 26% used)

hc32f334

  • FLASH: .text +32 B (+0.0%, 99,528 B / 131,072 B, total: 76% used)

hpmicro-hpm5301evklite

  • XPI0: .text -24 B (-0.0%, 124,968 B / 1,048,576 B, total: 12% used)

infineon-psoc6

  • flash: .text +32 B (+0.0%, 116,172 B / 262,144 B, total: 44% used)

k230

  • SRAM: .eh_frame +112 B, .text +112 B (+0.0%, 1,123,227 B / 268,300,288 B, total: 0% used)

loongson-ls1cdev

  • Code: .text +56 B (+0.0%, 363,304 B)

nuvoton-m487

  • CODE: .text +8 B (+0.0%, 355,940 B / 524,288 B, total: 68% used)

qemu-virt64-aarch64

  • Code: .text +64 B (+0.0%, 1,070,276 B)

raspberry-pico-rp2040

  • FLASH: .text +32 B (+0.0%, 114,136 B / 2,097,152 B, total: 5% used)

renesas-ra2l1

  • FLASH: .text -20 B (-0.0%, 98,256 B / 262,144 B, total: 37% used)

simulator

  • Code: .rela.dyn +120 B, .rodata +32 B, .text +137 B (+0.0%, 2,672,978 B)
  • Data: .data +128 B (+0.0%, 1,609,604 B)

stm32f407-rt-spark

  • CODE: .text +32 B (+0.0%, 85,796 B / 1,048,576 B, total: 8% used)

stm32l475-atk-pandora-llvm

  • ROM: .text +32 B (+0.0%, 83,460 B / 524,288 B, total: 16% used)

wch-ch32v208w-r0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Harden serial ioctl handling at RT-Smart user/kernel boundaries and validate UART configuration. / 加固 RT-Smart 串口 ioctl 的用户态/内核态边界并校验 UART 配置。

Changes:

  • Marshal known v1/v2 ioctl payloads. / 封送已知的 v1/v2 ioctl 参数。
  • Validate baud rates and defer configuration commits. / 校验波特率并延后配置提交。
  • Reject user-space callback registration. / 拒绝用户态回调注册。

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
components/drivers/serial/dev_serial.c Adds v1 ioctl marshalling and configuration validation.
components/drivers/serial/dev_serial_v2.c Adds v2 ioctl marshalling and safer configuration updates.
bsp/nxp/imx/imx6ull-smart/drivers/drv_uart.c Rejects unsupported baud rates before register writes.
Suppressed comments (2)

components/drivers/serial/dev_serial_v2.c:229

  • 🟠 [Bug/错误]: TCSETS* commands never reach the v2 termios setter / TCSETS* 命令不会进入 v2 termios 设置逻辑

English: rt_serial_control() handles only TCSETA, TCSETAW, and TCSETAF; these TCSETS* commands fall through to the BSP control callback and return -RT_EINVAL on imx6ull-smart. Consequently Musl's tcsetattr() cannot configure this serial device. Add these aliases to the v2 termios setter cases.
中文:rt_serial_control() 仅处理 TCSETATCSETAWTCSETAF;这些 TCSETS* 命令会落入 BSP control 回调,并在 imx6ull-smart 上返回 -RT_EINVAL。因此 Musl 的 tcsetattr() 无法配置该串口设备。请将这些别名加入 v2 termios 设置分支。

        case TCSETS:
        case TCSETSW:
        case TCSETSF:
            arg_size = sizeof(karg.termios);
            kptr = &karg.termios;

components/drivers/serial/dev_serial.c:187

  • 🔴 [Security/安全]: TCGETA still copies uninitialized kernel data / TCGETA 仍会复制未初始化的内核数据

English: Although this boundary now uses a kernel buffer, the downstream v1 TCGETA handler declares an uninitialized struct termios tmp and _termios_to_termio() copies its unset c_line and c_cc fields into this output buffer. The subsequent copyout still leaks kernel stack bytes. Initialize that temporary structure before populating and converting it.
中文:虽然此边界现在使用内核缓冲区,但后续 v1 TCGETA 处理器声明了未初始化的 struct termios tmp,随后 _termios_to_termio() 将其中未设置的 c_linec_cc 字段复制到该输出缓冲区。最终 copyout 仍会泄露内核栈字节。请在填充和转换前初始化该临时结构。

        case TCGETA:
            arg_size = sizeof(karg.termio);
            kptr = &karg.termio;
            copy_out = RT_TRUE;

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

#endif
rt_uint16_t oflag;
rt_size_t unread_bytes;
} karg;
rt_size_t unread_bytes;
rt_ssize_t unread_count;
rt_int32_t timeout;
} karg;
Comment on lines +178 to +181
case FIONREAD:
arg_size = sizeof(karg.unread_bytes);
kptr = &karg.unread_bytes;
copy_out = RT_TRUE;
Comment on lines +205 to +208
case FIONREAD:
arg_size = sizeof(karg.unread_bytes);
kptr = &karg.unread_bytes;
copy_out = RT_TRUE;
Comment on lines +216 to +220
case TCGETA:
case TCGETS:
arg_size = sizeof(karg.termios);
kptr = &karg.termios;
copy_out = RT_TRUE;
Comment on lines +194 to +199
case TCSETA:
case TCSETAW:
case TCSETAF:
arg_size = sizeof(karg.termio);
kptr = &karg.termio;
copy_in = RT_TRUE;
Comment on lines +222 to +224
case TCSETA:
case TCSETAW:
case TCSETAF:
@BernardXiong

Copy link
Copy Markdown
Member Author

Addressed the review findings in commit f64916761d:

  • Zero-initialize v1/v2 ioctl staging unions before dispatch to prevent partial output leaks.
  • Keep FIONREAD at its declared int ABI size and convert the serial unread count before copyout.
  • Add v2 TCGETS and TCSETS* handling so termios requests do not fall through to the BSP callback.
  • Initialize the v1 TCGETA conversion temporary.
  • Propagate termios hardware configuration errors and update serial->config only after successful configuration in both serial implementations.

The original PR validation limitation remains unchanged: the full imx6ull-smart build cannot enter compilation because the existing configuration selects Newlib while components/lwp/lwp.h has no compatible Newlib status definition.

@BernardXiong BernardXiong added the 🎯 Focus Should focus on this issue/discussion/pr label Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BSP: NXP Code related with NXP BSP component: drivers/serial component: drivers Component 🎯 Focus Should focus on this issue/discussion/pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants