Re: [PATCH net-next v1] net: core: fix logical inconsistency in failover_slave_register()

[Simon Horman]

On Wed, Feb 25, 2026 at 03:04:25PM +0500, Zeeshan Ahmad wrote:
> 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.

Thanks. I did a not very deep audit before writing my previous email.
And my conclusion from that was the same as yours.

> 
> > 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.

Agree, that seems to be the best plan.

> 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.

Great, thanks.