Bug 289121 - A time-of-check to time-of-use race exists in sysctl_requested_fec() of the cxgbe subsystem
Summary: A time-of-check to time-of-use race exists in sysctl_requested_fec() of the c...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 14.3-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Navdeep Parhar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-08-27 07:56 UTC by Qiu-ji Chen
Modified: 2025-11-20 07:28 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Qiu-ji Chen 2025-08-27 07:56:17 UTC
Summary
In sysctl_requested_fec(), the output string is chosen via a ternary that reads lc->requested_fec twice without synchronization:
lc->requested_fec == FEC_AUTO ? -1 :
(lc->requested_fec & (M_FW_PORT_CAP32_FEC | FEC_MODULE))
Because lc->requested_fec may change between the check and the use, the “lc->requested_fec == FEC_AUTO” test can pass/fail on a stale value, and the masked use can observe a concurrent FEC_AUTO, yielding 0 (since FEC_AUTO & (M_FW_PORT_CAP32_FEC | FEC_MODULE) == 0). The result “0” is then handed to sysctl_handle_string() as the old value. However, under the allowed mask no real FEC combination produces 0; real FECs live in bit0..2 (FEC_RS=1<<0, FEC_BASER_RS=1<<1, FEC_NONE=1<<2), while pseudo FECs start at bit5 (FEC_AUTO=1<<5, FEC_MODULE=1<<6). AUTO should be serialized as “-1”, not “0”.

Impact
• Wrong interface semantics: the old value reported to user space can become “0”, which encodes neither AUTO (-1) nor any valid real-FEC bitset, misleading management tools and scripts and causing faulty decisions.

Suggested fix
Snapshot lc->requested_fec once into local variables and compute from that single snapshot, instead of repeatedly reading shared fields.
Comment 1 Navdeep Parhar freebsd_committer freebsd_triage 2025-08-28 06:32:40 UTC
Please try https://reviews.freebsd.org/D52199
Comment 2 Qiu-ji Chen 2025-11-14 07:30:34 UTC
(In reply to Navdeep Parhar from comment #1)
I think this patch solves this problem, thank you.
Comment 3 commit-hook freebsd_committer freebsd_triage 2025-11-14 08:44:17 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b0cfa27c91c400dcf3e8441cc04f9e5300c7c3af

commit b0cfa27c91c400dcf3e8441cc04f9e5300c7c3af
Author:     Navdeep Parhar <np@FreeBSD.org>
AuthorDate: 2025-11-14 07:45:59 +0000
Commit:     Navdeep Parhar <np@FreeBSD.org>
CommitDate: 2025-11-14 07:45:59 +0000

    cxgbe(4): Avoid racy access to requested_fec in sysctl_requested_fec

    PR:             289121
    Reported by:    Qiu-ji Chen
    MFC after:      1 week
    Sponsored by:   Chelsio Communications
    Differential Revision:  https://reviews.freebsd.org/D52199

 sys/dev/cxgbe/t4_main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)