[PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Rahul Bhattacharjee]

qmi_msg_handler is required to be null terminated by QMI module.
There might be a case where a handler for a msg id is not present in the
handlers array which can lead to infinite loop while searching the handler
and therefore out of bound access in qmi_invoke_handler().
Hence update the initialization in qmi_msg_handler data structure.

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1

Signed-off-by: Rahul Bhattacharjee <quic_rbhattac@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/qmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 145f20a681bd..bda4921208cc 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -3090,6 +3090,7 @@ static const struct qmi_msg_handler ath11k_qmi_msg_handlers[] = {
 			sizeof(struct qmi_wlfw_fw_init_done_ind_msg_v01),
 		.fn = ath11k_qmi_msg_fw_init_done_cb,
 	},
+	{/* end of list */}
 };
 
 static int ath11k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,

base-commit: 087c436cbc8b1bf3d3bc7ea94d6757d74ea2f470
-- 
2.38.0


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Joseph S. Barrera III]

On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote:

>   	},
> +	{/* end of list */}
>   };

Do you want to add a comma after that last list element?

Actually, I normally see the last list element simply being

 > +	{},

... with no comment necessary.



-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Kalle Valo]

"Joseph S. Barrera III" <joebar@chromium.org> writes:

> On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote:
>
>>   	},
>> +	{/* end of list */}
>>   };
>
> Do you want to add a comma after that last list element?

I can add that in the pending branch.

>
> Actually, I normally see the last list element simply being
>
>> +	{},
>
> ... with no comment necessary.

I would prefer to have a comment to make it more visible that an empty
element is needed at the end, but I would add that outside of braces?

/* end of list */
{},

Thoughts? I can change this in the pending branch.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Rahul Bhattacharjee]

On 10/28/2022 4:14 PM, Kalle Valo wrote:
> "Joseph S. Barrera III" <joebar@chromium.org> writes:
>
>> On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote:
>>
>>>    	},
>>> +	{/* end of list */}
>>>    };
>> Do you want to add a comma after that last list element?
> I can add that in the pending branch.
>
>> Actually, I normally see the last list element simply being
>>
>>> +	{},
>> ... with no comment necessary.
> I would prefer to have a comment to make it more visible that an empty
> element is needed at the end, but I would add that outside of braces?
>
> /* end of list */
> {},
>
> Thoughts? I can change this in the pending branch.

LGTM!

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Joseph S. Barrera III]

On 10/28/22 3:44 AM, Kalle Valo wrote:
> "Joseph S. Barrera III" <joebar@chromium.org> writes:
>> Do you want to add a comma after that last list element?
> 
> I can add that in the pending branch.

I think that would be good.

> I would prefer to have a comment to make it more visible that an empty
> element is needed at the end, but I would add that outside of braces?
> 
> /* end of list */
> {},

Ending a list with {}, is such a common idiom that adding a comment
is kind of just noise. And if you look at other such instances of
ending with an empty element, you'll find no comment, just the {},.
When making changes I prefer to stick to the existing coding style
as much as possible, so in this case I would definitely omit the 
comment. But if you do feel like you need to add a comment, I would
keep it on the same line as the empty element, probably like

   {}, // end of list



-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] wifi: ath11k: Fix qmi_msg_handler data structure initialization

[Kalle Valo]

Rahul Bhattacharjee <quic_rbhattac@quicinc.com> wrote:

> qmi_msg_handler is required to be null terminated by QMI module.
> There might be a case where a handler for a msg id is not present in the
> handlers array which can lead to infinite loop while searching the handler
> and therefore out of bound access in qmi_invoke_handler().
> Hence update the initialization in qmi_msg_handler data structure.
> 
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Rahul Bhattacharjee <quic_rbhattac@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

ed3725e15a15 wifi: ath11k: Fix qmi_msg_handler data structure initialization

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20221021090126.28626-1-quic_rbhattac@quicinc.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k