Re: [net-dsa-mv88e6xxx] question about potential use of uninitialized variable

[Gustavo A. R. Silva]

Hi Andrew,

Quoting Andrew Lunn <and...@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, &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, &reg);
 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