2024-05-20 02:17:24

by Su Hui

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method

Clang static checker(scan-build) warning:
drivers/bluetooth/btintel.c:2537:14:
Value stored to 'handle' during its initialization is never read.

No need to repeatedly assign values to 'handle'. Remove this useless
code to save some space.

Signed-off-by: Su Hui <[email protected]>
---
drivers/bluetooth/btintel.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 0c855c3ee1c1..f1c101dc0c28 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2542,8 +2542,6 @@ static void btintel_set_dsm_reset_method(struct hci_dev *hdev,
RESET_TYPE_VSEC
};

- handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
-
if (!handle) {
bt_dev_dbg(hdev, "No support for bluetooth device in ACPI firmware");
return;
--
2.30.2



2024-05-20 02:17:31

by Su Hui

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback()

Clang static checker(scan-build) warning:
drivers/bluetooth/btintel.c:1369:8: Use of memory after it is freed.

'p' is equal to 'buffer.pointer', using of 'p->type' after releasing
'buffer.pointer' causes this use after free problem.
Change the order of releasing buffer.pointer to fix this problem.

Fixes: c585a92b2f9c ("Bluetooth: btintel: Set Per Platform Antenna Gain(PPAG)")
Signed-off-by: Su Hui <[email protected]>
---
drivers/bluetooth/btintel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index f1c101dc0c28..d94a8ccd1428 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1364,9 +1364,9 @@ static acpi_status btintel_ppag_callback(acpi_handle handle, u32 lvl, void *data
ppag = (struct btintel_ppag *)data;

if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) {
- kfree(buffer.pointer);
bt_dev_warn(hdev, "PPAG-BT: Invalid object type: %d or package count: %d",
p->type, p->package.count);
+ kfree(buffer.pointer);
ppag->status = AE_ERROR;
return AE_ERROR;
}
--
2.30.2


2024-05-20 02:30:00

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: drivers/bluetooth/btintel.c:1364
error: drivers/bluetooth/btintel.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

2024-05-20 05:13:06

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method

Dear Su,


Thank you for your patch. Some minor comments.


Am 20.05.24 um 04:16 schrieb Su Hui:
> Clang static checker(scan-build) warning:

Please add a space before (. Noting the version of scan build would also
be nice.

> drivers/bluetooth/btintel.c:2537:14:
> Value stored to 'handle' during its initialization is never read.
>
> No need to repeatedly assign values to 'handle'. Remove this useless
> code to save some space.

The plural “values” is misleading to me. Maybe just remove the sentence,
and say:

Remove this unused assignment.

For the summary, “useless code” could also be more specific:

Bluetooth: btintel: Remove unused assignement in
btintel_set_dsm_reset_method()

Maybe also add a Fixes: tag.

> Signed-off-by: Su Hui <[email protected]>


Kind regards,

Paul


> ---
> drivers/bluetooth/btintel.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 0c855c3ee1c1..f1c101dc0c28 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2542,8 +2542,6 @@ static void btintel_set_dsm_reset_method(struct hci_dev *hdev,
> RESET_TYPE_VSEC
> };
>
> - handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
> -
> if (!handle) {
> bt_dev_dbg(hdev, "No support for bluetooth device in ACPI firmware");
> return;

2024-05-20 09:42:07

by Su Hui

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method

On 2024/5/20 13:12, Paul Menzel wrote:
> Dear Su,
>
>
> Thank you for your patch. Some minor comments.
>
>
> Am 20.05.24 um 04:16 schrieb Su Hui:
>> Clang static checker(scan-build) warning:
>
> Please add a space before (. Noting the version of scan build would
> also be nice.
Sure, I will add this in v2 patch. By the way, the scan-build's version
is llvm-17.0.
>
>> drivers/bluetooth/btintel.c:2537:14:
>> Value stored to 'handle' during its initialization is never read.
>>
>> No need to repeatedly assign values to 'handle'. Remove this useless
>> code to save some space.
>
> The plural “values” is misleading to me. Maybe just remove the
> sentence, and say:
>
> Remove this unused assignment.
>
> For the summary, “useless code” could also be more specific:
>
> Bluetooth: btintel: Remove unused assignement in
> btintel_set_dsm_reset_method()
Yes, it's better for me.
>
> Maybe also add a Fixes: tag.

It's a cleanup not a bug fixing, so I think Fixes: tag is unnecessary.

Thanks for you suggestions! I will send v2 patch after the CI tests done.

Su Hui

>
>
>> ---
>>   drivers/bluetooth/btintel.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index 0c855c3ee1c1..f1c101dc0c28 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -2542,8 +2542,6 @@ static void btintel_set_dsm_reset_method(struct
>> hci_dev *hdev,
>>           RESET_TYPE_VSEC
>>       };
>>   -    handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
>> -
>>       if (!handle) {
>>           bt_dev_dbg(hdev, "No support for bluetooth device in ACPI
>> firmware");
>>           return;

2024-05-21 00:20:23

by K, Kiran

[permalink] [raw]
Subject: RE: [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback()

Hi Su Hui,

Thanks for your patch. 'btintel_ppag_callback' has been removed as part of 287da9035b2e.

>-----Original Message-----
>From: Su Hui <[email protected]>
>Sent: Monday, May 20, 2024 7:46 AM
>To: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]
>Cc: Su Hui <[email protected]>; K, Kiran <[email protected]>;
>[email protected]; [email protected]; linux-
>[email protected]; [email protected]; [email protected]
>Subject: [PATCH 2/2] Bluetooth: btintel: fix use after free problem in
>btintel_ppag_callback()
>
>Clang static checker(scan-build) warning:
>drivers/bluetooth/btintel.c:1369:8: Use of memory after it is freed.
>
>'p' is equal to 'buffer.pointer', using of 'p->type' after releasing 'buffer.pointer'
>causes this use after free problem.
>Change the order of releasing buffer.pointer to fix this problem.
>
>Fixes: c585a92b2f9c ("Bluetooth: btintel: Set Per Platform Antenna
>Gain(PPAG)")
>Signed-off-by: Su Hui <[email protected]>
>---
> drivers/bluetooth/btintel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index
>f1c101dc0c28..d94a8ccd1428 100644
>--- a/drivers/bluetooth/btintel.c
>+++ b/drivers/bluetooth/btintel.c
>@@ -1364,9 +1364,9 @@ static acpi_status btintel_ppag_callback(acpi_handle
>handle, u32 lvl, void *data
> ppag = (struct btintel_ppag *)data;
>
> if (p->type != ACPI_TYPE_PACKAGE || p->package.count != 2) {
>- kfree(buffer.pointer);
> bt_dev_warn(hdev, "PPAG-BT: Invalid object type: %d or
>package count: %d",
> p->type, p->package.count);
>+ kfree(buffer.pointer);
> ppag->status = AE_ERROR;
> return AE_ERROR;
> }
>--
>2.30.2

Thanks,
Kiran