netdev - Re: [PATCH] can: rx-offload: Break loop on queue full

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: <20220811094329.hgkgeydboswlfwvr@pengutronix.de>
Date:   Thu, 11 Aug 2022 11:43:29 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Wolfgang Grandegger <wg@...ndegger.com>, linux-can@...r.kernel.org,
        netdev@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH] can: rx-offload: Break loop on queue full

On 11.08.2022 10:30:39, Marc Kleine-Budde wrote:
> On 10.08.2022 16:45:36, Uwe Kleine-König wrote:
> > The following happend on an i.MX25 using flexcan with many packets on
> > the bus:
> > 
> > The rx-offload queue reached a length more than skb_queue_len_max. So in
> > can_rx_offload_offload_one() the drop variable was set to true which
> > made the call to .mailbox_read() (here: flexcan_mailbox_read()) just
> > return ERR_PTR(-ENOBUFS) (plus some irrelevant hardware interaction) and
> > so can_rx_offload_offload_one() returned ERR_PTR(-ENOBUFS), too.
> > 
> > Now can_rx_offload_irq_offload_fifo() looks as follows:
> > 
> > 	while (1) {
> > 		skb = can_rx_offload_offload_one(offload, 0);
> > 		if (IS_ERR(skb))
> > 			continue;
> > 		...
> > 	}
> > 
> > As the i.MX25 is a single core CPU while the rx-offload processing is
> > active there is no thread to process packets from the offload queue and
> > so it doesn't get shorter.
> > 
> > The result is a tight loop: can_rx_offload_offload_one() does nothing
> > relevant and returns an error code and so
> > can_rx_offload_irq_offload_fifo() calls can_rx_offload_offload_one()
> > again.
> > 
> > To break that loop don't continue calling can_rx_offload_offload_one()
> > after it reported an error.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> > ---
> > Hello,
> > 
> > this patch just implements the obvious change to break the loop. I'm not
> > 100% certain that there is no corner case where the break is wrong. The
> > problem exists at least since v5.6, didn't go back further to check.
> > 
> > This fixes a hard hang on said i.MX25.
> 
> As Uwe suggested in an IRC conversation, the correct fix for the flexcan
> driver is to return NULL if there is no CAN frame pending.
> 
> I'll send a -v2.

https://lore.kernel.org/all/20220811094254.1864367-1-mkl@pengutronix.de

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)