Re: [PATCH net-next v1] net: core: fix logical inconsistency in failover_slave_register()
From: Zeeshan Ahmad
Date: Wed Feb 25 2026 - 05:05:58 EST
Hi Simon,
Thank you for the detailed feedback.
On Thu, Feb 19, 2026 at 2:40 PM Simon Horman <horms@xxxxxxxxxx> wrote:
> It's not entirely clear to me what the behaviour should be if fops is
> NULL, or indeed if fops can be NULL.
I've performed a deeper audit of the failover module and found that
failover_register() currently allows a master instance to be registered
with ops = NULL. This appears to be the root of the issue. However, I
checked all current in-tree callers (e.g. net_failover.c) and confirmed
they always pass valid ops. So while it practically doesn't happen
today, the framework technically allows this inconsistent state.
> So I think it would be best to do the same here - that is modify the
> code around line 66 to make it conditional on fops not being NULL.
> Otherwise, if fops is NULL then steps that would have been taken are
> skipped.
Wouldn't skipping the rx_handler registration at line 66 lead to an
inconsistent state? If we skip that hook but continue to link the slave
to the master (line 75) and set the failover flags (line 83), the device
might appear linked but the data path would remain unhooked. This
concern is why I am leaning toward a more definitive "abort" if fops
is missing.
> It is true that in those steps would never be reached and the kernel
> would have panic'ed due to a NULL dereference on line 66. So maybe your
> approach is better, perhaps with the addition of a WARN_ON_ONCE.
I agree that WARN_ON_ONCE(!fops) is the best way to handle this. It
provides a clear signal to developers of a misconfiguration without
allowing the kernel to panic.
Based on this, I will prepare a v2 targeting the 'net' tree. It will use
the WARN_ON_ONCE check to both prevent the panic and abort the
registration (returning NOTIFY_DONE) to prevent an inconsistent failover
state.
Best regards,
Zeeshan