All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method
@ 2024-05-20  2:16 Su Hui
  2024-05-20  2:16 ` [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback() Su Hui
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Su Hui @ 2024-05-20  2:16 UTC (permalink / raw
  To: marcel, luiz.dentz, nathan, ndesaulniers, morbo, justinstitt
  Cc: Su Hui, linux-bluetooth, linux-kernel, llvm, kernel-janitors

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2]  Bluetooth: btintel: fix use after free problem in btintel_ppag_callback()
  2024-05-20  2:16 [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method Su Hui
@ 2024-05-20  2:16 ` Su Hui
  2024-05-21  0:19   ` K, Kiran
  2024-05-20  2:29 ` [1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method bluez.test.bot
  2024-05-20  5:12 ` [PATCH 1/2] " Paul Menzel
  2 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2024-05-20  2:16 UTC (permalink / raw
  To: marcel, luiz.dentz, nathan, ndesaulniers, morbo, justinstitt
  Cc: Su Hui, kiran.k, seema.sreemantha, linux-bluetooth, linux-kernel,
	llvm, kernel-janitors

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method
  2024-05-20  2:16 [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method Su Hui
  2024-05-20  2:16 ` [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback() Su Hui
@ 2024-05-20  2:29 ` bluez.test.bot
  2024-05-20  5:12 ` [PATCH 1/2] " Paul Menzel
  2 siblings, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2024-05-20  2:29 UTC (permalink / raw
  To: linux-bluetooth, suhui

[-- 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method
  2024-05-20  2:16 [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method Su Hui
  2024-05-20  2:16 ` [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback() Su Hui
  2024-05-20  2:29 ` [1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method bluez.test.bot
@ 2024-05-20  5:12 ` Paul Menzel
  2024-05-20  9:41   ` Su Hui
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2024-05-20  5:12 UTC (permalink / raw
  To: Su Hui
  Cc: marcel, luiz.dentz, nathan, ndesaulniers, morbo, justinstitt,
	linux-bluetooth, linux-kernel, llvm, kernel-janitors

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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method
  2024-05-20  5:12 ` [PATCH 1/2] " Paul Menzel
@ 2024-05-20  9:41   ` Su Hui
  0 siblings, 0 replies; 6+ messages in thread
From: Su Hui @ 2024-05-20  9:41 UTC (permalink / raw
  To: Paul Menzel
  Cc: marcel, luiz.dentz, nathan, ndesaulniers, morbo, justinstitt,
	linux-bluetooth, linux-kernel, llvm, kernel-janitors

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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 2/2]  Bluetooth: btintel: fix use after free problem in btintel_ppag_callback()
  2024-05-20  2:16 ` [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback() Su Hui
@ 2024-05-21  0:19   ` K, Kiran
  0 siblings, 0 replies; 6+ messages in thread
From: K, Kiran @ 2024-05-21  0:19 UTC (permalink / raw
  To: Su Hui, marcel@holtmann.org, luiz.dentz@gmail.com,
	nathan@kernel.org, ndesaulniers@google.com, morbo@google.com,
	justinstitt@google.com
  Cc: seema.sreemantha@intel.com, linux-bluetooth@vger.kernel.org,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	kernel-janitors@vger.kernel.org

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-21  0:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-20  2:16 [PATCH 1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method Su Hui
2024-05-20  2:16 ` [PATCH 2/2] Bluetooth: btintel: fix use after free problem in btintel_ppag_callback() Su Hui
2024-05-21  0:19   ` K, Kiran
2024-05-20  2:29 ` [1/2] Bluetooth: btintel: remove useless code in btintel_set_dsm_reset_method bluez.test.bot
2024-05-20  5:12 ` [PATCH 1/2] " Paul Menzel
2024-05-20  9:41   ` Su Hui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.