Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

On 7/2/2026 8:18 PM, Alexander Wilhelm wrote:
> On Thu, Jul 02, 2026 at 10:44:46AM +0200, Alexander Wilhelm wrote:
>> On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
>>>
>>>
>>> On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
>>>> The ath12k_wmi_scan_req_arg structure defines the channel list in
>>>> CPU-native order, while wmi_start_scan_cmd expects the values in
>>>> little-endian format. The simple memcpy causes the hardware scan to fail on
>>>> big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
>>>> values to support both architectures correctly.
>>>>
>>>> Signed-off-by: Alexander Wilhelm <[email protected]>
>>>> ---
>>>>  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>>>> index 65a05a9520ff..9e1d3c662852 100644
>>>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>>>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>>>> @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>  	struct wmi_tlv *tlv;
>>>>  	void *ptr;
>>>>  	int i, ret, len;
>>>> -	u32 *tmp_ptr, extraie_len_with_pad = 0;
>>>> +	__le32 *tmp_ptr;
>>>> +	u32 extraie_len_with_pad = 0;
>>>>  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
>>>>  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
>>>>  
>>>> @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>  	tlv = ptr;
>>>>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
>>>>  	ptr += TLV_HDR_SIZE;
>>>> -	tmp_ptr = (u32 *)ptr;
>>>> +	tmp_ptr = (__le32 *)ptr;
>>>>  
>>>> -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
>>>> +	for (i = 0; i < arg->num_chan; i++)
>>>> +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
>>>>  
>>>>  	ptr += len;
>>>>  
>>>>
>>>
>>> seems hint_s_ssid and hint_bssid at the last also need the endian conversion?
> 
> `hist_s_ssid` and `hint_bssid` are both structs within
> `ath12k_wmi_scan_req_arg`, and the remaining member variables are also stored in
> CPU order. Therefore, it seems wrong to me to perform byte swapping at this
> point. What I actually need to swap is the data written through `ptr`, not the
> structure members themselves. For that reason, I could also use the `__le32
> *tmp_ptr` approach. What do you think about that?

Maybe I was not clear. I intended to mean the ptr:

@@ -2728,8 +2728,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
                ptr += TLV_HDR_SIZE;
                s_ssid = ptr;
                for (i = 0; i < arg->num_hint_s_ssid; ++i) {
-                       s_ssid->freq_flags = arg->hint_s_ssid[i].freq_flags;
-                       s_ssid->short_ssid = arg->hint_s_ssid[i].short_ssid;
+                       s_ssid->freq_flags = cpu_to_le32(arg->hint_s_ssid[i].freq_flags);
+                       s_ssid->short_ssid = cpu_to_le32(arg->hint_s_ssid[i].short_ssid);
                        s_ssid++;
                }
                ptr += len;
@@ -2743,7 +2743,7 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
                hint_bssid = ptr;
                for (i = 0; i < arg->num_hint_bssid; ++i) {
                        hint_bssid->freq_flags =
-                               arg->hint_bssid[i].freq_flags;
+                               cpu_to_le32(arg->hint_bssid[i].freq_flags);
                        ether_addr_copy(&arg->hint_bssid[i].bssid.addr[0],
                                        &hint_bssid->bssid.addr[0]);

> 
> One thing I am still unsure about is the use of `ether_addr_copy()`. Can I
> simply copy the bytes as-is here, or does the address also need to be
> byte-swapped? I could not find any place where this address is being populated,
> so I am not sure what byte order it is expected to be in.

I think you are talking about ath12k_wmi_hint_bssid_arg::bssid, right? For now ath12k does
not populate hint_bssid or hint_s_ssid members, so bssid always remain empty. But if we
are going to populate it I think the address originates from userspace and there it is
passed in byte steams, so byte-swapping not required I think.

> 
> 
> Best regards
> Alexander Wilhelm