From: Nathan Chancellor <natechancellor@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Willem de Bruijn <willemb@google.com>,
Martin Varghese <martin.varghese@nokia.com>,
Taehee Yoo <ap420073@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com
Subject: Re: [PATCH] net: bareudp: avoid uninitialized variable warning
Date: Tue, 5 May 2020 19:59:55 -0700 [thread overview]
Message-ID: <20200506025955.GA433288@ubuntu-s3-xlarge-x86> (raw)
In-Reply-To: <20200505172232.1034560-1-arnd@arndb.de>
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
>
next prev parent reply other threads:[~2020-05-06 3:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-05 17:22 [PATCH] net: bareudp: avoid uninitialized variable warning Arnd Bergmann
2020-05-06 2:59 ` Nathan Chancellor [this message]
2020-05-08 0:28 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200506025955.GA433288@ubuntu-s3-xlarge-x86 \
--to=natechancellor@gmail.com \
--cc=ap420073@gmail.com \
--cc=arnd@arndb.de \
--cc=clang-built-linux@googlegroups.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.varghese@nokia.com \
--cc=netdev@vger.kernel.org \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox