2018-06-01 13:46:29

by Gustavo A. R. Silva

Subject: [PATCH] qtnfmac: fix NULL pointer dereference

In case *vif* is NULL at 655: if (!vif), the execution path jumps to
label out, where *vif* is dereferenced at 679:

if (vif->sta_state == QTNF_STA_CONNECTING)

Fix this by immediately returning when *vif* is NULL instead of
jumping to label out.

Addresses-Coverity-ID: 1469567 ("Dereference after null check")
Fixes: 480daa9cb62c ("qtnfmac: fix invalid STA state on EAPOL failure")
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 220e2b7..ae0ca80 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -654,8 +654,7 @@ qtnf_disconnect(struct wiphy *wiphy, struct net_device *dev,
vif = qtnf_mac_get_base_vif(mac);
if (!vif) {
pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
- ret = -EFAULT;
- goto out;
+ return -EFAULT;
}

if (vif->wdev.iftype != NL80211_IFTYPE_STATION) {
--
2.7.4

2018-06-01 15:52:57

by Kalle Valo

Subject: Re: [PATCH] qtnfmac: fix NULL pointer dereference

"Gustavo A. R. Silva" <[email protected]> writes:

> In case *vif* is NULL at 655: if (!vif), the execution path jumps to
> label out, where *vif* is dereferenced at 679:
>
> if (vif->sta_state == QTNF_STA_CONNECTING)
>
> Fix this by immediately returning when *vif* is NULL instead of
> jumping to label out.
>
> Addresses-Coverity-ID: 1469567 ("Dereference after null check")
> Fixes: 480daa9cb62c ("qtnfmac: fix invalid STA state on EAPOL failure")
> Signed-off-by: Gustavo A. R. Silva <[email protected]>

As commit 480daa9cb62c was recently applied to wireless-drivers-next
I'll queue this to 4.18.

--
Kalle Valo

2018-06-18 08:55:36

by Kalle Valo

Subject: Re: qtnfmac: fix NULL pointer dereference

"Gustavo A. R. Silva" <[email protected]> wrote:

> In case *vif* is NULL at 655: if (!vif), the execution path jumps to
> label out, where *vif* is dereferenced at 679:
>
> if (vif->sta_state == QTNF_STA_CONNECTING)
>
> Fix this by immediately returning when *vif* is NULL instead of
> jumping to label out.
>
> Addresses-Coverity-ID: 1469567 ("Dereference after null check")
> Fixes: 480daa9cb62c ("qtnfmac: fix invalid STA state on EAPOL failure")
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> Reviewed-by: Sergey Matyukevich <[email protected]>

Patch applied to wireless-drivers.git, thanks.

c1e3f64f8738 qtnfmac: fix NULL pointer dereference

--
https://patchwork.kernel.org/patch/10443507/

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

2018-06-01 14:09:07

by Sergey Matyukevich

Subject: Re: [PATCH] qtnfmac: fix NULL pointer dereference

Hello Gustavo,

> diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> index 220e2b7..ae0ca80 100644
> --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
> @@ -654,8 +654,7 @@ qtnf_disconnect(struct wiphy *wiphy, struct net_device *dev,
> vif = qtnf_mac_get_base_vif(mac);
> if (!vif) {
> pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
> - ret = -EFAULT;
> - goto out;
> + return -EFAULT;
> }
>
> if (vif->wdev.iftype != NL80211_IFTYPE_STATION) {

That was my fault. Thanks for the fix!

Reviewed-by: Sergey Matyukevich <[email protected]>

Regards,
Sergey

2018-06-01 14:35:58

by Gustavo A. R. Silva

Subject: Re: [PATCH] qtnfmac: fix NULL pointer dereference

Hi Sergey,

On 06/01/2018 09:08 AM, Sergey Matyukevich wrote:
> Hello Gustavo,
>
>> diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
>> index 220e2b7..ae0ca80 100644
>> --- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
>> +++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
>> @@ -654,8 +654,7 @@ qtnf_disconnect(struct wiphy *wiphy, struct net_device *dev,
>> vif = qtnf_mac_get_base_vif(mac);
>> if (!vif) {
>> pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
>> - ret = -EFAULT;
>> - goto out;
>> + return -EFAULT;
>> }
>>
>> if (vif->wdev.iftype != NL80211_IFTYPE_STATION) {
>
> That was my fault. Thanks for the fix!
>

Glad to help. :)

> Reviewed-by: Sergey Matyukevich <[email protected]>
>

Thanks
--
Gustavo