Re: [PATCH net 1/1] ipv4: Fix fib_rebalance() divide-by-zero race

[zihan xi]

On Wed, Aug 19, 2026 at 11:31 PM Ido Schimmel <idosch@xxxxxxxxxx> wrote:
>
> On Mon, Aug 17, 2026 at 01:12:10PM +0000, Zihan Xi wrote:
> > fib_rebalance() reads ignore_routes_with_linkdown while summing active
> > nexthop weights and then reads it again while calculating each upper
> > bound. Sysctl writes are not serialized by RTNL. If all nexthops are
> > link-down, a concurrent 1-to-0 change can leave total at zero and make
> > the second pass divide by zero.
> >
> > Route insertion and link-state updates call fib_rebalance() under RTNL,
> > and netlink devconf changes are already protected by RTNL. Make the
> > sysctl handler take the per-net RTNL lock before changing
> > ignore_routes_with_linkdown. This gives both passes a stable policy view
> > without adding work to route lookup or rebalance paths.
> >
> > Fixes: 0e884c78ee19 ("ipv4: L3 hash-based multipath")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Reported-by: Vega <vega@xxxxxxxxxx>
> > Assisted-by: Codex:gpt-5.4
> > Signed-off-by: Zihan Xi <zihanx@xxxxxxxxxx>
>
> I think that this is a better direction:
>
> https://lore.kernel.org/netdev/20260817013237.2797-1-blbllhy@xxxxxxxxx/
>
> Instead of taking RTNL, make it consistent with IPv6 and mark all the
> nexthops as dead (upper_bound = -1) when total==0, as the first pass
> indicated. Something like [1].
>
> It's true that it can lead to incorrect upper bounds if a concurrent
> 1-to-0 change didn't result in total==0: during the second pass we will
> account for nexthops that we skipped in the first pass.
>
> It's not very interesting. This sysctl was never meant to be configured
> on the fly and the kernel doesn't rebalance nexthop groups when it is
> toggled.
>
> Let's wait for the IPv6 patch to be applied so that you could reference
> it in the commit message.
>
> [1]
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 0483519b7fb0..7a362f2e2c2b 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -874,7 +874,7 @@ static void fib_rebalance(struct fib_info *fi)
>         change_nexthops(fi) {
>                 int upper_bound;
>
> -               if (nexthop_nh->fib_nh_flags & RTNH_F_DEAD) {
> +               if (!total || nexthop_nh->fib_nh_flags & RTNH_F_DEAD) {
>                         upper_bound = -1;
>                 } else if (ip_ignore_linkdown(nexthop_nh->fib_nh_dev) &&
>                            nexthop_nh->fib_nh_flags & RTNH_F_LINKDOWN) {

Hi Ido,

Thanks for the guidance.

Agreed. Taking RTNL is unnecessary here. I will revise the IPv4 fix to
skip the upper-bound calculation and set upper_bound to -1 when the
first pass reports a zero total, consistent with the IPv6 fix.

I will wait for the IPv6 patch to be applied and reference it in the v2
commit message.

Thanks,
Zihan