Dongliang Mu <dzm91@xxxxxxxxxxx> writes:
On 2023/9/1 18:41, 'Toke Høiland-Jørgensen' via HUST OS KernelBut only this one has ATH_CHANCTX_EVENT_BEACON_PREPARE as an argument,
Contribution wrote:
Dongliang Mu <dzm91@xxxxxxxxxxx> writes:Before sending this patch, I searched in the code, there are many call
Smatch reports:Please don't send patches for static checker errors without actually
ath_chanctx_event() error: we previously assumed 'vif' could be null
The function ath_chanctx_event can be called with vif argument as NULL.
If vif is NULL, ath_dbg can trigger a null pointer dereference.
Fix this by adding a null pointer check.
Fixes: 878066e745b5 ("ath9k: Add more debug statements for channel context")
Signed-off-by: Dongliang Mu <dzm91@xxxxxxxxxxx>
---
drivers/net/wireless/ath/ath9k/channel.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index 571062f2e82a..e343c8962d14 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -576,7 +576,9 @@ void ath_chanctx_event(struct ath_softc *sc, struct ieee80211_vif *vif,
if (sc->sched.state != ATH_CHANCTX_STATE_WAIT_FOR_BEACON)
break;
- ath_dbg(common, CHAN_CTX, "Preparing beacon for vif: %pM\n", vif->addr);
+ if (vif)
+ ath_dbg(common, CHAN_CTX,
+ "Preparing beacon for vif: %pM\n", vif->addr);
checking if there is a valid bug. Which there isn't in this case.
sites of ath_chanctx_event with argument vif as NULL.
Functions calling this function: ath_chanctx_event
File Function Line
0 beacon.c ath9k_beacon_tasklet 459 ath_chanctx_event(sc, vif,
ATH_CHANCTX_EVENT_BEACON_PREPARE);
which is required to hit the code path you are changing.
1 channel.c ath_chanctx_check_active 321 ath_chanctx_event(sc, NULL,There is no crash, see above.
2 channel.c ath_chanctx_beacon_sent_ev 781 ath_chanctx_event(sc, NULL, ev);
3 channel.c ath_chanctx_beacon_recv_ev 787 ath_chanctx_event(sc, NULL, ev);
4 channel.c ath_chanctx_timer 1054 ath_chanctx_event(sc, NULL,
ATH_CHANCTX_EVENT_TSF_TIMER);
5 channel.c ath_chanctx_set_next 1321 ath_chanctx_event(sc, NULL,
ATH_CHANCTX_EVENT_SWITCH);
6 channel.c ath9k_p2p_ps_timer 1566 ath_chanctx_event(sc, NULL,
ATH_CHANCTX_EVENT_TSF_TIMER);
7 main.c ath9k_sta_state 1671 ath_chanctx_event(sc, vif,
8 main.c ath9k_remove_chanctx 2577 ath_chanctx_event(sc, NULL,
ATH_CHANCTX_EVENT_UNASSIGN);
9 xmit.c ath_tx_edma_tasklet 2749 ath_chanctx_event(sc, NULL,
This NULL parameters would cause some abnormal behaviors.
Specifically, that branch of the switch statement dereferences the avpYeah, you are right. However, no matter where or which variable causing
pointer, which will be NULL if 'vif' is. Meaning we will have crashed
way before reaching this statement if vif is indeed NULL.
the null-ptr-def crash, the crash is there.
-Toke