netdev - Re: [PATCH] net: ethtool: avoid allocation failure for dump

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

Message-ID: <87y3y6lzve.fsf@kamboji.qca.qualcomm.com>
Date:   Fri, 20 Jan 2017 13:44:37 +0200
From:   Kalle Valo <kvalo@...eaurora.org>
To:     David Miller <davem@...emloft.net>
Cc:     linville@...driver.com, darcari@...hat.com, netdev@...r.kernel.org,
        johannes.berg@...el.com, linux-wireless@...r.kernel.org
Subject: Re: [PATCH] net: ethtool: avoid allocation failure for dump_regs

David Miller <davem@...emloft.net> writes:

> From: Kalle Valo <kvalo@...eaurora.org>
> Date: Thu, 19 Jan 2017 20:08:30 +0200
>
>> "John W. Linville" <linville@...driver.com> writes:
>> 
>>> I forgot to Cc Johannes and Kalle...
>> 
>> Also adding linux-wireless.
>> 
>>> On Thu, Jan 19, 2017 at 09:15:09AM -0500, John W. Linville wrote:
>>>
>>>> I'm responsible for this mess. The original idea was for various
>>>> mac80211-based drivers to override the ethtool operation and provide
>>>> their own dump operation, but the mac80211 crowd never embraced
>>>> the idea.
>>>> 
>>>> In the meantime, I added the default implementation which just
>>>> passed-up wdev->wiphy->hw_version as the version info for a 0-length
>>>> register dump. I then implemented a driver-specific regiser dump
>>>> handler for userland ethtool that would interpret the hardware version
>>>> information for the at76c50x-usb driver.
>>>> 
>>>> So the net of it is, if we treat a return of 0 from get_regs_len()
>>>> as "not supported", we break this one driver-specific feature for
>>>> userland ethtool. Realistically, there are probably very few users
>>>> to care. But I can't guarantee that the number is zero.
>> 
>> I know the number is not zero, because I remember using it years back
>> with something else than at76c50x-usb. But is the number more than one,
>> I don't know :)
>
> I'm trying to dig down and figure out why this problem is showing up now.
> ethtool_get_regs() has been using vzalloc() since 2011, and before that it
> used plain vmalloc().
>
> This code has therefore been using v{m,z}alloc() forever.  What changed?
>
> The zero size check has been in the vmalloc implementation since at least
> 2009.
>
> I don't understand why this is all triggering and being noticed now.  The
> whole ieee80211 "return zero length regs and return hw version in get_regs"
> thing should have been failing for at least 7 years now.

Maybe just nobody hasn't used it since? If my memory serves me right
(too often it does not) It's 6-7 years since I used this, and if the
kernel I worked on at the time was a year or two old, I might have used
a version without the zero size check.

But I'm just hand-waving here, I cannot be sure what's the last kernel I
used.

-- 
Kalle Valo