Messages in this thread | | | Date | Thu, 11 May 2017 21:48:14 -0500 | From | "Gustavo A. R. Silva" <> | Subject | Re: [net-dsa-mv88e6xxx] question about potential use of uninitialized variable |
| |
Hi Andrew,
Quoting Andrew Lunn <andrew@lunn.ch>:
> On Thu, May 11, 2017 at 04:35:37PM -0500, Gustavo A. R. Silva wrote: >> >> Hello everybody, >> >> While looking into Coverity ID 1398130 I ran into the following >> piece of code at drivers/net/dsa/mv88e6xxx/chip.c:849: >> >> 849static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip, >> 850 struct mv88e6xxx_hw_stat *s, >> 851 int port, u16 bank1_select, >> 852 u16 histogram) >> 853{ >> 854 u32 low; >> 855 u32 high = 0; >> 856 u16 reg = 0; >> 857 int err; >> 858 u64 value; >> 859 >> 860 switch (s->type) { >> 861 case STATS_TYPE_PORT: >> 862 err = mv88e6xxx_port_read(chip, port, s->reg, ®); >> 863 if (err) >> 864 return UINT64_MAX; >> 865 >> 866 low = reg; >> 867 if (s->sizeof_stat == 4) { >> 868 err = mv88e6xxx_port_read(chip, port, >> s->reg + 1, ®); >> 869 if (err) >> 870 return UINT64_MAX; >> 871 high = reg; >> 872 } >> 873 break; >> 874 case STATS_TYPE_BANK1: >> 875 reg = bank1_select; >> 876 /* fall through */ >> 877 case STATS_TYPE_BANK0: >> 878 reg |= s->reg | histogram; >> 879 mv88e6xxx_g1_stats_read(chip, reg, &low); >> 880 if (s->sizeof_stat == 8) >> 881 mv88e6xxx_g1_stats_read(chip, reg + 1, &high); >> 882 } >> 883 value = (((u64)high) << 16) | low; >> 884 return value; >> 885} >> >> My question here is if there is any chance for the execution path to >> directly jump from line 860 to line 883, hence ending up using the >> uninitialized variable _low_? > > Hi Gustavo > > It would require that s->type not have one of the listed case values. > Currently all members of mv88e6xxx_hw_stats due use expected values. > However, it would not hurt to add a > > default: > return UINT64_MAX; > > Do you want to submit a patch? >
Sure, I'll send it shortly.
Thanks for clarifying! -- Gustavo A. R. Silva
| |