2023-03-20 06:32:22

by Jiasheng Jiang

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

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 <[email protected]>
---
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

2023-03-20 06:32:23

by Jiasheng Jiang

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

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 <[email protected]>
---
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

2023-03-20 07:01:51

by bluez.test.bot

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=731694

---Test result---

Test Summary:
CheckPatch PASS 1.19 seconds
GitLint PASS 0.58 seconds
SubjectPrefix PASS 0.21 seconds
BuildKernel PASS 31.55 seconds
CheckAllWarning PASS 33.97 seconds
CheckSparse PASS 38.70 seconds
CheckSmatch PASS 107.11 seconds
BuildKernel32 PASS 30.13 seconds
TestRunnerSetup PASS 432.96 seconds
TestRunner_l2cap-tester PASS 16.56 seconds
TestRunner_iso-tester PASS 16.45 seconds
TestRunner_bnep-tester PASS 5.35 seconds
TestRunner_mgmt-tester PASS 107.41 seconds
TestRunner_rfcomm-tester PASS 8.65 seconds
TestRunner_sco-tester PASS 7.99 seconds
TestRunner_ioctl-tester PASS 9.25 seconds
TestRunner_mesh-tester PASS 6.81 seconds
TestRunner_smp-tester PASS 7.76 seconds
TestRunner_userchan-tester PASS 5.58 seconds
IncrementalBuild PASS 33.26 seconds



---
Regards,
Linux Bluetooth

2023-03-20 16:19:36

by Simon Horman

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

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 <[email protected]>

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
>