From: Yue Haibing yuehaibing@huawei.com
[ Upstream commit 3d95261eeb74958cd496e1875684827dc5d028cc ]
In ipv6_rpl_srh_rcv() we use min(net->ipv6.devconf_all->rpl_seg_enabled, idev->cnf.rpl_seg_enabled) is intended to return 0 when either value is zero, but if one of the values is negative it will in fact return non-zero.
Signed-off-by: Yue Haibing yuehaibing@huawei.com Link: https://patch.msgid.link/20250901123726.1972881-3-yuehaibing@huawei.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - In `ipv6_rpl_srh_rcv()` the decision to process RPL SRH uses `accept_rpl_seg = min(net->ipv6.devconf_all->rpl_seg_enabled, idev->cnf.rpl_seg_enabled);` and then checks `if (!accept_rpl_seg)` to drop packets. A negative value for either sysctl makes `min()` negative (non-zero), which is treated as “true”, unintentionally enabling processing when it should be disabled. See `net/ipv6/exthdrs.c:497` and `net/ipv6/exthdrs.c:499`. - The change bounds `rpl_seg_enabled` to 0..1 via sysctl, preventing negative values and restoring intended boolean semantics.
- Code changes and their effect - Sysctl registration for `rpl_seg_enabled` switches from `proc_dointvec` to `proc_dointvec_minmax` and adds bounds: - `.proc_handler = proc_dointvec_minmax`, `.extra1 = SYSCTL_ZERO`, `.extra2 = SYSCTL_ONE` at `net/ipv6/addrconf.c:7241`, `net/ipv6/addrconf.c:7242`, `net/ipv6/addrconf.c:7243`. - This mirrors existing practice for boolean-like IPv6 sysctls (e.g., `ioam6_enabled` immediately below uses min/max too; `net/ipv6/addrconf.c:7246`). - The sysctl table is cloned for `conf/all`, `conf/default`, and each device. Critically, when cloning the table the kernel only fills handler “extra” fields if both are unset; since this patch sets both `.extra1` and `.extra2`, the bounds are preserved for per-net/per- device sysctls as well: - See the cloning logic guarding extra fields at `net/ipv6/addrconf.c:7315`–`net/ipv6/addrconf.c:7318`.
- Why this is a good stable backport - Bug impact: Admins (CAP_NET_ADMIN) could inadvertently set a negative value (e.g., -1) and expect “disabled”, but the code interprets it as enabled due to non-zero truthiness. This causes unintended acceptance of RPL SRH packets, affecting system behavior and potentially security posture. - Scope: Single-field sysctl bounds change; no functional restructuring or architectural changes. - Risk: Minimal. Values >1 or negative will now be rejected, aligning with boolean expectations. No in-tree code relies on non-boolean semantics; `rpl_seg_enabled` is only consumed as a boolean via `min(...)` and `if (!accept_rpl_seg)` in `net/ipv6/exthdrs.c:497`–`net/ipv6/exthdrs.c:499`. - Consistency: Aligns `rpl_seg_enabled` with other similar sysctls that already use min/max bounds.
- Conclusion - The patch is a small, contained bugfix enforcing correct boolean semantics and preventing misconfiguration from bypassing the intended disable path for RPL SRH processing. It fits stable rules and has very low regression risk.
net/ipv6/addrconf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index f17a5dd4789fb..40e9c336f6c55 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -7238,7 +7238,9 @@ static const struct ctl_table addrconf_sysctl[] = { .data = &ipv6_devconf.rpl_seg_enabled, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, }, { .procname = "ioam6_enabled",