RTEMS | Draft: libfs/rfs: Reject max-held-bufs=0 instead of dereferencing NULL (!1442)

[Sam Price (@TheSamPrice)]

Sam Price created a merge request: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1442

Project:Branches: TheSamPrice/rtems:rfs/reject-zero-max-held-bufs to rtems/rtos/rtems:main
Author:   Sam Price



Mounting RFS with max-held-bufs=0 dereferences a NULL pointer on the
first buffer release.

rtems_rfs_buffer_handle_release evicts a held buffer when

  (fs->release_count + fs->release_modified_count) >= fs->max_held_buffers

taking it from fs->release when release_count is the larger and from
fs->release_modified otherwise, then doing buffer->user = (void*)0 at
rtems-rfs-buffer.c:274 with no NULL test.  rtems_chain_get_unprotected
returns NULL for an empty chain by design.

For max_held_buffers >= 1 the guard implies at least one count is
non-zero and the branch selects the queue holding it, so the result is
never NULL.  At zero the guard is 0 >= 0, true on the first release with
both queues still empty; 0 > 0 is false so the else branch runs;
fs->release_modified_count-- underflows from 0 to UINT32_MAX; and the
NULL reaches :274.

The value is a mount option and nothing range checked it: the parser used
strtoul straight into the local and rtems_rfs_fs_open assigned it
unchecked.  Zero held buffers is not a meaningful configuration, so
reject it where it is parsed.

GCC's -fanalyzer reports the dereference at rtems-rfs-buffer.c:274.
Testing the chain result there is worth doing regardless, but on its own
it converts the dereference into a silently skipped eviction and leaves
the counter underflow, so the option is validated instead.

The failure was easy to misread.  Before this change the mount does fail,
with EIO, which looks like the option being rejected.  It is not: EIO is
what rtems_rfs_buffer_bdbuf_release returns after rtems_bdbuf_release is
handed the NULL, so it is the defect reported downstream of
buffer->user = (void*)0 having already run on a NULL pointer.  It did not
fault only because address zero is mapped on the test target.

fsrfsmaxheld01 is added with the fix.  It mounts with no options, with
max-held-bufs=5 and with max-held-bufs=1, all of which must succeed, then
with max-held-bufs=0 and requires EINVAL.  The controls matter: at one the
first release sees 0 >= 1 and evicts nothing, which puts the boundary
exactly where the arithmetic says it is.

Measured on riscv/mbv: EIO before, EINVAL after, test passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply at anthropic.com>
Signed-off-by: Samuel Price <thesamprice at gmail.com>

-- 
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1442
You're receiving this email because of your account on gitlab.rtems.org. Unsubscribe from this thread: https://gitlab.rtems.org/-/sent_notifications/5-b4e6j0u1cfuvfaslhxaxg41ni-1d/unsubscribe | Manage all notifications: https://gitlab.rtems.org/-/profile/notifications | Help: https://gitlab.rtems.org/help


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20260821/a0611353/attachment-0001.htm>