[PATCH] wifi: Check if socket flags are valid

[Bert Karwatzki <spasswolf@web.de>]

The check, if a particular SO_* flag_bit is set, may give a wrong result
since sk_flags are part of a union and the union is used otherwise.
This happens, if a socket is not a full socket, like a request socket 
for example.

Add a check to verify, if the union is used for sk_flags.

This solution is taken from commit
e8a64bbaaad1 ("net/sched: taprio: Check if socket flags are valid").

Fixes: 76a853f86c97 ("wifi: free SKBTX_WIFI_STATUS skb tx_flags flag")

Signed-off-by: Bert Karwatzki <spasswolf@web.de>
---
 drivers/net/wireless/ath/wil6210/txrx.h     | 2 +-
 drivers/net/wireless/marvell/mwifiex/main.c | 2 +-
 net/mac80211/mesh.c                         | 2 +-
 net/mac80211/tx.c                           | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.h b/drivers/net/wireless/ath/wil6210/txrx.h
index 33ccd0b248d4..91432b318ec2 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.h
+++ b/drivers/net/wireless/ath/wil6210/txrx.h
@@ -618,7 +618,7 @@ static inline bool wil_need_txstat(struct sk_buff *skb)
 	const u8 *da = wil_skb_get_da(skb);
 
 	return is_unicast_ether_addr(da) && skb->sk &&
-	       sock_flag(skb->sk, SOCK_WIFI_STATUS);
+	       sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_WIFI_STATUS);
 }
 
 static inline void wil_consume_skb(struct sk_buff *skb, bool acked)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 1485f949ad4e..973df2656238 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -913,7 +913,7 @@ mwifiex_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	multicast = is_multicast_ether_addr(skb->data);
 
-	if (unlikely(!multicast && skb->sk &&
+	if (unlikely(!multicast && skb->sk && sk_fullsock(skb->sk) &&
 		     sock_flag(skb->sk, SOCK_WIFI_STATUS) &&
 		     priv->adapter->fw_api_ver == MWIFIEX_FW_V15))
 		skb = mwifiex_clone_skb_for_tx_status(priv,
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index a381b4b756ea..11b6cb639ae7 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -777,7 +777,7 @@ bool ieee80211_mesh_xmit_fast(struct ieee80211_sub_if_data *sdata,
 	if (ethertype < ETH_P_802_3_MIN)
 		return false;
 
-	if (skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))
+	if (skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_WIFI_STATUS))
 		return false;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 3b9392a6ddb2..8b5bcddd5cc9 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2859,7 +2859,7 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (unlikely(!multicast &&
-		     ((skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS)) ||
+		     ((skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_WIFI_STATUS)) ||
 		      ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS)))
 		info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
 						  cookie);
@@ -3756,7 +3756,7 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
 		return false;
 
 	/* don't handle TX status request here either */
-	if (skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))
+	if (skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_WIFI_STATUS))
 		return false;
 
 	if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
@@ -4648,7 +4648,7 @@ static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata,
 			memcpy(IEEE80211_SKB_CB(seg), info, sizeof(*info));
 	}
 
-	if (unlikely(skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))) {
+	if (unlikely(skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_WIFI_STATUS))) {
 		info->status_data = ieee80211_store_ack_skb(local, skb,
 							    &info->flags, NULL);
 		if (info->status_data)
-- 
2.49.0

I've dug a little into the history of sk_flags (which have been introduced in
v4.4) and found commit e8a64bbaaad1 ("net/sched: taprio: Check if socket flags 
are valid"), which seems to address the same problem we're currently facing:

commit e8a64bbaaad1f6548cec5508297bc6d45e8ab69e
Author: Benedikt Spranger <b.spranger@linutronix.de>
Date:   Fri Apr 8 11:47:45 2022 +0200

    net/sched: taprio: Check if socket flags are valid
    
    A user may set the SO_TXTIME socket option to ensure a packet is send
    at a given time. The taprio scheduler has to confirm, that it is allowed
    to send a packet at that given time, by a check against the packet time
    schedule. The scheduler drop the packet, if the gates are closed at the
    given send time.
    
    The check, if SO_TXTIME is set, may fail since sk_flags are part of an
    union and the union is used otherwise. This happen, if a socket is not
    a full socket, like a request socket for example.
    
    Add a check to verify, if the union is used for sk_flags.
    
    Fixes: 4cfd5779bd6e ("taprio: Add support for txtime-assist mode")
    Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
    Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
    Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 377f896bdedc..b9c71a304d39 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -417,7 +417,8 @@ static int taprio_enqueue_one(struct sk_buff *skb, struct Qdisc *sch,
 {
 	struct taprio_sched *q = qdisc_priv(sch);
 
-	if (skb->sk && sock_flag(skb->sk, SOCK_TXTIME)) {
+	/* sk_flags are only safe to use on full sockets. */
+	if (skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_TXTIME)) {
 		if (!is_valid_interval(skb, sch))
 			return qdisc_drop(skb, sch, to_free);
 	} else if (TXTIME_ASSIST_IS_ENABLED(q->flags)) {

I'm not sure if all sk_fullsock() checks are necessary, or if it can
be guessed from context if the socket is valid, though.

This has been tested for ~1h so far.

Bert Karwatzki