Re: [PATCH] wifi: brcmfmac: Check the return value of of_property_read_string_index

[Arend van Spriel <arend.vanspriel@broadcom.com>]

On 1/6/2025 11:37 AM, Stefan Dösinger wrote:
> Somewhen between 6.10 and 6.11 the driver started to crash on my
> MacBookPro14,3. The property doesn't exist and 'tmp' remains
> uninitialized, so we pass a random pointer to devm_kstrdup().

By the looks of it this is an intel-based platform. Is that correct? So 
does it have a devicetree? I would expect the root node find to fail, 
but apparently is does not. Strange though that root node does not have 
a compatible property. Anyway, the analysis looks sane so ...

minor remark below.

Acked-by: Arend van Spriel
> Signed-off-by: Stefan Dösinger <stefan@codeweavers.com>
> 

[...]

> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> index c1f18e2fe540..ee589a7b4f4f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -99,13 +99,15 @@ int brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   	/* Set board-type to the first string of the machine compatible prop */
>   	root = of_find_node_by_path("/");
>   	if (root && err) {
> -		char *board_type;
> +		char *board_type = NULL;
>   		const char *tmp;
>   
> -		of_property_read_string_index(root, "compatible", 0, &tmp);
> +		err = of_property_read_string_index(root, "compatible", 0, &tmp);
>   
>   		/* get rid of '/' in the compatible string to be able to find the FW */
> -		board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);

No need to use 'err'. You can directly do 
of_property_read_string_index() in the if statement below.

> +		if (!err)
> +			board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
> +
>   		if (!board_type) {
>   			of_node_put(root);
>   			return 0;