* [bug report] ath10k: abstract htt_rx_desc structure
@ 2022-02-01 13:09 Dan Carpenter
2022-02-06 10:25 ` Francesco Magliocca
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-02-01 13:09 UTC (permalink / raw)
To: franciman12; +Cc: ath10k
Hello Francesco Magliocca,
The patch 6bae9de622d3: "ath10k: abstract htt_rx_desc structure" from
Jan 12, 2022, leads to the following Smatch static checker warning:
drivers/net/wireless/ath/ath10k/htt_rx.c:432 ath10k_htt_rx_amsdu_pop()
warn: potential pointer math issue ('rx_desc' is a 32 bit pointer)
drivers/net/wireless/ath/ath10k/htt_rx.c
346 static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
347 struct sk_buff_head *amsdu)
348 {
349 struct ath10k *ar = htt->ar;
350 struct ath10k_hw_params *hw = &ar->hw_params;
351 int msdu_len, msdu_chaining = 0;
352 struct sk_buff *msdu;
353 struct htt_rx_desc *rx_desc;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
354 struct rx_attention *rx_desc_attention;
355 struct rx_frag_info_common *rx_desc_frag_info_common;
356 struct rx_msdu_start_common *rx_desc_msdu_start_common;
357 struct rx_msdu_end_common *rx_desc_msdu_end_common;
358
[ snip ]
427
428 last_msdu = __le32_to_cpu(rx_desc_msdu_end_common->info0) &
429 RX_MSDU_END_INFO0_LAST_MSDU;
430
431 /* FIXME: why are we skipping the first part of the rx_desc? */
--> 432 trace_ath10k_htt_rx_desc(ar, rx_desc + sizeof(u32),
^^^^^^^^^^^^^^^^^^^^^
This is a pointer math bug. It's possible that it should be:
trace_ath10k_htt_rx_desc(ar, (u8 *)rx_desc + sizeof(u32),
But as your FIXME notes, it's hard to tell what's going on here...
433 hw->rx_desc_ops->rx_desc_size - sizeof(u32));
434
435 if (last_msdu)
436 break;
437 }
regards,
dan carpenter
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ath10k: abstract htt_rx_desc structure
2022-02-01 13:09 [bug report] ath10k: abstract htt_rx_desc structure Dan Carpenter
@ 2022-02-06 10:25 ` Francesco Magliocca
2022-02-07 8:20 ` Kalle Valo
0 siblings, 1 reply; 3+ messages in thread
From: Francesco Magliocca @ 2022-02-06 10:25 UTC (permalink / raw)
To: Dan Carpenter, Kalle Valo; +Cc: ath10k
Hello Dan Carpenter,
auch, nice catch, thank you very much.
Yes, your solution is correct,
even if I think that we could avoid skipping the first part of the
rx_desc altogether,
because it is just a trace.
Kalle, will you amend my commit or do I need to file a new patch?
Best regards,
Francesco Magliocca
Il giorno mar 1 feb 2022 alle ore 14:09 Dan Carpenter
<dan.carpenter@oracle.com> ha scritto:
>
> Hello Francesco Magliocca,
>
> The patch 6bae9de622d3: "ath10k: abstract htt_rx_desc structure" from
> Jan 12, 2022, leads to the following Smatch static checker warning:
>
> drivers/net/wireless/ath/ath10k/htt_rx.c:432 ath10k_htt_rx_amsdu_pop()
> warn: potential pointer math issue ('rx_desc' is a 32 bit pointer)
>
> drivers/net/wireless/ath/ath10k/htt_rx.c
> 346 static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
> 347 struct sk_buff_head *amsdu)
> 348 {
> 349 struct ath10k *ar = htt->ar;
> 350 struct ath10k_hw_params *hw = &ar->hw_params;
> 351 int msdu_len, msdu_chaining = 0;
> 352 struct sk_buff *msdu;
> 353 struct htt_rx_desc *rx_desc;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 354 struct rx_attention *rx_desc_attention;
> 355 struct rx_frag_info_common *rx_desc_frag_info_common;
> 356 struct rx_msdu_start_common *rx_desc_msdu_start_common;
> 357 struct rx_msdu_end_common *rx_desc_msdu_end_common;
> 358
>
> [ snip ]
>
> 427
> 428 last_msdu = __le32_to_cpu(rx_desc_msdu_end_common->info0) &
> 429 RX_MSDU_END_INFO0_LAST_MSDU;
> 430
> 431 /* FIXME: why are we skipping the first part of the rx_desc? */
> --> 432 trace_ath10k_htt_rx_desc(ar, rx_desc + sizeof(u32),
> ^^^^^^^^^^^^^^^^^^^^^
> This is a pointer math bug. It's possible that it should be:
>
> trace_ath10k_htt_rx_desc(ar, (u8 *)rx_desc + sizeof(u32),
>
> But as your FIXME notes, it's hard to tell what's going on here...
>
>
> 433 hw->rx_desc_ops->rx_desc_size - sizeof(u32));
> 434
> 435 if (last_msdu)
> 436 break;
> 437 }
>
> regards,
> dan carpenter
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ath10k: abstract htt_rx_desc structure
2022-02-06 10:25 ` Francesco Magliocca
@ 2022-02-07 8:20 ` Kalle Valo
0 siblings, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2022-02-07 8:20 UTC (permalink / raw)
To: Francesco Magliocca; +Cc: Dan Carpenter, ath10k
Francesco Magliocca <franciman12@gmail.com> writes:
> Hello Dan Carpenter,
> auch, nice catch, thank you very much.
> Yes, your solution is correct,
> even if I think that we could avoid skipping the first part of the
> rx_desc altogether,
> because it is just a trace.
>
> Kalle, will you amend my commit or do I need to file a new patch?
I don't rebase my trees, so please submit a new patch fixing this issue.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-07 8:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 13:09 [bug report] ath10k: abstract htt_rx_desc structure Dan Carpenter
2022-02-06 10:25 ` Francesco Magliocca
2022-02-07 8:20 ` Kalle Valo
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.