Re: [PATCH 2/2] Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bluetooth: btbcm: devm_kstrdup memory leak of board string on repeated initialization
--

commit d78be9f56325d78f88a3903ffe62866de6e22e52
Author: Thomas Gerner <thomas.gerner@muenchen-mail.de>

Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

The bluetooth module on the BeagleV-Ahead reports LE tx power read
capabilities but fails when actually doing it. This patch adds a quirk to
disable this reading based on the board's compatible string from the DTB.

> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 463d59890befe..2d02cc1863d0b 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
[ ... ]
> @@ -537,12 +542,13 @@ static const struct bcm_subver_table bcm_usb_subver_table[] = {
>   * This currently only looks up the device tree board appendix,
>   * but can be expanded to other mechanisms.
>   */
> -static const char *btbcm_get_board_name(struct device *dev)
> +static const char *btbcm_get_board_name(struct hci_dev *hdev)
>  {
>  #ifdef CONFIG_OF
>  	struct device_node *root __free(device_node) = of_find_node_by_path("/");
>  	char *board_type;
>  	const char *tmp;
> +	const char **s;
>  
>  	if (!root)
>  		return NULL;
> @@ -550,8 +556,15 @@ static const char *btbcm_get_board_name(struct device *dev)
>  	if (of_property_read_string_index(root, "compatible", 0, &tmp))
>  		return NULL;
>  
> +	/* disable broken Read LE Min/Max Tx Power on first board found in list */
> +	for (s = board_broken_read_transmit_power; *s; s++)
> +		if (!strcmp(tmp, *s)) {
> +			hci_set_quirk(hdev, HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER);
> +			break;
> +		}
> +
>  	/* get rid of any '/' in the compatible string */
> -	board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
> +	board_type = devm_kstrdup(&hdev->dev, tmp, GFP_KERNEL);

[Severity: Low]
Can this leak the board string on repeated initialization?

During device setup, btbcm_setup_patchram() calls btbcm_initialize(), which
calls btbcm_get_board_name(). The board_type string is allocated using
devm_kstrdup() and returned, but it is only stored locally as board_name in
btbcm_initialize() and discarded when the function returns.

Since btbcm_setup_patchram() subsequently calls btbcm_finalize(), which will
invoke btbcm_initialize() a second time if the firmware was successfully
loaded, the devm_kstrdup() allocation happens again, adding another permanent
entry to the devres list.

Could kstrdup() be used instead so the memory can be explicitly freed when it
is no longer needed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791720467.git.thomas.gerner@muenchen-mail.de?part=2