linux-kernel - Re: [PATCH] ath9k: fix null-ptr-deref in ath_chanctx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87y1hqtbtu.fsf@toke.dk>
Date:   Fri, 01 Sep 2023 12:41:33 +0200
From:   Toke Høiland-Jørgensen <toke@...e.dk>
To:     Dongliang Mu <dzm91@...t.edu.cn>, Kalle Valo <kvalo@...nel.org>,
        Sujith Manoharan <c_manoha@....qualcomm.com>,
        "John W. Linville" <linville@...driver.com>
Cc:     hust-os-kernel-patches@...glegroups.com,
        Dongliang Mu <dzm91@...t.edu.cn>,
        linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ath9k: fix null-ptr-deref in ath_chanctx_event

Dongliang Mu <dzm91@...t.edu.cn> writes:

> Smatch reports:
>
> 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@...t.edu.cn>
> ---
>  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);

Please don't send patches for static checker errors without actually
checking if there is a valid bug. Which there isn't in this case.

Specifically, that branch of the switch statement dereferences the avp
pointer, which will be NULL if 'vif' is. Meaning we will have crashed
way before reaching this statement if vif is indeed NULL.

-Toke