[PATCH net v3] net: core: failover: fix NULL pointer dereference in failover_slave_register()

[PATCH net v3] net: core: failover: fix NULL pointer dereference in failover_slave_register()

[Zeeshan Ahmad]

Smatch warns that 'fops' is dereferenced without a NULL check.
While other callbacks in this function properly check 'fops', the
rx_handler registration does not.

Consolidate the NULL check for 'fops' at the beginning of the function,
before it is first used in slave_pre_register(). This ensures 'fops' is
valid for the entire function scope and allows the removal of redundant
NULL checks later in the function, as suggested by Dan Carpenter.

Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")
Signed-off-by: Zeeshan Ahmad <zeeshanahmad022019@gmail.com>
---
v3:
 - Move the fops NULL check to the top of the function before any 
   dereferences occur and remove subsequent redundant NULL checks, 
   as suggested by Dan Carpenter.
v2:
 - Target 'net' tree as this is a bug fix.
 - Change logic from an early return (v1) to WARN_ON_ONCE() and abort
   registration, as discussed with Simon Horman.

 net/core/failover.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/failover.c b/net/core/failover.c
index 2a140b3ea669..47e4a91dcaa6 100644
--- a/net/core/failover.c
+++ b/net/core/failover.c
@@ -59,7 +59,10 @@ static int failover_slave_register(struct net_device *slave_dev)
 	if (!failover_dev)
 		goto done;
 
-	if (fops && fops->slave_pre_register &&
+	if (WARN_ON_ONCE(!fops))
+		goto done;
+
+	if (fops->slave_pre_register &&
 	    fops->slave_pre_register(slave_dev, failover_dev))
 		goto done;
 
@@ -82,7 +85,7 @@ static int failover_slave_register(struct net_device *slave_dev)
 
 	slave_dev->priv_flags |= (IFF_FAILOVER_SLAVE | IFF_NO_ADDRCONF);
 
-	if (fops && fops->slave_register &&
+	if (fops->slave_register &&
 	    !fops->slave_register(slave_dev, failover_dev))
 		return NOTIFY_OK;
 
-- 
2.43.0

Re: [PATCH net v3] net: core: failover: fix NULL pointer dereference in failover_slave_register()

[Dan Carpenter]

On Thu, Feb 26, 2026 at 01:56:01PM +0500, Zeeshan Ahmad wrote:
> Smatch warns that 'fops' is dereferenced without a NULL check.
> While other callbacks in this function properly check 'fops', the
> rx_handler registration does not.
> 
> Consolidate the NULL check for 'fops' at the beginning of the function,
> before it is first used in slave_pre_register(). This ensures 'fops' is
> valid for the entire function scope and allows the removal of redundant
> NULL checks later in the function, as suggested by Dan Carpenter.
> 
> Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")
> Signed-off-by: Zeeshan Ahmad <zeeshanahmad022019@gmail.com>
> ---
> v3:
>  - Move the fops NULL check to the top of the function before any 
>    dereferences occur and remove subsequent redundant NULL checks, 
>    as suggested by Dan Carpenter.

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH net v3] net: core: failover: fix NULL pointer dereference in failover_slave_register()

[Jakub Kicinski]

On Thu, 26 Feb 2026 13:56:01 +0500 Zeeshan Ahmad wrote:
> Smatch warns that 'fops' is dereferenced without a NULL check.
> While other callbacks in this function properly check 'fops', the
> rx_handler registration does not.
> 
> Consolidate the NULL check for 'fops' at the beginning of the function,
> before it is first used in slave_pre_register(). This ensures 'fops' is
> valid for the entire function scope and allows the removal of redundant
> NULL checks later in the function, as suggested by Dan Carpenter.
> 
> Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")

Not really a fix, there's only one user - net_failover and it always
passes the ops and has all callbacks.

> diff --git a/net/core/failover.c b/net/core/failover.c
> index 2a140b3ea669..47e4a91dcaa6 100644
> --- a/net/core/failover.c
> +++ b/net/core/failover.c
> @@ -59,7 +59,10 @@ static int failover_slave_register(struct net_device *slave_dev)
>  	if (!failover_dev)
>  		goto done;
>  
> -	if (fops && fops->slave_pre_register &&
> +	if (WARN_ON_ONCE(!fops))
> +		goto done;

Better still please add this check in failover_register()
so that we don't allow any device to be registered without ops.
Then you can delete all the fops checks throughout this file,
not just in this one function.
-- 
pw-bot: cr