[PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[Weiming Shi]

bareudp_fill_metadata_dst() passes bareudp->sock to
udp_tunnel6_dst_lookup() in the IPv6 path without a NULL check.
The socket is only created in bareudp_open() and NULLed in
bareudp_stop(), so calling this function while the device is down
triggers a NULL dereference via sock->sk.

 BUG: kernel NULL pointer dereference, address: 0000000000000018
 RIP: 0010:udp_tunnel6_dst_lookup (net/ipv6/ip6_udp_tunnel.c:160)
 Call Trace:
  <TASK>
  bareudp_fill_metadata_dst (drivers/net/bareudp.c:532)
  do_execute_actions (net/openvswitch/actions.c:901)
  ovs_execute_actions (net/openvswitch/actions.c:1589)
  ovs_packet_cmd_execute (net/openvswitch/datapath.c:700)
  genl_family_rcv_msg_doit (net/netlink/genetlink.c:1114)
  genl_rcv_msg (net/netlink/genetlink.c:1209)
  netlink_rcv_skb (net/netlink/af_netlink.c:2550)
  </TASK>

Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
in the same driver.

Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/net/bareudp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index 0df3208783ad..da5866ba0699 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
 		struct in6_addr saddr;
 		struct socket *sock = rcu_dereference(bareudp->sock);
 
+		if (!sock)
+			return -ESHUTDOWN;
+
 		dst = udp_tunnel6_dst_lookup(skb, dev, bareudp->net, sock,
 					     0, &saddr, &info->key,
 					     sport, bareudp->port, info->key.tos,
-- 
2.43.0

Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[Kuniyuki Iwashima]

From: Weiming Shi <bestswngs@gmail.com>
Date: Sun, 26 Apr 2026 09:53:51 -0700
> bareudp_fill_metadata_dst() passes bareudp->sock to
> udp_tunnel6_dst_lookup() in the IPv6 path without a NULL check.
> The socket is only created in bareudp_open() and NULLed in
> bareudp_stop(), so calling this function while the device is down
> triggers a NULL dereference via sock->sk.
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000018
>  RIP: 0010:udp_tunnel6_dst_lookup (net/ipv6/ip6_udp_tunnel.c:160)
>  Call Trace:
>   <TASK>
>   bareudp_fill_metadata_dst (drivers/net/bareudp.c:532)
>   do_execute_actions (net/openvswitch/actions.c:901)
>   ovs_execute_actions (net/openvswitch/actions.c:1589)
>   ovs_packet_cmd_execute (net/openvswitch/datapath.c:700)
>   genl_family_rcv_msg_doit (net/netlink/genetlink.c:1114)
>   genl_rcv_msg (net/netlink/genetlink.c:1209)
>   netlink_rcv_skb (net/netlink/af_netlink.c:2550)
>   </TASK>
> 
> Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> in the same driver.
> 
> Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

This commit fixes a NULL pointer dereference in bareudp_fill_metadata_dst()
by adding a check for the socket in the IPv6 path. If the device is down
and the socket is NULL, it now returns -ESHUTDOWN.

> Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> in the same driver.

Does the IPv4 path also need to be updated to remain consistent?

While the IPv4 path lookup doesn't use the socket pointer directly, if the
socket is NULL (meaning the device is down), shouldn't the IPv4 path also
return -ESHUTDOWN instead of successfully performing a route lookup?

> diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> index 0df3208783ad..da5866ba0699 100644
> --- a/drivers/net/bareudp.c
> +++ b/drivers/net/bareudp.c
> @@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,

Is it possible for an IPv6 tunnel key to be incorrectly processed as IPv4
earlier in this function?

Looking at the preceding condition in this function:

    if (!ipv6_mod_enabled() || ip_tunnel_info_af(info) == AF_INET) {
        struct rtable *rt;
        ...
        info->key.u.ipv4.src = saddr;
    }

If the IPv6 module is disabled at runtime, but an IPv6 tunnel key is provided
(e.g., via OVS or TC), the code falls back to the IPv4 branch.

Since key->u.ipv4.dst aliases exactly with bytes 4-7 of key->u.ipv6.src due
to the union layout in struct ip_tunnel_key, could this result in an invalid
IPv4 route lookup using portions of the IPv6 source address?

Would it be safer to explicitly reject IPv6 tunnel keys when the IPv6 module
is disabled?

>  		struct in6_addr saddr;
>  		struct socket *sock = rcu_dereference(bareudp->sock);
>  
> +		if (!sock)
> +			return -ESHUTDOWN;
> +
>  		dst = udp_tunnel6_dst_lookup(skb, dev, bareudp->net, sock,
>  					     0, &saddr, &info->key,
>  					     sport, bareudp->port, info->key.tos,

Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[Kuniyuki Iwashima]

From: Simon Horman <horms@kernel.org>
Date: Tue, 28 Apr 2026 17:04:46 +0100
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
> 
> This commit fixes a NULL pointer dereference in bareudp_fill_metadata_dst()
> by adding a check for the socket in the IPv6 path. If the device is down
> and the socket is NULL, it now returns -ESHUTDOWN.
> 
> > Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> > in the same driver.
> 
> Does the IPv4 path also need to be updated to remain consistent?

No, because udp_tunnel_dst_lookup() does not take struct
sock while udp_tunnel6_dst_lookup() does.

And all the other fast paths (bareudp_xmit_skb() and
bareudp6_xmit_skb()) have NULL checks properly.


> 
> While the IPv4 path lookup doesn't use the socket pointer directly, if the
> socket is NULL (meaning the device is down), shouldn't the IPv4 path also
> return -ESHUTDOWN instead of successfully performing a route lookup?
> 
> > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> > index 0df3208783ad..da5866ba0699 100644
> > --- a/drivers/net/bareudp.c
> > +++ b/drivers/net/bareudp.c
> > @@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
> 
> Is it possible for an IPv6 tunnel key to be incorrectly processed as IPv4
> earlier in this function?

I think this is orthogonal to this patch and can be a follow-up.

I have a series to remove all synchronize_rcu() from all UDP tunnel
users and udp_tunnel_sock_release(), but it conflict with this patch,
so I hope this land net-next this Thursday :)


> 
> Looking at the preceding condition in this function:
> 
>     if (!ipv6_mod_enabled() || ip_tunnel_info_af(info) == AF_INET) {
>         struct rtable *rt;
>         ...
>         info->key.u.ipv4.src = saddr;
>     }
> 
> If the IPv6 module is disabled at runtime, but an IPv6 tunnel key is provided
> (e.g., via OVS or TC), the code falls back to the IPv4 branch.
> 
> Since key->u.ipv4.dst aliases exactly with bytes 4-7 of key->u.ipv6.src due
> to the union layout in struct ip_tunnel_key, could this result in an invalid
> IPv4 route lookup using portions of the IPv6 source address?
> 
> Would it be safer to explicitly reject IPv6 tunnel keys when the IPv6 module
> is disabled?

Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[Eric Dumazet]

On Tue, Apr 28, 2026 at 5:14 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> From: Simon Horman <horms@kernel.org>
> Date: Tue, 28 Apr 2026 17:04:46 +0100
> > From: 'Simon Horman' <horms@kernel.org>
> >
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
> >
> > This commit fixes a NULL pointer dereference in bareudp_fill_metadata_dst()
> > by adding a check for the socket in the IPv6 path. If the device is down
> > and the socket is NULL, it now returns -ESHUTDOWN.
> >
> > > Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> > > in the same driver.
> >
> > Does the IPv4 path also need to be updated to remain consistent?
>
> No, because udp_tunnel_dst_lookup() does not take struct
> sock while udp_tunnel6_dst_lookup() does.
>
> And all the other fast paths (bareudp_xmit_skb() and
> bareudp6_xmit_skb()) have NULL checks properly.
>
>
> >
> > While the IPv4 path lookup doesn't use the socket pointer directly, if the
> > socket is NULL (meaning the device is down), shouldn't the IPv4 path also
> > return -ESHUTDOWN instead of successfully performing a route lookup?
> >
> > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> > > index 0df3208783ad..da5866ba0699 100644
> > > --- a/drivers/net/bareudp.c
> > > +++ b/drivers/net/bareudp.c
> > > @@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
> >
> > Is it possible for an IPv6 tunnel key to be incorrectly processed as IPv4
> > earlier in this function?
>
> I think this is orthogonal to this patch and can be a follow-up.
>
> I have a series to remove all synchronize_rcu() from all UDP tunnel
> users and udp_tunnel_sock_release(), but it conflict with this patch,
> so I hope this land net-next this Thursday :)

Same here hope for me!

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 26 Apr 2026 09:53:51 -0700 you wrote:
> bareudp_fill_metadata_dst() passes bareudp->sock to
> udp_tunnel6_dst_lookup() in the IPv6 path without a NULL check.
> The socket is only created in bareudp_open() and NULLed in
> bareudp_stop(), so calling this function while the device is down
> triggers a NULL dereference via sock->sk.
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000018
>  RIP: 0010:udp_tunnel6_dst_lookup (net/ipv6/ip6_udp_tunnel.c:160)
>  Call Trace:
>   <TASK>
>   bareudp_fill_metadata_dst (drivers/net/bareudp.c:532)
>   do_execute_actions (net/openvswitch/actions.c:901)
>   ovs_execute_actions (net/openvswitch/actions.c:1589)
>   ovs_packet_cmd_execute (net/openvswitch/datapath.c:700)
>   genl_family_rcv_msg_doit (net/netlink/genetlink.c:1114)
>   genl_rcv_msg (net/netlink/genetlink.c:1209)
>   netlink_rcv_skb (net/netlink/af_netlink.c:2550)
>   </TASK>
> 
> [...]

Here is the summary with links:
  - [net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
    https://git.kernel.org/netdev/net/c/aa6c6d9ee064

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html