diff mbox series

net/vxlan: fix potential null pointer deference

Message ID 1559291681-6002-1-git-send-email-92siuyang@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series net/vxlan: fix potential null pointer deference | expand

Commit Message

Young Xiao May 31, 2019, 8:34 a.m. UTC
There is a possible null pointer deference bug in vxlan_fdb_info(),
which is similar to the bug which was fixed in commit 6adc5fd6a142
("net/neighbour: fix crash at dumping device-agnostic proxy entries").

Signed-off-by: Young Xiao <92siuyang@gmail.com>
---
 drivers/net/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Abeni May 31, 2019, 8:50 a.m. UTC | #1
On Fri, 2019-05-31 at 16:34 +0800, Young Xiao wrote:
> There is a possible null pointer deference bug in vxlan_fdb_info(),
> which is similar to the bug which was fixed in commit 6adc5fd6a142
> ("net/neighbour: fix crash at dumping device-agnostic proxy entries").
> 
> Signed-off-by: Young Xiao <92siuyang@gmail.com>
> ---
>  drivers/net/vxlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 5994d54..1ba5977 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -274,7 +274,7 @@ static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
>  	} else
>  		ndm->ndm_family	= AF_BRIDGE;
>  	ndm->ndm_state = fdb->state;
> -	ndm->ndm_ifindex = vxlan->dev->ifindex;
> +	ndm->ndm_ifindex = vxlan->dev ? vxlan->dev->ifindex : 0;
>  	ndm->ndm_flags = fdb->flags;
>  	if (rdst->offloaded)
>  		ndm->ndm_flags |= NTF_OFFLOADED;

vxlan->dev points to the struct net_device for this vxlan device. It
can't be NULL. 

I suggest to look for working reproducer for this kind of issue before
adding additional, unneeded, checks. We want to avoid as many unneeded
conditionals as possible.

Thanks,

Paolo
Eric Dumazet May 31, 2019, 3 p.m. UTC | #2
On 5/31/19 1:34 AM, Young Xiao wrote:
> There is a possible null pointer deference bug in vxlan_fdb_info(),
> which is similar to the bug which was fixed in commit 6adc5fd6a142
> ("net/neighbour: fix crash at dumping device-agnostic proxy entries").
> 
> Signed-off-by: Young Xiao <92siuyang@gmail.com>
> ---
>  drivers/net/vxlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 5994d54..1ba5977 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -274,7 +274,7 @@ static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
>  	} else
>  		ndm->ndm_family	= AF_BRIDGE;
>  	ndm->ndm_state = fdb->state;
> -	ndm->ndm_ifindex = vxlan->dev->ifindex;
> +	ndm->ndm_ifindex = vxlan->dev ? vxlan->dev->ifindex : 0;
>  	ndm->ndm_flags = fdb->flags;
>  	if (rdst->offloaded)
>  		ndm->ndm_flags |= NTF_OFFLOADED;
> 

Please provide a stack trace, and/or a Fixes: tag, and a detailed
analysis.
diff mbox series

Patch

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 5994d54..1ba5977 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -274,7 +274,7 @@  static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
 	} else
 		ndm->ndm_family	= AF_BRIDGE;
 	ndm->ndm_state = fdb->state;
-	ndm->ndm_ifindex = vxlan->dev->ifindex;
+	ndm->ndm_ifindex = vxlan->dev ? vxlan->dev->ifindex : 0;
 	ndm->ndm_flags = fdb->flags;
 	if (rdst->offloaded)
 		ndm->ndm_flags |= NTF_OFFLOADED;