linux-kernel - Re: [PATCH] can: at91_can: fix passive-state AERR flooding

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]

Date:   Thu, 7 Oct 2021 15:28:23 +0200
From:   Nicolas Ferre <nicolas.ferre@...rochip.com>
To:     Brandon Maier <brandon.maier@...kwellcollins.com>
CC:     Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "Ludovic Desroches" <ludovic.desroches@...rochip.com>,
        "open list:CAN NETWORK DRIVERS" <linux-can@...r.kernel.org>,
        "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
        "moderated list:ARM/Microchip (AT91) SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] can: at91_can: fix passive-state AERR flooding

On 05/10/2021 at 20:30, Brandon Maier wrote:
> When the at91_can is a single node on the bus and a user attempts to
> transmit, the can state machine will report ack errors and increment the
> transmit error count until it reaches the passive-state. Per the
> specification, it will then transmit with a passive error, but will stop
> incrementing the transmit error count. This results in the host machine
> being flooded with the AERR interrupt forever, or until another node
> rejoins the bus.
> 
> To prevent the AERR flooding, disable the AERR interrupt when we are in
> passive mode.
> 
> Signed-off-by: Brandon Maier <brandon.maier@...kwellcollins.com>

Even if I'm not familiar with the matter, the explanation above makes sense:
Acked-by: Nicolas Ferre <nicolas.ferre@...rochip.com>

Thanks Brandon, best regards,
   Nicolas

> ---
>   drivers/net/can/at91_can.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index b06af90a9964..2a8831127bd0 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -804,8 +804,13 @@ static int at91_poll(struct napi_struct *napi, int quota)
>                  work_done += at91_poll_err(dev, quota - work_done, reg_sr);
> 
>          if (work_done < quota) {
> -               /* enable IRQs for frame errors and all mailboxes >= rx_next */
> +               /* enable IRQs for frame errors and all mailboxes >= rx_next,
> +                * disable the ack error in passive mode to avoid flooding
> +                * ourselves with interrupts
> +                */
>                  u32 reg_ier = AT91_IRQ_ERR_FRAME;
> +               if (priv->can.state == CAN_STATE_ERROR_PASSIVE)
> +                       reg_ier &= ~AT91_IRQ_AERR;
> 
>                  reg_ier |= get_irq_mb_rx(priv) & ~AT91_MB_MASK(priv->rx_next);
> 
> --
> 2.30.2
> 


-- 
Nicolas Ferre