netdev - RE: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 30 Nov 2022 15:57:35 +0000
From:   <Jerry.Ray@...rochip.com>
To:     <olteanv@...il.com>
CC:     <andrew@...n.ch>, <f.fainelli@...il.com>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats

>>  static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
>>                                     uint64_t *data)
>>  {
>>       struct lan9303 *chip = ds->priv;
>> -     unsigned int u;
>> +     unsigned int i, u;
>>
>>       for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
>>               u32 reg;
>>               int ret;
>>
>> -             ret = lan9303_read_switch_port(
>> -                     chip, port, lan9303_mib[u].offset, &reg);
>> -
>> +             /* Read Port-based MIB stats. */
>> +             ret = lan9303_read_switch_port(chip, port,
>> +                                            lan9303_mib[u].offset,
>> +                                            &reg);
>
>Speaking of unrelated changes...
>

Cleaning up a warning generated from checkpatch

>>               if (ret)
>>                       dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
>>                                port, lan9303_mib[u].offset);
>
>...If lan9303_read_switch_port() fails, should we copy kernel stack
>uninitialized memory (reg) to user space?
>

Good catch.  I'll zero out the returned stat.

>>               data[u] = reg;
>>       }
>> +     for (i = 0; i < ARRAY_SIZE(lan9303_switch_mib); i++) {
>> +             u32 reg;
>> +             int ret;
>> +
>> +             /* Read Switch stats indexed by port. */
>> +             ret = lan9303_read_switch_reg(chip,
>> +                                           (lan9303_switch_mib[i].offset +
>> +                                            port), &reg);
>> +             if (ret)
>> +                     dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
>> +                              port, lan9303_switch_mib[i].offset);
>
>Because the same, existing pattern is also used for new code here.
>

Ditto

>> +             data[i + u] = reg;
>> +     }
>>  }
>>
>>  static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
>> @@ -1017,7 +1061,7 @@ static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
>>       if (sset != ETH_SS_STATS)
>>               return 0;
>>
>> -     return ARRAY_SIZE(lan9303_mib);
>> +     return ARRAY_SIZE(lan9303_mib) + ARRAY_SIZE(lan9303_switch_mib);
>>  }
>>
>>  static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
>> --
>> 2.17.1
>>
>