netdev - [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-next>] [day] [month] [year] [list]

Message-Id: <20220810144536.389237-1-u.kleine-koenig@pengutronix.de>
Date:   Wed, 10 Aug 2022 16:45:36 +0200
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     linux-can@...r.kernel.org, netdev@...r.kernel.org,
        kernel@...gutronix.de
Subject: [PATCH] can: rx-offload: Break loop on queue full

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.

Best regards
Uwe

 drivers/net/can/dev/rx-offload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c
index a32a01c172d4..d5d33692bb6a 100644
--- a/drivers/net/can/dev/rx-offload.c
+++ b/drivers/net/can/dev/rx-offload.c
@@ -207,7 +207,7 @@ int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload)
 	while (1) {
 		skb = can_rx_offload_offload_one(offload, 0);
 		if (IS_ERR(skb))
-			continue;
+			break;
 		if (!skb)
 			break;

-- 
2.36.1