netdev - Re: [PATCH] ethtool: Zero memory allocated for statistics

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>] [day] [month] [year] [list]

Message-ID: <CAH0z3hPgebpUF_PCf0LCRDdJpH_JH3ASRhONjr2j4NYJDXCzuA@mail.gmail.com>
Date:   Sat, 15 Oct 2016 17:13:09 +0200
From:   Vlad Tsyrklevich <vlad@...rklevich.net>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] ethtool: Zero memory allocated for statistics

I agree that we should propagate those errors and I'll prepare a new
change to do so for phy_driver.get_stats(), ethtool_ops.self_test(),
and ethtool_ops.get_ethtool_stats(). However, I still think this
change should be adopted. 3/5 of the cases here are reachable without
any special capabilities and programming defensively at the ethtool
interface can eliminate an entire class of potential driver bugs
instead of fixing them one by one. For example, get_eeprom()
propagates errors but with a brief grep I found that
qlcnic_get_eeprom() will return 0 incorrectly even though it read
nothing for some NICs. Deeper bugs are undoubtedly laying around.

On Sat, Oct 15, 2016 at 5:11 PM, Vlad Tsyrklevich <vlad@...rklevich.net> wrote:
> I agree that we should propagate those errors and I'll prepare a new change
> to do so for phy_driver.get_stats(), ethtool_ops.self_test(), and
> ethtool_ops.get_ethtool_stats(). However, I still think this change should
> be adopted. 3/5 of the cases here are reachable without any special
> capabilities and programming defensively at the ethtool interface can
> eliminate an entire class of potential driver bugs instead of fixing them
> one by one. For example, get_eeprom() propagates errors but with a brief
> grep I found that qlcnic_get_eeprom() will return 0 incorrectly even though
> it read nothing for some NICs. Deeper bugs are undoubtedly laying around.
>
> On Sat, Oct 15, 2016 at 3:21 AM David Miller <davem@...emloft.net> wrote:
>>
>> From: Vlad Tsyrklevich <vlad@...rklevich.net>
>> Date: Fri, 14 Oct 2016 11:59:18 +0200
>>
>> > enic_get_ethtool_stats()
>>
>> Looknig merely at this shows the real problem.
>>
>> We don't propagate and handle errors for this method.
>>
>> And that's what we should fix, making the get_ethtool_stats() method
>> return an integer error.
>>
>> Then ethtool_get_stats() should return any non-zero value provided by
>> ops->get_ethtool_stats() and not attempt to copy any bytes of 'data'
>> to userspace in that case.