2023-03-13 09:11:45

by Jiasheng Jiang

Subject: [PATCH] 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 4eb1b3ced0d2..bf42a0b03e20 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -477,6 +477,10 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
int ret;

local_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!local_skb) {
+ rcu_read_unlock();
+ return -ENOMEM;
+ }

BT_DBG("xmit %s to %pMR type %u IP %pI6c chan %p",
netdev->name,
--
2.25.1

2023-03-13 09:36:55

by bluez.test.bot

Subject: RE: Bluetooth: 6LoWPAN: Add missing check for skb_clone

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=729360

---Test result---

Test Summary:
CheckPatch PASS 0.61 seconds
GitLint PASS 0.29 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 31.73 seconds
CheckAllWarning PASS 35.05 seconds
CheckSparse PASS 39.86 seconds
CheckSmatch PASS 107.69 seconds
BuildKernel32 PASS 31.26 seconds
TestRunnerSetup PASS 444.70 seconds
TestRunner_l2cap-tester PASS 17.17 seconds
TestRunner_iso-tester PASS 17.53 seconds
TestRunner_bnep-tester PASS 5.62 seconds
TestRunner_mgmt-tester PASS 114.29 seconds
TestRunner_rfcomm-tester PASS 9.09 seconds
TestRunner_sco-tester PASS 8.41 seconds
TestRunner_ioctl-tester PASS 9.86 seconds
TestRunner_mesh-tester PASS 7.27 seconds
TestRunner_smp-tester PASS 8.31 seconds
TestRunner_userchan-tester PASS 5.97 seconds
IncrementalBuild PASS 28.66 seconds



---
Regards,
Linux Bluetooth

2023-03-14 14:58:42

by Simon Horman

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

On Mon, Mar 13, 2023 at 05:03:46PM +0800, Jiasheng Jiang wrote:
> 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 4eb1b3ced0d2..bf42a0b03e20 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -477,6 +477,10 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
> int ret;
>
> local_skb = skb_clone(skb, GFP_ATOMIC);
> + if (!local_skb) {
> + rcu_read_unlock();
> + return -ENOMEM;
> + }

Further down in this loop an error is handled as follows,
I wonder if that pattern is appropriate here too.

ret = send_pkt(pentry->chan, local_skb, netdev);
if (ret < 0)
err = ret;

> BT_DBG("xmit %s to %pMR type %u IP %pI6c chan %p",
> netdev->name,
> --
> 2.25.1
>

2023-03-15 07:03:16

by Jiasheng Jiang

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

On Tue, Mar 14, 2023 at 10:58:23PM +0800, Simon Horman wrote:
>On Mon, Mar 13, 2023 at 05:03:46PM +0800, Jiasheng Jiang wrote:
>> 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 4eb1b3ced0d2..bf42a0b03e20 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -477,6 +477,10 @@ static int send_mcast_pkt(struct sk_buff *skb, struct net_device *netdev)
>> int ret;
>>
>> local_skb = skb_clone(skb, GFP_ATOMIC);
>> + if (!local_skb) {
>> + rcu_read_unlock();
>> + return -ENOMEM;
>> + }
>
> Further down in this loop an error is handled as follows,
> I wonder if that pattern is appropriate here too.
>
> ret = send_pkt(pentry->chan, local_skb, netdev);
> if (ret < 0)
> err = ret;
>

I think it should be better to return error here in order to avoid the
error being overwritten.
I will submit a v2 to modify the error handling here.

Thanks,
Jiang