From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "kbuild@lists.01.org" <kbuild@lists.01.org>,
"lkp@intel.com" <lkp@intel.com>,
"kbuild-all@lists.01.org" <kbuild-all@lists.01.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: net/dsa/tag_sja1105.c:432 sja1105_rcv() error: uninitialized symbol 'vid'.
Date: Wed, 27 Apr 2022 13:51:28 +0000 [thread overview]
Message-ID: <20220427135128.la6ifcrs2nmnx4ro@skbuf> (raw)
In-Reply-To: <202204270649.Eun9P40C-lkp@intel.com>
On Wed, Apr 27, 2022 at 10:13:30AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: cf424ef014ac30b0da27125dd1fbdf10b0d3a520
> commit: 04a1758348a87eb73b8a4554d0c227831e2bb33e net: dsa: tag_sja1105: fix control packets on SJA1110 being received on an imprecise port
> config: openrisc-randconfig-m031-20220425 (https://download.01.org/0day-ci/archive/20220427/202204270649.Eun9P40C-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 11.2.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> net/dsa/tag_sja1105.c:432 sja1105_rcv() error: uninitialized symbol 'vid'.
>
> Old smatch warnings:
> net/dsa/tag_sja1105.c:564 sja1110_rcv() error: uninitialized symbol 'vid'.
>
> vim +/vid +432 net/dsa/tag_sja1105.c
>
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 393 static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 394 struct net_device *netdev,
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 395 struct packet_type *pt)
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 396 {
> 884be12f85666c6 Vladimir Oltean 2021-07-26 397 int source_port = -1, switch_id = -1;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 398 struct sja1105_meta meta = {0};
> e80f40cbe4dd513 Vladimir Oltean 2020-03-24 399 struct ethhdr *hdr;
> 42824463d38d273 Vladimir Oltean 2019-06-08 400 bool is_link_local;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 401 bool is_meta;
> 884be12f85666c6 Vladimir Oltean 2021-07-26 402 u16 vid;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 403
> e80f40cbe4dd513 Vladimir Oltean 2020-03-24 404 hdr = eth_hdr(skb);
> 42824463d38d273 Vladimir Oltean 2019-06-08 405 is_link_local = sja1105_is_link_local(skb);
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 406 is_meta = sja1105_is_meta_frame(skb);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 407
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 408 skb->offload_fwd_mark = 1;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 409
> 233697b3b3f60b1 Vladimir Oltean 2021-06-11 410 if (sja1105_skb_has_tag_8021q(skb)) {
> 42824463d38d273 Vladimir Oltean 2019-06-08 411 /* Normal traffic path. */
> 04a1758348a87eb Vladimir Oltean 2021-07-29 412 sja1105_vlan_rcv(skb, &source_port, &switch_id, &vid);
>
> There is a return where *vid is not set
And also one where source_port and switch_id are left with their default
values of -1.
> 42824463d38d273 Vladimir Oltean 2019-06-08 413 } else if (is_link_local) {
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 414 /* Management traffic path. Switch embeds the switch ID and
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 415 * port ID into bytes of the destination MAC, courtesy of
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 416 * the incl_srcpt options.
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 417 */
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 418 source_port = hdr->h_dest[3];
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 419 switch_id = hdr->h_dest[4];
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 420 /* Clear the DMAC bytes that were mangled by the switch */
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 421 hdr->h_dest[3] = 0;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 422 hdr->h_dest[4] = 0;
>
> Not set here
This branch updates source_port and switch_id with values from an
unsigned char, so they definitely won't be -1 anymore after this step.
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 423 } else if (is_meta) {
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 424 sja1105_meta_unpack(skb, &meta);
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 425 source_port = meta.source_port;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 426 switch_id = meta.switch_id;
>
> Or here
Same here, source_port and switch_id are updated with bound values taken
from an u64. Definitely not -1.
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 427 } else {
> 42824463d38d273 Vladimir Oltean 2019-06-08 428 return NULL;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 429 }
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 430
> 04a1758348a87eb Vladimir Oltean 2021-07-29 431 if (source_port == -1 || switch_id == -1)
> 884be12f85666c6 Vladimir Oltean 2021-07-26 @432 skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
> ^^^
This branch is executed only if source_port == -1 || switch_id == -1.
Those were left as -1 only by the same branch that did populate "vid" to
a valid value.
> So the static checker complains
>
> GCC would also complain if we enabled -Wmaybe-uninitialized
>
> net/dsa/tag_sja1105.c: In function ‘sja1105_rcv’:
> net/dsa/tag_sja1105.c:567:28: warning: ‘vid’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 567 | skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 884be12f85666c6 Vladimir Oltean 2021-07-26 433 else
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 434 skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 435 if (!skb->dev) {
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 436 netdev_warn(netdev, "Couldn't decode source port\n");
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 437 return NULL;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 438 }
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 439
> f3097be21bf17ae Vladimir Oltean 2019-06-08 440 return sja1105_rcv_meta_state_machine(skb, &meta, is_link_local,
> f3097be21bf17ae Vladimir Oltean 2019-06-08 441 is_meta);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 442 }
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>
WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: kbuild-all@lists.01.org
Subject: Re: net/dsa/tag_sja1105.c:432 sja1105_rcv() error: uninitialized symbol 'vid'.
Date: Wed, 27 Apr 2022 13:51:28 +0000 [thread overview]
Message-ID: <20220427135128.la6ifcrs2nmnx4ro@skbuf> (raw)
In-Reply-To: <202204270649.Eun9P40C-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 6209 bytes --]
On Wed, Apr 27, 2022 at 10:13:30AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: cf424ef014ac30b0da27125dd1fbdf10b0d3a520
> commit: 04a1758348a87eb73b8a4554d0c227831e2bb33e net: dsa: tag_sja1105: fix control packets on SJA1110 being received on an imprecise port
> config: openrisc-randconfig-m031-20220425 (https://download.01.org/0day-ci/archive/20220427/202204270649.Eun9P40C-lkp(a)intel.com/config)
> compiler: or1k-linux-gcc (GCC) 11.2.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> net/dsa/tag_sja1105.c:432 sja1105_rcv() error: uninitialized symbol 'vid'.
>
> Old smatch warnings:
> net/dsa/tag_sja1105.c:564 sja1110_rcv() error: uninitialized symbol 'vid'.
>
> vim +/vid +432 net/dsa/tag_sja1105.c
>
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 393 static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 394 struct net_device *netdev,
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 395 struct packet_type *pt)
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 396 {
> 884be12f85666c6 Vladimir Oltean 2021-07-26 397 int source_port = -1, switch_id = -1;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 398 struct sja1105_meta meta = {0};
> e80f40cbe4dd513 Vladimir Oltean 2020-03-24 399 struct ethhdr *hdr;
> 42824463d38d273 Vladimir Oltean 2019-06-08 400 bool is_link_local;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 401 bool is_meta;
> 884be12f85666c6 Vladimir Oltean 2021-07-26 402 u16 vid;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 403
> e80f40cbe4dd513 Vladimir Oltean 2020-03-24 404 hdr = eth_hdr(skb);
> 42824463d38d273 Vladimir Oltean 2019-06-08 405 is_link_local = sja1105_is_link_local(skb);
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 406 is_meta = sja1105_is_meta_frame(skb);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 407
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 408 skb->offload_fwd_mark = 1;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 409
> 233697b3b3f60b1 Vladimir Oltean 2021-06-11 410 if (sja1105_skb_has_tag_8021q(skb)) {
> 42824463d38d273 Vladimir Oltean 2019-06-08 411 /* Normal traffic path. */
> 04a1758348a87eb Vladimir Oltean 2021-07-29 412 sja1105_vlan_rcv(skb, &source_port, &switch_id, &vid);
>
> There is a return where *vid is not set
And also one where source_port and switch_id are left with their default
values of -1.
> 42824463d38d273 Vladimir Oltean 2019-06-08 413 } else if (is_link_local) {
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 414 /* Management traffic path. Switch embeds the switch ID and
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 415 * port ID into bytes of the destination MAC, courtesy of
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 416 * the incl_srcpt options.
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 417 */
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 418 source_port = hdr->h_dest[3];
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 419 switch_id = hdr->h_dest[4];
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 420 /* Clear the DMAC bytes that were mangled by the switch */
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 421 hdr->h_dest[3] = 0;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 422 hdr->h_dest[4] = 0;
>
> Not set here
This branch updates source_port and switch_id with values from an
unsigned char, so they definitely won't be -1 anymore after this step.
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 423 } else if (is_meta) {
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 424 sja1105_meta_unpack(skb, &meta);
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 425 source_port = meta.source_port;
> e53e18a6fe4d3ae Vladimir Oltean 2019-06-08 426 switch_id = meta.switch_id;
>
> Or here
Same here, source_port and switch_id are updated with bound values taken
from an u64. Definitely not -1.
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 427 } else {
> 42824463d38d273 Vladimir Oltean 2019-06-08 428 return NULL;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 429 }
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 430
> 04a1758348a87eb Vladimir Oltean 2021-07-29 431 if (source_port == -1 || switch_id == -1)
> 884be12f85666c6 Vladimir Oltean 2021-07-26 @432 skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
> ^^^
This branch is executed only if source_port == -1 || switch_id == -1.
Those were left as -1 only by the same branch that did populate "vid" to
a valid value.
> So the static checker complains
>
> GCC would also complain if we enabled -Wmaybe-uninitialized
>
> net/dsa/tag_sja1105.c: In function ‘sja1105_rcv’:
> net/dsa/tag_sja1105.c:567:28: warning: ‘vid’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 567 | skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 884be12f85666c6 Vladimir Oltean 2021-07-26 433 else
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 434 skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 435 if (!skb->dev) {
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 436 netdev_warn(netdev, "Couldn't decode source port\n");
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 437 return NULL;
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 438 }
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 439
> f3097be21bf17ae Vladimir Oltean 2019-06-08 440 return sja1105_rcv_meta_state_machine(skb, &meta, is_link_local,
> f3097be21bf17ae Vladimir Oltean 2019-06-08 441 is_meta);
> 227d07a07ef1262 Vladimir Oltean 2019-05-05 442 }
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>
next prev parent reply other threads:[~2022-04-27 13:51 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-26 22:09 net/dsa/tag_sja1105.c:432 sja1105_rcv() error: uninitialized symbol 'vid' kernel test robot
2022-04-27 7:13 ` Dan Carpenter
2022-04-27 7:13 ` Dan Carpenter
2022-04-27 13:51 ` Vladimir Oltean [this message]
2022-04-27 13:51 ` Vladimir Oltean
2022-04-27 14:28 ` Dan Carpenter
2022-04-27 14:28 ` Dan Carpenter
2022-04-27 14:28 ` Dan Carpenter
2022-04-27 15:17 ` Vladimir Oltean
2022-04-27 15:17 ` Vladimir Oltean
2022-04-27 18:53 ` Dan Carpenter
2022-04-27 18:53 ` Dan Carpenter
2022-04-27 18:53 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2022-06-27 3:04 kernel test robot
2021-12-11 7:06 kernel test robot
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=20220427135128.la6ifcrs2nmnx4ro@skbuf \
--to=vladimir.oltean@nxp.com \
--cc=dan.carpenter@oracle.com \
--cc=kbuild-all@lists.01.org \
--cc=kbuild@lists.01.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.