Skip to content

fs/romfs: reject negative resulting position in romfs_seek()#19420

Open
94xhn wants to merge 1 commit into
apache:masterfrom
94xhn:fix-romfs-seek-negative-offset
Open

fs/romfs: reject negative resulting position in romfs_seek()#19420
94xhn wants to merge 1 commit into
apache:masterfrom
94xhn:fix-romfs-seek-negative-offset

Conversation

@94xhn

@94xhn 94xhn commented Jul 13, 2026

Copy link
Copy Markdown

Summary

romfs_seek() clamps the computed position to the file size when it exceeds rf->rf_size, but never checks for a negative result. lseek(fd, offset, SEEK_SET/SEEK_CUR/SEEK_END) calls that produce a negative position (e.g. a negative SEEK_SET offset, or a SEEK_CUR/SEEK_END offset more negative than the current position/file size) write that negative value straight into filep->f_pos.

The subsequent romfs_read() computes rf->rf_startoffset + filep->f_pos into a uint32_t (fs/romfs/fs_romfs.c), so a negative f_pos wraps around to a huge unsigned offset, and romfs_hwread()'s XIP path memcpy()s from rm_xipbase plus that offset — an out-of-bounds read far past the mapped flash region.

Fix

Add the same if (position < 0) return -EINVAL guard already used by fs/fat/fs_fat32.c's seek function, before the existing end-of-file clamp.

Testing

Wrote a standalone host-side C reproduction transcribing romfs_seek(), the downstream romfs_read() address computation, and romfs_hwread()'s XIP memcpy() verbatim from the current source, with a SIGSEGV handler around a deliberately small (1 MiB) rm_xipbase mapping:

  • Before fix: lseek(fd, -600, SEEK_SET) returns -1 but with the wrong errno (leaks the raw negative value instead of EINVAL), leaves f_pos corrupted at -600, and the downstream address computation wraps to 0xFFFFFDC8 (~4 GB out of bounds) — romfs_hwread_XIP() reading from that offset triggers a real SIGSEGV against the mapped buffer.
  • After fix: the same call correctly returns -1 with errno == EINVAL, and f_pos is left unchanged.
  • Regression checks: normal SEEK_SET/SEEK_CUR/SEEK_END seeks, EOF clamping (seeking past end-of-file), and the exact-zero boundary case (a SEEK_CUR that lands exactly at 0, which must not be rejected) all produce identical results before and after the fix.

Assisted-by: Claude:claude-sonnet-5

romfs_seek() clamps the computed position to the file size when it
exceeds rf_size, but never checks for a negative result. lseek(fd,
offset, SEEK_SET/SEEK_CUR/SEEK_END) with an offset that produces a
negative position (e.g. a negative SEEK_SET offset, or a SEEK_CUR/
SEEK_END offset more negative than the current position/file size)
is written straight into filep->f_pos.

The subsequent romfs_read() computes
`rf->rf_startoffset + filep->f_pos` into a uint32_t, so a negative
f_pos wraps around to a huge unsigned offset, and romfs_hwread()'s
XIP path memcpy()s from rm_xipbase plus that offset -- an
out-of-bounds read far past the mapped flash region.

Add the same "if (position < 0) return -EINVAL" guard already used
by fs/fat/fs_fat32.c's seek function, before the existing
end-of-file clamp.

Assisted-by: Claude:claude-sonnet-5
@github-actions github-actions Bot added Area: File System File System issues Size: XS The size of the change in this PR is very small labels Jul 13, 2026
@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

qemu-armv8a

  • Code: .text.romfs_seek +20 B (+0.0%, 315,378 B)

s698pm-dkit

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@94xhn please fix:

625562d962 fs/romfs: reject negative resulting position in romfs_seek()
../nuttx/tools/checkpatch.sh -c -u -m -g  cd6ed0dbf92713110ef217b9bf3c07846cf11bca..HEAD
❌ Missing Signed-off-by

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: File System File System issues Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants