linux-kernel - Re: [PATCH] rt2x00: improve calling conventions for register accessors

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]

Message-ID: <0ce21c48-1d2c-3035-3c6e-52c7debf86d5@gmail.com>
Date:   Tue, 16 May 2017 09:44:19 -0400
From:   Jes Sorensen <jes.sorensen@...il.com>
To:     Stanislaw Gruszka <sgruszka@...hat.com>
Cc:     David Miller <davem@...emloft.net>, arnd@...db.de,
        helmut.schaa@...glemail.com, kvalo@...eaurora.org,
        daniel@...rotopia.org, dev@...sin.me, johannes.berg@...el.com,
        pozega.tomislav@...il.com, vasilugin@...dex.ru, roman@...em.lv,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rt2x00: improve calling conventions for register
 accessors

On 05/16/2017 07:55 AM, Stanislaw Gruszka wrote:
> On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:
>> Passing return values by reference is and always has been a really
>> poor way to achieve what these functions are doing.
>>
>> And frankly, whilst the tool could see what's going on here better, we
>> should be making code easier rather than more difficult to audit.
>>
>> I am therefore very much in favor of Arnd's change.
>>
>> This isn't even a situation where there are multiple return values,
>> such as needing to signal an error and return an unsigned value at the
>> same time.
>>
>> These functions return _one_ value, and therefore they should be
>> returned as a true return value.
> 
> In rt2x00 driver we use poor convention in other kind of registers
> accessors like bbp, mac, eeprom. I dislike to changing only rfcsr
> accessors and leaving others in the old way. And changing all accessors
> would be massive and error prone change, which I'm not prefer either.

That's why you do it in multiple smaller patches rather than one ugly 
giant patch.

The rt2x00 current register accessor functions makes the Realtek vendor 
driver equivalent ones look pretty, which is saying something.

Jes