linux-kernel - Re: [PATCH 12/18] staging: rtl8723bs: remove VHT dead code

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: <d80cf1a5-d70c-97c2-a6a2-af2f4209715d@redhat.com>
Date:   Tue, 22 Jun 2021 13:15:00 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Fabio Aiuto <fabioaiuto83@...il.com>
Cc:     gregkh@...uxfoundation.org, Larry.Finger@...inger.net,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 12/18] staging: rtl8723bs: remove VHT dead code

Hi Fabio,

On 6/22/21 11:57 AM, Fabio Aiuto wrote:
> On Tue, Jun 22, 2021 at 11:19:36AM +0200, Hans de Goede wrote:
> 
> Hi Hans,
> 
>> Hi Fabio,
>>
>>> Moreover I have been quite conservative, for I left untouched HT indexes above
>>> 7 which rtl8723bs doesn't support.
>>>
>>> So IMO I think this patch is fine as is...
>>>> Perhaps this entire block can never be executed ?
>>>
>>> the block is executed but there's no register write happening. Just
>>> updating of values which will never be fetched.
>>
>> Ack, my bad I was under the impression that phy_SetTxPowerByRateBase()
>> would actually do a register write, but I checked and it just updates
>> some unused table values, so dropping this code is fine and you can
>> keep this patch for v2 of the patch set.
>>
>> Regards,
>>
>> Hans
>>
> 
> thank you, what do you think about what I replied about patch 1,

I somehow did not receive your reply, so I've just read it on the archives.

> shall
> I remove the '> 14 if block' or leave it as is?

I think it would be best to keep the '> 14 if block' for now and
remove all of them in a later patch-series (I assume there will
be more of them).

> Do you think is necessary
> to keep the conditions inside the block and pack them?

You could also remove the condition and just set
the band to WIRELESS_INVALID unconditionally as you
suggest, that is fine.

But if you keep the condition, like you did in v1 of the
patch, then you must pack the 2 masks together.

Regards,

Hans