Messages in this thread | | | From | Colin Ian King <> | Subject | issue with set_vlan_mode in starfire driver | Date | Fri, 9 Jul 2021 14:46:09 +0100 |
| |
Hi,
Static analysis with Coverity on the starfire driver has detected an issue introduced in function set_vlan_mode with the following commit:
commit 5da96be53a16a62488316810d0c7c5d58ce3ee4f Author: Jiri Pirko <jpirko@redhat.com> Date: Wed Jul 20 04:54:31 2011 +0000
starfire: do vlan cleanup
The analysis is as follows:
1743 static u32 set_vlan_mode(struct netdev_private *np) 1744 { 1745 u32 ret = VlanMode; 1746 u16 vid; 1747 void __iomem *filter_addr = np->base + HashTable + 8; 1748 int vlan_count = 0; 1749 1750 for_each_set_bit(vid, np->active_vlans, VLAN_N_VID) { 1751 if (vlan_count == 32) 1752 break; 1753 writew(vid, filter_addr); 1754 filter_addr += 16; 1755 vlan_count++; 1756 }
cond_const: Condition vlan_count == 32, taking true branch. Now the value of vlan_count is equal to 32.
1757 if (vlan_count == 32) { 1758 ret |= PerfectFilterVlan;
const: At condition vlan_count < 32, the value of vlan_count must be equal to 32. dead_error_condition: The condition vlan_count < 32 cannot be true.
1759 while (vlan_count < 32) {
Logically dead code (DEADCODE) dead_error_begin: Execution cannot reach this statement: writew(0, filter_addr);.
1760 writew(0, filter_addr); 1761 filter_addr += 16; 1762 vlan_count++; 1763 } 1764 } 1765 return ret; 1766 }
Looking at commit 5da96be53a16a62488316810d0c7c5d58ce3ee4f it appears that the check if (vlan_count == 32) should be if (vid == VLAN_N_VID) if I understand things correctly.
However, I'm not sure about the setting of ret |= PerfectFilterVlan - should that be set if the vlan_count reaches 32 or if vid reaches VLAN_N_VID. I don't understand the semantics of setting the PerfectFilterVlan bit so I'm a bit stuck at figuring out an appropriate fix.
Thought had better flag this up as an issue since I can't resolve it.
Colin
| |