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

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

[Su Hui]

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 <suhui@nfschina.com>
---
 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

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

[Su Hui]

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 <suhui@nfschina.com>
---
 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

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

[K, Kiran]

Hi Su Hui,

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

>-----Original Message-----
>From: Su Hui <suhui@nfschina.com>
>Sent: Monday, May 20, 2024 7:46 AM
>To: marcel@holtmann.org; luiz.dentz@gmail.com; nathan@kernel.org;
>ndesaulniers@google.com; morbo@google.com; justinstitt@google.com
>Cc: Su Hui <suhui@nfschina.com>; K, Kiran <kiran.k@intel.com>;
>seema.sreemantha@intel.com; linux-bluetooth@vger.kernel.org; linux-
>kernel@vger.kernel.org; llvm@lists.linux.dev; kernel-janitors@vger.kernel.org
>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 <suhui@nfschina.com>
>---
> 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

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

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

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

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

[Paul Menzel]

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 <suhui@nfschina.com>


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;

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

[Su Hui]

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;