* [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning
@ 2023-11-07 12:12 Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 2/9] wifi: mac80211: don't return unset power in ieee80211_get_tx_power() Sasha Levin
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dmitry Antipov, Johannes Berg, Sasha Levin, kvalo, nathan,
ndesaulniers, linux-wireless, llvm
From: Dmitry Antipov <dmantipov@yandex.ru>
[ Upstream commit cbaccdc42483c65016f1bae89128c08dc17cfb2a ]
When compiling with clang 16.0.6 and CONFIG_FORTIFY_SOURCE=y, I've
noticed the following (somewhat confusing due to absence of an actual
source code location):
In file included from drivers/net/wireless/virtual/mac80211_hwsim.c:18:
In file included from ./include/linux/slab.h:16:
In file included from ./include/linux/gfp.h:7:
In file included from ./include/linux/mmzone.h:8:
In file included from ./include/linux/spinlock.h:56:
In file included from ./include/linux/preempt.h:79:
In file included from ./arch/x86/include/asm/preempt.h:9:
In file included from ./include/linux/thread_info.h:60:
In file included from ./arch/x86/include/asm/thread_info.h:53:
In file included from ./arch/x86/include/asm/cpufeature.h:5:
In file included from ./arch/x86/include/asm/processor.h:23:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:11:
In file included from ./include/linux/string.h:254:
./include/linux/fortify-string.h:592:4: warning: call to '__read_overflow2_field'
declared with 'warning' attribute: detected read beyond size of field (2nd
parameter); maybe use struct_group()? [-Wattribute-warning]
__read_overflow2_field(q_size_field, size);
The compiler actually complains on 'mac80211_hwsim_get_et_strings()' where
fortification logic inteprets call to 'memcpy()' as an attempt to copy the
whole 'mac80211_hwsim_gstrings_stats' array from its first member and so
issues an overread warning. This warning may be silenced by passing
an address of the whole array and not the first member to 'memcpy()'.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Link: https://lore.kernel.org/r/20230829094140.234636-1-dmantipov@yandex.ru
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/mac80211_hwsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index a21739b2f44e6..634e8c1e71cca 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2323,7 +2323,7 @@ static void mac80211_hwsim_get_et_strings(struct ieee80211_hw *hw,
u32 sset, u8 *data)
{
if (sset == ETH_SS_STATS)
- memcpy(data, *mac80211_hwsim_gstrings_stats,
+ memcpy(data, mac80211_hwsim_gstrings_stats,
sizeof(mac80211_hwsim_gstrings_stats));
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 2/9] wifi: mac80211: don't return unset power in ieee80211_get_tx_power()
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 3/9] wifi: ath9k: fix clang-specific fortify warnings Sasha Levin
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Ping-Ke Shih, Zong-Zhe Yang, Johannes Berg, Sasha Levin, johannes,
davem, edumazet, kuba, pabeni, linux-wireless, netdev
From: Ping-Ke Shih <pkshih@realtek.com>
[ Upstream commit e160ab85166e77347d0cbe5149045cb25e83937f ]
We can get a UBSAN warning if ieee80211_get_tx_power() returns the
INT_MIN value mac80211 internally uses for "unset power level".
UBSAN: signed-integer-overflow in net/wireless/nl80211.c:3816:5
-2147483648 * 100 cannot be represented in type 'int'
CPU: 0 PID: 20433 Comm: insmod Tainted: G WC OE
Call Trace:
dump_stack+0x74/0x92
ubsan_epilogue+0x9/0x50
handle_overflow+0x8d/0xd0
__ubsan_handle_mul_overflow+0xe/0x10
nl80211_send_iface+0x688/0x6b0 [cfg80211]
[...]
cfg80211_register_wdev+0x78/0xb0 [cfg80211]
cfg80211_netdev_notifier_call+0x200/0x620 [cfg80211]
[...]
ieee80211_if_add+0x60e/0x8f0 [mac80211]
ieee80211_register_hw+0xda5/0x1170 [mac80211]
In this case, simply return an error instead, to indicate
that no data is available.
Cc: Zong-Zhe Yang <kevin_yang@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Link: https://lore.kernel.org/r/20230203023636.4418-1-pkshih@realtek.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/mac80211/cfg.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9e3bff5aaf8b8..6428c0d371458 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2581,6 +2581,10 @@ static int ieee80211_get_tx_power(struct wiphy *wiphy,
else
*dbm = sdata->vif.bss_conf.txpower;
+ /* INT_MIN indicates no power level was set yet */
+ if (*dbm == INT_MIN)
+ return -EINVAL;
+
return 0;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 3/9] wifi: ath9k: fix clang-specific fortify warnings
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 2/9] wifi: mac80211: don't return unset power in ieee80211_get_tx_power() Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 4/9] wifi: ath10k: fix clang-specific fortify warning Sasha Levin
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dmitry Antipov, Toke Høiland-Jørgensen, Kalle Valo,
Sasha Levin, kvalo, nathan, ndesaulniers, linux-wireless, llvm
From: Dmitry Antipov <dmantipov@yandex.ru>
[ Upstream commit 95f97fe0ac974467ab4da215985a32b2fdf48af0 ]
When compiling with clang 16.0.6 and CONFIG_FORTIFY_SOURCE=y, I've
noticed the following (somewhat confusing due to absence of an actual
source code location):
In file included from drivers/net/wireless/ath/ath9k/debug.c:17:
In file included from ./include/linux/slab.h:16:
In file included from ./include/linux/gfp.h:7:
In file included from ./include/linux/mmzone.h:8:
In file included from ./include/linux/spinlock.h:56:
In file included from ./include/linux/preempt.h:79:
In file included from ./arch/x86/include/asm/preempt.h:9:
In file included from ./include/linux/thread_info.h:60:
In file included from ./arch/x86/include/asm/thread_info.h:53:
In file included from ./arch/x86/include/asm/cpufeature.h:5:
In file included from ./arch/x86/include/asm/processor.h:23:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:11:
In file included from ./include/linux/string.h:254:
./include/linux/fortify-string.h:592:4: warning: call to '__read_overflow2_field'
declared with 'warning' attribute: detected read beyond size of field (2nd
parameter); maybe use struct_group()? [-Wattribute-warning]
__read_overflow2_field(q_size_field, size);
In file included from drivers/net/wireless/ath/ath9k/htc_drv_debug.c:17:
In file included from drivers/net/wireless/ath/ath9k/htc.h:20:
In file included from ./include/linux/module.h:13:
In file included from ./include/linux/stat.h:19:
In file included from ./include/linux/time.h:60:
In file included from ./include/linux/time32.h:13:
In file included from ./include/linux/timex.h:67:
In file included from ./arch/x86/include/asm/timex.h:5:
In file included from ./arch/x86/include/asm/processor.h:23:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:11:
In file included from ./include/linux/string.h:254:
./include/linux/fortify-string.h:592:4: warning: call to '__read_overflow2_field'
declared with 'warning' attribute: detected read beyond size of field (2nd
parameter); maybe use struct_group()? [-Wattribute-warning]
__read_overflow2_field(q_size_field, size);
The compiler actually complains on 'ath9k_get_et_strings()' and
'ath9k_htc_get_et_strings()' due to the same reason: fortification logic
inteprets call to 'memcpy()' as an attempt to copy the whole array from
it's first member and so issues an overread warning. These warnings may
be silenced by passing an address of the whole array and not the first
member to 'memcpy()'.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20230829093856.234584-1-dmantipov@yandex.ru
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/ath/ath9k/debug.c | 2 +-
drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 859a865c59950..8d98347e0ddff 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1284,7 +1284,7 @@ void ath9k_get_et_strings(struct ieee80211_hw *hw,
u32 sset, u8 *data)
{
if (sset == ETH_SS_STATS)
- memcpy(data, *ath9k_gstrings_stats,
+ memcpy(data, ath9k_gstrings_stats,
sizeof(ath9k_gstrings_stats));
}
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
index c55aab01fff5d..e79bbcd3279af 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
@@ -428,7 +428,7 @@ void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
u32 sset, u8 *data)
{
if (sset == ETH_SS_STATS)
- memcpy(data, *ath9k_htc_gstrings_stats,
+ memcpy(data, ath9k_htc_gstrings_stats,
sizeof(ath9k_htc_gstrings_stats));
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 4/9] wifi: ath10k: fix clang-specific fortify warning
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 2/9] wifi: mac80211: don't return unset power in ieee80211_get_tx_power() Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 3/9] wifi: ath9k: fix clang-specific fortify warnings Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 5/9] net: annotate data-races around sk->sk_tx_queue_mapping Sasha Levin
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dmitry Antipov, Jeff Johnson, Kalle Valo, Sasha Levin, kvalo,
nathan, ndesaulniers, ath10k, linux-wireless, llvm
From: Dmitry Antipov <dmantipov@yandex.ru>
[ Upstream commit cb4c132ebfeac5962f7258ffc831caa0c4dada1a ]
When compiling with clang 16.0.6 and CONFIG_FORTIFY_SOURCE=y, I've
noticed the following (somewhat confusing due to absence of an actual
source code location):
In file included from drivers/net/wireless/ath/ath10k/debug.c:8:
In file included from ./include/linux/module.h:13:
In file included from ./include/linux/stat.h:19:
In file included from ./include/linux/time.h:60:
In file included from ./include/linux/time32.h:13:
In file included from ./include/linux/timex.h:67:
In file included from ./arch/x86/include/asm/timex.h:5:
In file included from ./arch/x86/include/asm/processor.h:23:
In file included from ./arch/x86/include/asm/msr.h:11:
In file included from ./arch/x86/include/asm/cpumask.h:5:
In file included from ./include/linux/cpumask.h:12:
In file included from ./include/linux/bitmap.h:11:
In file included from ./include/linux/string.h:254:
./include/linux/fortify-string.h:592:4: warning: call to '__read_overflow2_field'
declared with 'warning' attribute: detected read beyond size of field (2nd
parameter); maybe use struct_group()? [-Wattribute-warning]
__read_overflow2_field(q_size_field, size);
The compiler actually complains on 'ath10k_debug_get_et_strings()' where
fortification logic inteprets call to 'memcpy()' as an attempt to copy
the whole 'ath10k_gstrings_stats' array from it's first member and so
issues an overread warning. This warning may be silenced by passing
an address of the whole array and not the first member to 'memcpy()'.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20230829093652.234537-1-dmantipov@yandex.ru
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/ath/ath10k/debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 04c50a26a4f47..34db968c4bd0b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1138,7 +1138,7 @@ void ath10k_debug_get_et_strings(struct ieee80211_hw *hw,
u32 sset, u8 *data)
{
if (sset == ETH_SS_STATS)
- memcpy(data, *ath10k_gstrings_stats,
+ memcpy(data, ath10k_gstrings_stats,
sizeof(ath10k_gstrings_stats));
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 5/9] net: annotate data-races around sk->sk_tx_queue_mapping
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
` (2 preceding siblings ...)
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 4/9] wifi: ath10k: fix clang-specific fortify warning Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 6/9] net: annotate data-races around sk->sk_dst_pending_confirm Sasha Levin
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni, netdev
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit 0bb4d124d34044179b42a769a0c76f389ae973b6 ]
This field can be read or written without socket lock being held.
Add annotations to avoid load-store tearing.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/sock.h | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index f73ef7087a187..b021c8912e2cf 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1782,21 +1782,33 @@ static inline void sk_tx_queue_set(struct sock *sk, int tx_queue)
/* sk_tx_queue_mapping accept only upto a 16-bit value */
if (WARN_ON_ONCE((unsigned short)tx_queue >= USHRT_MAX))
return;
- sk->sk_tx_queue_mapping = tx_queue;
+ /* Paired with READ_ONCE() in sk_tx_queue_get() and
+ * other WRITE_ONCE() because socket lock might be not held.
+ */
+ WRITE_ONCE(sk->sk_tx_queue_mapping, tx_queue);
}
#define NO_QUEUE_MAPPING USHRT_MAX
static inline void sk_tx_queue_clear(struct sock *sk)
{
- sk->sk_tx_queue_mapping = NO_QUEUE_MAPPING;
+ /* Paired with READ_ONCE() in sk_tx_queue_get() and
+ * other WRITE_ONCE() because socket lock might be not held.
+ */
+ WRITE_ONCE(sk->sk_tx_queue_mapping, NO_QUEUE_MAPPING);
}
static inline int sk_tx_queue_get(const struct sock *sk)
{
- if (sk && sk->sk_tx_queue_mapping != NO_QUEUE_MAPPING)
- return sk->sk_tx_queue_mapping;
+ if (sk) {
+ /* Paired with WRITE_ONCE() in sk_tx_queue_clear()
+ * and sk_tx_queue_set().
+ */
+ int val = READ_ONCE(sk->sk_tx_queue_mapping);
+ if (val != NO_QUEUE_MAPPING)
+ return val;
+ }
return -1;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 6/9] net: annotate data-races around sk->sk_dst_pending_confirm
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
` (3 preceding siblings ...)
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 5/9] net: annotate data-races around sk->sk_tx_queue_mapping Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 7/9] wifi: ath10k: Don't touch the CE interrupt registers after power up Sasha Levin
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Dumazet, David S . Miller, Sasha Levin, kuba, pabeni,
dsahern, kuniyu, wuyun.abel, leitao, alexander, dhowells, netdev
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit eb44ad4e635132754bfbcb18103f1dcb7058aedd ]
This field can be read or written without socket lock being held.
Add annotations to avoid load-store tearing.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/sock.h | 6 +++---
net/core/sock.c | 2 +-
net/ipv4/tcp_output.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index b021c8912e2cf..5293f2b65fb55 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1941,7 +1941,7 @@ static inline void dst_negative_advice(struct sock *sk)
if (ndst != dst) {
rcu_assign_pointer(sk->sk_dst_cache, ndst);
sk_tx_queue_clear(sk);
- sk->sk_dst_pending_confirm = 0;
+ WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
}
}
}
@@ -1952,7 +1952,7 @@ __sk_dst_set(struct sock *sk, struct dst_entry *dst)
struct dst_entry *old_dst;
sk_tx_queue_clear(sk);
- sk->sk_dst_pending_confirm = 0;
+ WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
old_dst = rcu_dereference_protected(sk->sk_dst_cache,
lockdep_sock_is_held(sk));
rcu_assign_pointer(sk->sk_dst_cache, dst);
@@ -1965,7 +1965,7 @@ sk_dst_set(struct sock *sk, struct dst_entry *dst)
struct dst_entry *old_dst;
sk_tx_queue_clear(sk);
- sk->sk_dst_pending_confirm = 0;
+ WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
old_dst = xchg((__force struct dst_entry **)&sk->sk_dst_cache, dst);
dst_release(old_dst);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 9979cd602dfac..2c3c5df139345 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -545,7 +545,7 @@ struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
if (dst && dst->obsolete && dst->ops->check(dst, cookie) == NULL) {
sk_tx_queue_clear(sk);
- sk->sk_dst_pending_confirm = 0;
+ WRITE_ONCE(sk->sk_dst_pending_confirm, 0);
RCU_INIT_POINTER(sk->sk_dst_cache, NULL);
dst_release(dst);
return NULL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 16e0249b11f6c..97bee820799ee 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1103,7 +1103,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
skb_set_hash_from_sk(skb, sk);
refcount_add(skb->truesize, &sk->sk_wmem_alloc);
- skb_set_dst_pending_confirm(skb, sk->sk_dst_pending_confirm);
+ skb_set_dst_pending_confirm(skb, READ_ONCE(sk->sk_dst_pending_confirm));
/* Build TCP header and checksum it. */
th = (struct tcphdr *)skb->data;
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 7/9] wifi: ath10k: Don't touch the CE interrupt registers after power up
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
` (4 preceding siblings ...)
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 6/9] net: annotate data-races around sk->sk_dst_pending_confirm Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 8/9] Bluetooth: Fix double free in hci_conn_cleanup Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 9/9] platform/x86: thinkpad_acpi: Add battery quirk for Thinkpad X120e Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Douglas Anderson, Kalle Valo, Sasha Levin, kvalo, quic_jjohnson,
ath10k, linux-wireless
From: Douglas Anderson <dianders@chromium.org>
[ Upstream commit 170c75d43a77dc937c58f07ecf847ba1b42ab74e ]
As talked about in commit d66d24ac300c ("ath10k: Keep track of which
interrupts fired, don't poll them"), if we access the copy engine
register at a bad time then ath10k can go boom. However, it's not
necessarily easy to know when it's safe to access them.
The ChromeOS test labs saw a crash that looked like this at
shutdown/reboot time (on a chromeos-5.15 kernel, but likely the
problem could also reproduce upstream):
Internal error: synchronous external abort: 96000010 [#1] PREEMPT SMP
...
CPU: 4 PID: 6168 Comm: reboot Not tainted 5.15.111-lockdep-19350-g1d624fe6758f #1 010b9b233ab055c27c6dc88efb0be2f4e9e86f51
Hardware name: Google Kingoftown (DT)
...
pc : ath10k_snoc_read32+0x50/0x74 [ath10k_snoc]
lr : ath10k_snoc_read32+0x24/0x74 [ath10k_snoc]
...
Call trace:
ath10k_snoc_read32+0x50/0x74 [ath10k_snoc ...]
ath10k_ce_disable_interrupt+0x190/0x65c [ath10k_core ...]
ath10k_ce_disable_interrupts+0x8c/0x120 [ath10k_core ...]
ath10k_snoc_hif_stop+0x78/0x660 [ath10k_snoc ...]
ath10k_core_stop+0x13c/0x1ec [ath10k_core ...]
ath10k_halt+0x398/0x5b0 [ath10k_core ...]
ath10k_stop+0xfc/0x1a8 [ath10k_core ...]
drv_stop+0x148/0x6b4 [mac80211 ...]
ieee80211_stop_device+0x70/0x80 [mac80211 ...]
ieee80211_do_stop+0x10d8/0x15b0 [mac80211 ...]
ieee80211_stop+0x144/0x1a0 [mac80211 ...]
__dev_close_many+0x1e8/0x2c0
dev_close_many+0x198/0x33c
dev_close+0x140/0x210
cfg80211_shutdown_all_interfaces+0xc8/0x1e0 [cfg80211 ...]
ieee80211_remove_interfaces+0x118/0x5c4 [mac80211 ...]
ieee80211_unregister_hw+0x64/0x1f4 [mac80211 ...]
ath10k_mac_unregister+0x4c/0xf0 [ath10k_core ...]
ath10k_core_unregister+0x80/0xb0 [ath10k_core ...]
ath10k_snoc_free_resources+0xb8/0x1ec [ath10k_snoc ...]
ath10k_snoc_shutdown+0x98/0xd0 [ath10k_snoc ...]
platform_shutdown+0x7c/0xa0
device_shutdown+0x3e0/0x58c
kernel_restart_prepare+0x68/0xa0
kernel_restart+0x28/0x7c
Though there's no known way to reproduce the problem, it makes sense
that it would be the same issue where we're trying to access copy
engine registers when it's not allowed.
Let's fix this by changing how we "disable" the interrupts. Instead of
tweaking the copy engine registers we'll just use disable_irq() and
enable_irq(). Then we'll configure the interrupts once at power up
time.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2.c10-00754-QCAHLSWMTPL-1
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://lore.kernel.org/r/20230630151842.1.If764ede23c4e09a43a842771c2ddf99608f25f8e@changeid
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/ath/ath10k/snoc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index b6762fe2efe26..29d52f7b4336d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -821,12 +821,20 @@ static void ath10k_snoc_hif_get_default_pipe(struct ath10k *ar,
static inline void ath10k_snoc_irq_disable(struct ath10k *ar)
{
- ath10k_ce_disable_interrupts(ar);
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int id;
+
+ for (id = 0; id < CE_COUNT_MAX; id++)
+ disable_irq(ar_snoc->ce_irqs[id].irq_line);
}
static inline void ath10k_snoc_irq_enable(struct ath10k *ar)
{
- ath10k_ce_enable_interrupts(ar);
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int id;
+
+ for (id = 0; id < CE_COUNT_MAX; id++)
+ enable_irq(ar_snoc->ce_irqs[id].irq_line);
}
static void ath10k_snoc_rx_pipe_cleanup(struct ath10k_snoc_pipe *snoc_pipe)
@@ -1042,6 +1050,8 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
goto err_free_rri;
}
+ ath10k_ce_enable_interrupts(ar);
+
return 0;
err_free_rri:
@@ -1196,8 +1206,8 @@ static int ath10k_snoc_request_irq(struct ath10k *ar)
for (id = 0; id < CE_COUNT_MAX; id++) {
ret = request_irq(ar_snoc->ce_irqs[id].irq_line,
- ath10k_snoc_per_engine_handler, 0,
- ce_name[id], ar);
+ ath10k_snoc_per_engine_handler,
+ IRQF_NO_AUTOEN, ce_name[id], ar);
if (ret) {
ath10k_err(ar,
"failed to register IRQ handler for CE %d: %d",
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 8/9] Bluetooth: Fix double free in hci_conn_cleanup
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
` (5 preceding siblings ...)
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 7/9] wifi: ath10k: Don't touch the CE interrupt registers after power up Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 9/9] platform/x86: thinkpad_acpi: Add battery quirk for Thinkpad X120e Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: ZhengHan Wang, Luiz Augusto von Dentz, Sasha Levin, marcel,
johan.hedberg, luiz.dentz, linux-bluetooth
From: ZhengHan Wang <wzhmmmmm@gmail.com>
[ Upstream commit a85fb91e3d728bdfc80833167e8162cce8bc7004 ]
syzbot reports a slab use-after-free in hci_conn_hash_flush [1].
After releasing an object using hci_conn_del_sysfs in the
hci_conn_cleanup function, releasing the same object again
using the hci_dev_put and hci_conn_put functions causes a double free.
Here's a simplified flow:
hci_conn_del_sysfs:
hci_dev_put
put_device
kobject_put
kref_put
kobject_release
kobject_cleanup
kfree_const
kfree(name)
hci_dev_put:
...
kfree(name)
hci_conn_put:
put_device
...
kfree(name)
This patch drop the hci_dev_put and hci_conn_put function
call in hci_conn_cleanup function, because the object is
freed in hci_conn_del_sysfs function.
This patch also fixes the refcounting in hci_conn_add_sysfs() and
hci_conn_del_sysfs() to take into account device_add() failures.
This fixes CVE-2023-28464.
Link: https://syzkaller.appspot.com/bug?id=1bb51491ca5df96a5f724899d1dbb87afda61419 [1]
Signed-off-by: ZhengHan Wang <wzhmmmmm@gmail.com>
Co-developed-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/bluetooth/hci_conn.c | 6 ++----
net/bluetooth/hci_sysfs.c | 23 ++++++++++++-----------
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index afdc0afa8ee7d..e129b7fb6540a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -125,13 +125,11 @@ static void hci_conn_cleanup(struct hci_conn *conn)
if (hdev->notify)
hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
- hci_conn_del_sysfs(conn);
-
debugfs_remove_recursive(conn->debugfs);
- hci_dev_put(hdev);
+ hci_conn_del_sysfs(conn);
- hci_conn_put(conn);
+ hci_dev_put(hdev);
}
static void le_scan_cleanup(struct work_struct *work)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index ccd2c377bf83c..266112c960ee8 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -33,7 +33,7 @@ void hci_conn_init_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
- BT_DBG("conn %p", conn);
+ bt_dev_dbg(hdev, "conn %p", conn);
conn->dev.type = &bt_link;
conn->dev.class = bt_class;
@@ -46,27 +46,30 @@ void hci_conn_add_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
- BT_DBG("conn %p", conn);
+ bt_dev_dbg(hdev, "conn %p", conn);
if (device_is_registered(&conn->dev))
return;
dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle);
- if (device_add(&conn->dev) < 0) {
+ if (device_add(&conn->dev) < 0)
bt_dev_err(hdev, "failed to register connection device");
- return;
- }
-
- hci_dev_hold(hdev);
}
void hci_conn_del_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
- if (!device_is_registered(&conn->dev))
+ bt_dev_dbg(hdev, "conn %p", conn);
+
+ if (!device_is_registered(&conn->dev)) {
+ /* If device_add() has *not* succeeded, use *only* put_device()
+ * to drop the reference count.
+ */
+ put_device(&conn->dev);
return;
+ }
while (1) {
struct device *dev;
@@ -78,9 +81,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn)
put_device(dev);
}
- device_del(&conn->dev);
-
- hci_dev_put(hdev);
+ device_unregister(&conn->dev);
}
static void bt_host_release(struct device *dev)
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 5.4 9/9] platform/x86: thinkpad_acpi: Add battery quirk for Thinkpad X120e
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
` (6 preceding siblings ...)
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 8/9] Bluetooth: Fix double free in hci_conn_cleanup Sasha Levin
@ 2023-11-07 12:12 ` Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2023-11-07 12:12 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Olli Asikainen, Ilpo Järvinen, Sasha Levin, hmh, hdegoede,
markgross, ibm-acpi-devel, platform-driver-x86
From: Olli Asikainen <olli.asikainen@gmail.com>
[ Upstream commit 916646758aea81a143ce89103910f715ed923346 ]
Thinkpad X120e also needs this battery quirk.
Signed-off-by: Olli Asikainen <olli.asikainen@gmail.com>
Link: https://lore.kernel.org/r/20231024190922.2742-1-olli.asikainen@gmail.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/platform/x86/thinkpad_acpi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 5d114088c88fb..f0d6bb567d1dc 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9699,6 +9699,7 @@ static const struct tpacpi_quirk battery_quirk_table[] __initconst = {
* Individual addressing is broken on models that expose the
* primary battery as BAT1.
*/
+ TPACPI_Q_LNV('8', 'F', true), /* Thinkpad X120e */
TPACPI_Q_LNV('J', '7', true), /* B5400 */
TPACPI_Q_LNV('J', 'I', true), /* Thinkpad 11e */
TPACPI_Q_LNV3('R', '0', 'B', true), /* Thinkpad 11e gen 3 */
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-11-07 12:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-07 12:12 [PATCH AUTOSEL 5.4 1/9] wifi: mac80211_hwsim: fix clang-specific fortify warning Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 2/9] wifi: mac80211: don't return unset power in ieee80211_get_tx_power() Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 3/9] wifi: ath9k: fix clang-specific fortify warnings Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 4/9] wifi: ath10k: fix clang-specific fortify warning Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 5/9] net: annotate data-races around sk->sk_tx_queue_mapping Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 6/9] net: annotate data-races around sk->sk_dst_pending_confirm Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 7/9] wifi: ath10k: Don't touch the CE interrupt registers after power up Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 8/9] Bluetooth: Fix double free in hci_conn_cleanup Sasha Levin
2023-11-07 12:12 ` [PATCH AUTOSEL 5.4 9/9] platform/x86: thinkpad_acpi: Add battery quirk for Thinkpad X120e Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox