[PATCH 1/2] Bluetooth: 6LoWPAN: Modify the error handling in the loop

[PATCH 1/2] Bluetooth: 6LoWPAN: Modify the error handling in the loop

[Jiasheng Jiang]

Return the error when send_pkt fails in order to avoid the error being
overwritten.
Moreover, remove the redundant 'ret'.

Fixes: 9c238ca8ec79 ("Bluetooth: 6lowpan: Check transmit errors for multicast packets")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 net/bluetooth/6lowpan.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 4eb1b3ced0d2..bd6dbca5747f 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -474,22 +474,20 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
 		dev = lowpan_btle_dev(entry->netdev);
 
 		list_for_each_entry_rcu(pentry, &dev->peers, list) {
-			int ret;
-
 			local_skb = skb_clone(skb, GFP_ATOMIC);
 
 			BT_DBG("xmit %s to %pMR type %u IP %pI6c chan %p",
 			       netdev->name,
 			       &pentry->chan->dst, pentry->chan->dst_type,
 			       &pentry->peer_addr, pentry->chan);
-			ret = send_pkt(pentry->chan, local_skb, netdev);
-			if (ret < 0)
-				err = ret;
-
+			err = send_pkt(pentry->chan, local_skb, netdev);
 			kfree_skb(local_skb);
+			if (err < 0)
+				goto out;
 		}
 	}
 
+out:
 	rcu_read_unlock();
 
 	return err;
-- 
2.25.1

[PATCH 2/2] Bluetooth: 6LoWPAN: Add missing check for skb_clone

[Jiasheng Jiang]

Add the check for the return value of skb_clone since it may return NULL
pointer and cause NULL pointer dereference in send_pkt.

Fixes: 18722c247023 ("Bluetooth: Enable 6LoWPAN support for BT LE devices")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 net/bluetooth/6lowpan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index bd6dbca5747f..3d4ef94405f8 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -475,6 +475,10 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
 
 		list_for_each_entry_rcu(pentry, &dev->peers, list) {
 			local_skb = skb_clone(skb, GFP_ATOMIC);
+			if (!local_skb) {
+				err = -ENOMEM;
+				goto out;
+			}
 
 			BT_DBG("xmit %s to %pMR type %u IP %pI6c chan %p",
 			       netdev->name,
-- 
2.25.1

Re: [PATCH 1/2] Bluetooth: 6LoWPAN: Modify the error handling in the loop

[Simon Horman]

On Mon, Mar 20, 2023 at 02:31:55PM +0800, Jiasheng Jiang wrote:
> Return the error when send_pkt fails in order to avoid the error being
> overwritten.
> Moreover, remove the redundant 'ret'.
> 
> Fixes: 9c238ca8ec79 ("Bluetooth: 6lowpan: Check transmit errors for multicast packets")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

I see that the error handling is imperfect - only the most recent
error value is returned.

But I think this patch introduces a behavioural change: if
an error occurs then no attempt is made to send the
multicast packet to devices that follow in the list of peers.

If so, I'd want to be sure that behaviour is desirable.

> ---
>  net/bluetooth/6lowpan.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index 4eb1b3ced0d2..bd6dbca5747f 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -474,22 +474,20 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
>  		dev = lowpan_btle_dev(entry->netdev);
>  
>  		list_for_each_entry_rcu(pentry, &dev->peers, list) {
> -			int ret;
> -
>  			local_skb = skb_clone(skb, GFP_ATOMIC);
>  
>  			BT_DBG("xmit %s to %pMR type %u IP %pI6c chan %p",
>  			       netdev->name,
>  			       &pentry->chan->dst, pentry->chan->dst_type,
>  			       &pentry->peer_addr, pentry->chan);
> -			ret = send_pkt(pentry->chan, local_skb, netdev);
> -			if (ret < 0)
> -				err = ret;
> -
> +			err = send_pkt(pentry->chan, local_skb, netdev);
>  			kfree_skb(local_skb);
> +			if (err < 0)
> +				goto out;
>  		}
>  	}
>  
> +out:
>  	rcu_read_unlock();
>  
>  	return err;
> -- 
> 2.25.1
>