netdev - Re: [PATCH net-next 1/3] r8169: replace BUG_ON with WARN in _rtl_eri

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0UcbDT_ccC0M=hC121YZ3pVC1ht2uv9-vPDjMFtM5mKDWw@mail.gmail.com>
Date:   Wed, 6 Jan 2021 14:18:54 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] r8169: replace BUG_ON with WARN in _rtl_eri_write

On Wed, Jan 6, 2021 at 5:32 AM Heiner Kallweit <hkallweit1@...il.com> wrote:
>
> Use WARN here to avoid stopping the system. In addition print the addr
> and mask values that triggered the warning.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 024042f37..9af048ad0 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -763,7 +763,7 @@ static void _rtl_eri_write(struct rtl8169_private *tp, int addr, u32 mask,
>  {
>         u32 cmd = ERIAR_WRITE_CMD | type | mask | addr;
>
> -       BUG_ON((addr & 3) || (mask == 0));
> +       WARN(addr & 3 || !mask, "addr: 0x%x, mask: 0x%08x\n", addr, mask);
>         RTL_W32(tp, ERIDR, val);
>         r8168fp_adjust_ocp_cmd(tp, &cmd, type);
>         RTL_W32(tp, ERIAR, cmd);

Would it make more sense to perhaps just catch the case via an if
statement, display the warning, and then return instead of proceeding
with the write with the bad values?

I'm just wondering if this could make things worse by putting the
device in a bad state in some way resulting in either a timeout
waiting for a response or a hang.