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/[email protected]/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 <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
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
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
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
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);
^^^
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
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/[email protected]/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 <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> 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
>
On Wed, Apr 27, 2022 at 01:51:28PM +0000, Vladimir Oltean wrote:
> 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/[email protected]/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 <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> >
> > 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.
>
I should probably have wondered why I hadn't emailed you before... For
some reason I got confused and thought it was an arch/ thing...
My desktop build does cross function analysis so it doesn't print this
false positive.
regards,
dan carpenter
On Wed, Apr 27, 2022 at 05:28:39PM +0300, Dan Carpenter wrote:
> I should probably have wondered why I hadn't emailed you before... For
> some reason I got confused and thought it was an arch/ thing...
>
> My desktop build does cross function analysis so it doesn't print this
> false positive.
>
> regards,
> dan carpenter
So I don't need to do anything? Cool.
On Wed, Apr 27, 2022 at 03:17:55PM +0000, Vladimir Oltean wrote:
> On Wed, Apr 27, 2022 at 05:28:39PM +0300, Dan Carpenter wrote:
> > I should probably have wondered why I hadn't emailed you before... For
> > some reason I got confused and thought it was an arch/ thing...
> >
> > My desktop build does cross function analysis so it doesn't print this
> > false positive.
> >
> > regards,
> > dan carpenter
>
> So I don't need to do anything? Cool.
Yeah. It's fine.
regards,
dan carpenter