linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: bareudp: avoid uninitialized variable warning
@ 2020-05-05 17:22 Arnd Bergmann
  2020-05-06  2:59 ` Nathan Chancellor
  2020-05-08  0:28 ` David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2020-05-05 17:22 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Willem de Bruijn
  Cc: Arnd Bergmann, Martin Varghese, Taehee Yoo, netdev, linux-kernel,
	clang-built-linux

clang points out that building without IPv6 would lead to returning
an uninitialized variable if a packet with family!=AF_INET is
passed into bareudp_udp_encap_recv():

drivers/net/bareudp.c:139:6: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
        if (family == AF_INET)
            ^~~~~~~~~~~~~~~~~
drivers/net/bareudp.c:146:15: note: uninitialized use occurs here
        if (unlikely(err)) {
                     ^~~
include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
 # define unlikely(x)    __builtin_expect(!!(x), 0)
                                            ^
drivers/net/bareudp.c:139:2: note: remove the 'if' if its condition is always true
        if (family == AF_INET)
        ^~~~~~~~~~~~~~~~~~~~~~

This cannot happen in practice, so change the condition in a way that
gcc sees the IPv4 case as unconditionally true here.
For consistency, change all the similar constructs in this file the
same way, using "if(IS_ENABLED())" instead of #if IS_ENABLED()".

Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/bareudp.c    | 18 ++++--------------
 include/net/udp_tunnel.h |  2 --
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index cc0703c3d57f..efd1a1d1f35e 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -136,25 +136,21 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	oiph = skb_network_header(skb);
 	skb_reset_network_header(skb);
 
-	if (family == AF_INET)
+	if (!IS_ENABLED(CONFIG_IPV6) || family == AF_INET)
 		err = IP_ECN_decapsulate(oiph, skb);
-#if IS_ENABLED(CONFIG_IPV6)
 	else
 		err = IP6_ECN_decapsulate(oiph, skb);
-#endif
 
 	if (unlikely(err)) {
 		if (log_ecn_error) {
-			if  (family == AF_INET)
+			if  (!IS_ENABLED(CONFIG_IPV6) || family == AF_INET)
 				net_info_ratelimited("non-ECT from %pI4 "
 						     "with TOS=%#x\n",
 						     &((struct iphdr *)oiph)->saddr,
 						     ((struct iphdr *)oiph)->tos);
-#if IS_ENABLED(CONFIG_IPV6)
 			else
 				net_info_ratelimited("non-ECT from %pI6\n",
 						     &((struct ipv6hdr *)oiph)->saddr);
-#endif
 		}
 		if (err > 1) {
 			++bareudp->dev->stats.rx_frame_errors;
@@ -350,7 +346,6 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	return err;
 }
 
-#if IS_ENABLED(CONFIG_IPV6)
 static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 			     struct bareudp_dev *bareudp,
 			     const struct ip_tunnel_info *info)
@@ -411,7 +406,6 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 	dst_release(dst);
 	return err;
 }
-#endif
 
 static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev)
 {
@@ -435,11 +429,9 @@ static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	rcu_read_lock();
-#if IS_ENABLED(CONFIG_IPV6)
-	if (info->mode & IP_TUNNEL_INFO_IPV6)
+	if (IS_ENABLED(CONFIG_IPV6) && info->mode & IP_TUNNEL_INFO_IPV6)
 		err = bareudp6_xmit_skb(skb, dev, bareudp, info);
 	else
-#endif
 		err = bareudp_xmit_skb(skb, dev, bareudp, info);
 
 	rcu_read_unlock();
@@ -467,7 +459,7 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
 
 	use_cache = ip_tunnel_dst_cache_usable(skb, info);
 
-	if (ip_tunnel_info_af(info) == AF_INET) {
+	if (!IS_ENABLED(CONFIG_IPV6) || ip_tunnel_info_af(info) == AF_INET) {
 		struct rtable *rt;
 		__be32 saddr;
 
@@ -478,7 +470,6 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
 
 		ip_rt_put(rt);
 		info->key.u.ipv4.src = saddr;
-#if IS_ENABLED(CONFIG_IPV6)
 	} else if (ip_tunnel_info_af(info) == AF_INET6) {
 		struct dst_entry *dst;
 		struct in6_addr saddr;
@@ -492,7 +483,6 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
 
 		dst_release(dst);
 		info->key.u.ipv6.src = saddr;
-#endif
 	} else {
 		return -EINVAL;
 	}
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 4b1f95e08307..e7312ceb2794 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -143,14 +143,12 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
 			 __be16 df, __be16 src_port, __be16 dst_port,
 			 bool xnet, bool nocheck);
 
-#if IS_ENABLED(CONFIG_IPV6)
 int udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 			 struct sk_buff *skb,
 			 struct net_device *dev, struct in6_addr *saddr,
 			 struct in6_addr *daddr,
 			 __u8 prio, __u8 ttl, __be32 label,
 			 __be16 src_port, __be16 dst_port, bool nocheck);
-#endif
 
 void udp_tunnel_sock_release(struct socket *sock);
 
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] net: bareudp: avoid uninitialized variable warning
  2020-05-05 17:22 [PATCH] net: bareudp: avoid uninitialized variable warning Arnd Bergmann
@ 2020-05-06  2:59 ` Nathan Chancellor
  2020-05-08  0:28 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2020-05-06  2:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Jakub Kicinski, Willem de Bruijn,
	Martin Varghese, Taehee Yoo, netdev, linux-kernel,
	clang-built-linux

On Tue, May 05, 2020 at 07:22:14PM +0200, Arnd Bergmann wrote:
> clang points out that building without IPv6 would lead to returning
> an uninitialized variable if a packet with family!=AF_INET is
> passed into bareudp_udp_encap_recv():
> 
> drivers/net/bareudp.c:139:6: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>         if (family == AF_INET)
>             ^~~~~~~~~~~~~~~~~
> drivers/net/bareudp.c:146:15: note: uninitialized use occurs here
>         if (unlikely(err)) {
>                      ^~~
> include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
>  # define unlikely(x)    __builtin_expect(!!(x), 0)
>                                             ^
> drivers/net/bareudp.c:139:2: note: remove the 'if' if its condition is always true
>         if (family == AF_INET)
>         ^~~~~~~~~~~~~~~~~~~~~~
> 
> This cannot happen in practice, so change the condition in a way that
> gcc sees the IPv4 case as unconditionally true here.
> For consistency, change all the similar constructs in this file the
> same way, using "if(IS_ENABLED())" instead of #if IS_ENABLED()".
> 
> Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  drivers/net/bareudp.c    | 18 ++++--------------
>  include/net/udp_tunnel.h |  2 --
>  2 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> index cc0703c3d57f..efd1a1d1f35e 100644
> --- a/drivers/net/bareudp.c
> +++ b/drivers/net/bareudp.c
> @@ -136,25 +136,21 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
>  	oiph = skb_network_header(skb);
>  	skb_reset_network_header(skb);
>  
> -	if (family == AF_INET)
> +	if (!IS_ENABLED(CONFIG_IPV6) || family == AF_INET)
>  		err = IP_ECN_decapsulate(oiph, skb);
> -#if IS_ENABLED(CONFIG_IPV6)
>  	else
>  		err = IP6_ECN_decapsulate(oiph, skb);
> -#endif
>  
>  	if (unlikely(err)) {
>  		if (log_ecn_error) {
> -			if  (family == AF_INET)
> +			if  (!IS_ENABLED(CONFIG_IPV6) || family == AF_INET)
>  				net_info_ratelimited("non-ECT from %pI4 "
>  						     "with TOS=%#x\n",
>  						     &((struct iphdr *)oiph)->saddr,
>  						     ((struct iphdr *)oiph)->tos);
> -#if IS_ENABLED(CONFIG_IPV6)
>  			else
>  				net_info_ratelimited("non-ECT from %pI6\n",
>  						     &((struct ipv6hdr *)oiph)->saddr);
> -#endif
>  		}
>  		if (err > 1) {
>  			++bareudp->dev->stats.rx_frame_errors;
> @@ -350,7 +346,6 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	return err;
>  }
>  
> -#if IS_ENABLED(CONFIG_IPV6)
>  static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  			     struct bareudp_dev *bareudp,
>  			     const struct ip_tunnel_info *info)
> @@ -411,7 +406,6 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	dst_release(dst);
>  	return err;
>  }
> -#endif
>  
>  static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
> @@ -435,11 +429,9 @@ static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev)
>  	}
>  
>  	rcu_read_lock();
> -#if IS_ENABLED(CONFIG_IPV6)
> -	if (info->mode & IP_TUNNEL_INFO_IPV6)
> +	if (IS_ENABLED(CONFIG_IPV6) && info->mode & IP_TUNNEL_INFO_IPV6)
>  		err = bareudp6_xmit_skb(skb, dev, bareudp, info);
>  	else
> -#endif
>  		err = bareudp_xmit_skb(skb, dev, bareudp, info);
>  
>  	rcu_read_unlock();
> @@ -467,7 +459,7 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
>  
>  	use_cache = ip_tunnel_dst_cache_usable(skb, info);
>  
> -	if (ip_tunnel_info_af(info) == AF_INET) {
> +	if (!IS_ENABLED(CONFIG_IPV6) || ip_tunnel_info_af(info) == AF_INET) {
>  		struct rtable *rt;
>  		__be32 saddr;
>  
> @@ -478,7 +470,6 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
>  
>  		ip_rt_put(rt);
>  		info->key.u.ipv4.src = saddr;
> -#if IS_ENABLED(CONFIG_IPV6)
>  	} else if (ip_tunnel_info_af(info) == AF_INET6) {
>  		struct dst_entry *dst;
>  		struct in6_addr saddr;
> @@ -492,7 +483,6 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
>  
>  		dst_release(dst);
>  		info->key.u.ipv6.src = saddr;
> -#endif
>  	} else {
>  		return -EINVAL;
>  	}
> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
> index 4b1f95e08307..e7312ceb2794 100644
> --- a/include/net/udp_tunnel.h
> +++ b/include/net/udp_tunnel.h
> @@ -143,14 +143,12 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
>  			 __be16 df, __be16 src_port, __be16 dst_port,
>  			 bool xnet, bool nocheck);
>  
> -#if IS_ENABLED(CONFIG_IPV6)
>  int udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
>  			 struct sk_buff *skb,
>  			 struct net_device *dev, struct in6_addr *saddr,
>  			 struct in6_addr *daddr,
>  			 __u8 prio, __u8 ttl, __be32 label,
>  			 __be16 src_port, __be16 dst_port, bool nocheck);
> -#endif
>  
>  void udp_tunnel_sock_release(struct socket *sock);
>  
> -- 
> 2.26.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] net: bareudp: avoid uninitialized variable warning
  2020-05-05 17:22 [PATCH] net: bareudp: avoid uninitialized variable warning Arnd Bergmann
  2020-05-06  2:59 ` Nathan Chancellor
@ 2020-05-08  0:28 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2020-05-08  0:28 UTC (permalink / raw)
  To: arnd
  Cc: kuba, willemb, martin.varghese, ap420073, netdev, linux-kernel,
	clang-built-linux

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue,  5 May 2020 19:22:14 +0200

> clang points out that building without IPv6 would lead to returning
> an uninitialized variable if a packet with family!=AF_INET is
> passed into bareudp_udp_encap_recv():
> 
> drivers/net/bareudp.c:139:6: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>         if (family == AF_INET)
>             ^~~~~~~~~~~~~~~~~
> drivers/net/bareudp.c:146:15: note: uninitialized use occurs here
>         if (unlikely(err)) {
>                      ^~~
> include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
>  # define unlikely(x)    __builtin_expect(!!(x), 0)
>                                             ^
> drivers/net/bareudp.c:139:2: note: remove the 'if' if its condition is always true
>         if (family == AF_INET)
>         ^~~~~~~~~~~~~~~~~~~~~~
> 
> This cannot happen in practice, so change the condition in a way that
> gcc sees the IPv4 case as unconditionally true here.
> For consistency, change all the similar constructs in this file the
> same way, using "if(IS_ENABLED())" instead of #if IS_ENABLED()".
> 
> Fixes: 571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-05-08  0:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 17:22 [PATCH] net: bareudp: avoid uninitialized variable warning Arnd Bergmann
2020-05-06  2:59 ` Nathan Chancellor
2020-05-08  0:28 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).