| 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
| ||
|
Message-ID: <20250518164546.4612-1-spasswolf@web.de>
Date: Sun, 18 May 2025 18:45:45 +0200
From: Bert Karwatzki <spasswolf@....de>
To: linux-wireless@...r.kernel.org
Cc: Bert Karwatzki <spasswolf@....de>,
linux-kernel@...r.kernel.org,
linux-next@...r.kernel.org,
Johannes Berg <johannes.berg@...el.com>,
Jason Xing <kerneljasonxing@...il.com>,
Thomas Gleixner <tglx@...utronix.de>
Subject: [PATCH] wifi: Check if socket flags are valid
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@....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@...utronix.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@...utronix.de>
Reviewed-by: Kurt Kanzenbach <kurt@...utronix.de>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>
Signed-off-by: David S. Miller <davem@...emloft.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
Powered by blists - more mailing lists