[PATCH] can: m_can: m_can_receive_skb(): add missing error handling to can_rx_offload_queue_sorted() call

[PATCH] can: m_can: m_can_receive_skb(): add missing error handling to can_rx_offload_queue_sorted() call

[Marc Kleine-Budde]

In commit 1be37d3b0414 ("can: m_can: fix periph RX path: use
rx-offload to ensure skbs are sent from softirq context") the RX path
for peripherals (i.e. SPI based m_can controllers) was converted to
the rx-offload infrastructure. However, the error handling for
can_rx_offload_queue_sorted() was forgotten.
can_rx_offload_queue_sorted() will return with an error if the
internal queue is full.

This patch adds the missing error handling, by increasing the
rx_fifo_errors.

Fixes: 1be37d3b0414 ("can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context")
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1503583 ("Error handling issues")
Cc: Torin Cooper-Bennun <torin@maxiluxsystems.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 890ed826a355..9df1ba1f2d04 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -466,10 +466,17 @@ static void m_can_receive_skb(struct m_can_classdev *cdev,
 			      struct sk_buff *skb,
 			      u32 timestamp)
 {
-	if (cdev->is_peripheral)
-		can_rx_offload_queue_sorted(&cdev->offload, skb, timestamp);
-	else
+	if (cdev->is_peripheral) {
+		struct net_device_stats *stats = &cdev->net->stats;
+		int err;
+		
+		err = can_rx_offload_queue_sorted(&cdev->offload, skb,
+						  timestamp);
+		if (err)
+			stats->rx_fifo_errors++;
+	} else {
 		netif_receive_skb(skb);
+	}
 }
 
 static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
-- 
2.30.2

Re: [PATCH] can: m_can: m_can_receive_skb(): add missing error handling to can_rx_offload_queue_sorted() call

[Kees Cook]

On Thu, Apr 01, 2021 at 10:45:15AM +0200, Marc Kleine-Budde wrote:
> In commit 1be37d3b0414 ("can: m_can: fix periph RX path: use
> rx-offload to ensure skbs are sent from softirq context") the RX path
> for peripherals (i.e. SPI based m_can controllers) was converted to
> the rx-offload infrastructure. However, the error handling for
> can_rx_offload_queue_sorted() was forgotten.
> can_rx_offload_queue_sorted() will return with an error if the
> internal queue is full.
> 
> This patch adds the missing error handling, by increasing the
> rx_fifo_errors.
> 
> Fixes: 1be37d3b0414 ("can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context")
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1503583 ("Error handling issues")
> Cc: Torin Cooper-Bennun <torin@maxiluxsystems.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

Thanks for the fix!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  drivers/net/can/m_can/m_can.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 890ed826a355..9df1ba1f2d04 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -466,10 +466,17 @@ static void m_can_receive_skb(struct m_can_classdev *cdev,
>  			      struct sk_buff *skb,
>  			      u32 timestamp)
>  {
> -	if (cdev->is_peripheral)
> -		can_rx_offload_queue_sorted(&cdev->offload, skb, timestamp);
> -	else
> +	if (cdev->is_peripheral) {
> +		struct net_device_stats *stats = &cdev->net->stats;
> +		int err;
> +		
> +		err = can_rx_offload_queue_sorted(&cdev->offload, skb,
> +						  timestamp);
> +		if (err)
> +			stats->rx_fifo_errors++;
> +	} else {
>  		netif_receive_skb(skb);
> +	}
>  }
>  
>  static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> -- 
> 2.30.2
> 
> 

-- 
Kees Cook