Bug 283818 - too-large rtmsg.rtm_family for RTM_GETADDR can cause wild pointer ref
Summary: too-large rtmsg.rtm_family for RTM_GETADDR can cause wild pointer ref
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Gleb Smirnoff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-01-03 14:01 UTC by Robert Morris
Modified: 2025-01-29 23:50 UTC (History)
6 users (show)

See Also:


Attachments
RTM_GETROUTE with illegal rtm_family (575 bytes, text/plain)
2025-01-03 14:01 UTC, Robert Morris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2025-01-03 14:01:03 UTC
Created attachment 256375 [details]
RTM_GETROUTE with illegal rtm_family

The attached program supplies an illegally large rtm_family to netlink
RTM_GETROUTE, which causes rt_tables_get_rnh_ptr() to fetch a pointer
from beyond the end of V_rt_tables, which can lead to a crash in
rib_walk_ext_internal(). With INVARIANTS, this fails:


        KASSERT(family < (AF_MAX + 1),
            ("%s: fam out of bounds (%d < %d)", __func__, family, AF_MAX + 1));

# uname -a
FreeBSD  15.0-CURRENT FreeBSD 15.0-CURRENT #339 main-n250995-3750873316a1-dirty: Fri Jan  3 08:33:55 EST 2025     rtm@xxx:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv
# cc netlink6c.c
# ./a.out
panic: Fatal page fault at 0xffffffc0002f6e64: 0x424242424242432a
cpuid = 0
time = 1735908209
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_trace_self_wrapper() at db_trace_self_wrapper+0x36
kdb_backtrace() at kdb_backtrace+0x2c
vpanic() at vpanic+0x16e
panic() at panic+0x26
page_fault_handler() at page_fault_handler+0x214
do_trap_supervisor() at do_trap_supervisor+0x6c
cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x74
--- exception 13, tval = 0x424242424242432a
_rm_rlock_debug() at _rm_rlock_debug+0x24
rib_walk() at rib_walk+0x7e
dump_rtable_one() at dump_rtable_one+0x68
rtnl_handle_getroute() at rtnl_handle_getroute+0x12c
rtnl_handle_message() at rtnl_handle_message+0x12a
nl_taskqueue_handler() at nl_taskqueue_handler+0x49a
taskqueue_run_locked() at taskqueue_run_locked+0x158
taskqueue_thread_loop() at taskqueue_thread_loop+0xd4
fork_exit() at fork_exit+0x68
fork_trampoline() at fork_trampoline+0xa
KDB: enter: panic
Comment 1 Zhenlei Huang freebsd_committer freebsd_triage 2025-01-08 02:22:49 UTC
I think we should do sanity check within `rtnl_handle_getroute()`. Meanwhile we can give netlink client a meaningful error message.
Comment 2 Jose Luis Duran freebsd_committer freebsd_triage 2025-01-16 16:52:20 UTC
(In reply to Zhenlei Huang from comment #1)
How about renaming rt_tables_get_rnh_safe() -> rt_tables_get_rnh()?
The _safe variant was added in fd1aa866eb22 ("routing: add rt_tables_get_rnh_safe() that doesn't panic when af/fib is incorrect.")
Comment 3 commit-hook freebsd_committer freebsd_triage 2025-01-29 23:50:22 UTC
A commit in branch main references this bug:

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

commit cdacb12065e4d85416655743da5bc6b17a9d9119
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2025-01-29 23:40:56 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2025-01-29 23:48:55 +0000

    netlink/route: validate family attribute

    PR:                     283818

 sys/netlink/route/rt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)