netdev - Re: [PATCH net] ionic: remove WARN_ON to prevent panic_on

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: <6bcbb3e5-e370-39eb-d96f-21b7fcfd62de@amd.com>
Date:   Thu, 29 Jun 2023 10:50:01 -0700
From:   Shannon Nelson <shannon.nelson@....com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     "Keller, Jacob E" <jacob.e.keller@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "brett.creeley@....com" <brett.creeley@....com>,
        "drivers@...sando.io" <drivers@...sando.io>,
        "nitya.sunkad@....com" <nitya.sunkad@....com>
Subject: Re: [PATCH net] ionic: remove WARN_ON to prevent panic_on_warn

On 6/28/23 2:06 PM, Jakub Kicinski wrote:
> 
> On Wed, 28 Jun 2023 11:26:18 -0700 Shannon Nelson wrote:
>>> This message could potentially use a bit more explanation since it
>>> doesn't look like you removed all the WARN_ONs in the driver, and
>>> it might help to explain why this particular WARN_ON was
>>> problematic. I don't think that would be worth a re-roll on its own
>>> though.
>>
>> There has been recent mention of not using WARNxxx macros because so
>> many folks have been setting panic_on_warn [1].  This is intended to
>> help mitigate the possibility of unnecessarily killing a machine when
>> we can adjust and continue.
>> [1]:
>> https://lore.kernel.org/netdev/2023060820-atom-doorstep-9442@gregkh/
>>
>> I believe the only other WARNxxx in this driver is a WARN_ON_ONCE in
>> ionic_regs.h which can be addressed in a separate patch.
>>
>> Neither of these are ever expected to be hit, but also neither should
>> ever kill a machine.
> 
> An explanation that this warning may in fact be hit and how in
> the commit message would be good.

Even better might be to dig further into the history of why this is even 
here and see if we can simply remove it altogether.  It is really old 
code from early development and probably should just go away.  We're 
checking and likely will followup with just a straight removal of the check.

sln