[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <753bc16161f9f1024d33b93d6aa984c0b6adf51d.camel@sipsolutions.net>
Date: Fri, 18 Jul 2025 20:03:16 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Eric Biggers <ebiggers@...nel.org>, linux-wireless@...r.kernel.org
Cc: syzbot <syzbot+d1008c24929007591b6b@...kaller.appspotmail.com>,
ardb@...nel.org, bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com,
linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
mingo@...hat.com, netdev@...r.kernel.org, syzkaller-bugs@...glegroups.com,
tglx@...utronix.de, x86@...nel.org
Subject: Re: [syzbot] [wireless] BUG: unable to handle kernel paging request
in ieee80211_wep_encrypt
On Fri, 2025-07-18 at 17:57 +0200, Johannes Berg wrote:
> On Fri, 2025-07-18 at 07:50 -0700, Eric Biggers wrote:
> > >
> > > BUG: unable to handle page fault for address: ffff8880bfffd000
> [...]
> > > Call Trace:
> > > <TASK>
> > > crc32_le_arch+0x56/0xa0 arch/x86/lib/crc32.c:21
> > > crc32_le include/linux/crc32.h:18 [inline]
> > > ieee80211_wep_encrypt_data net/mac80211/wep.c:114 [inline]
> > > ieee80211_wep_encrypt+0x228/0x410 net/mac80211/wep.c:158
> [...]
> > > nl80211_tx_mgmt+0x9fd/0xd50 net/wireless/nl80211.c:12921
> >
> > syzbot assigned this to the "crypto" subsystem. However, the crash
> > happened in crc32_le() which is not part of the crypto subsystem. Also,
> > crc32_le() is well-tested (e.g. by crc_kunit), and the bug is unlikely
> > to be there. Rather, the calling code in ieee80211_wep_encrypt_data()
> > is passing an invalid data buffer to crc32_le(). So let's do:
>
> Agree, that makes sense, looks like we never check the frame length
> correctly. Since there's no reproducer (yet) I guess we won't be testing
> against it with syzbot though :)
Well, hmm. So we're in
int ieee80211_wep_encrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
size_t klen, u8 *data, size_t data_len)
{
__le32 icv;
icv = cpu_to_le32(~crc32_le(~0, data, data_len));
with presumably data/data_len being garbage. Via
int ieee80211_wep_encrypt(struct ieee80211_local *local,
struct sk_buff *skb,
const u8 *key, int keylen, int keyidx)
{
u8 *iv;
size_t len;
u8 rc4key[3 + WLAN_KEY_LEN_WEP104];
if (WARN_ON(skb_tailroom(skb) < IEEE80211_WEP_ICV_LEN))
return -1;
iv = ieee80211_wep_add_iv(local, skb, keylen, keyidx);
if (!iv)
return -1;
len = skb->len - (iv + IEEE80211_WEP_IV_LEN - skb->data);
which _looks_ OK at first, however, looking at
static u8 *ieee80211_wep_add_iv(struct ieee80211_local *local,
struct sk_buff *skb,
int keylen, int keyidx)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
unsigned int hdrlen;
u8 *newhdr;
hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PROTECTED);
if (WARN_ON(skb_headroom(skb) < IEEE80211_WEP_IV_LEN))
return NULL;
hdrlen = ieee80211_hdrlen(hdr->frame_control);
newhdr = skb_push(skb, IEEE80211_WEP_IV_LEN);
memmove(newhdr, newhdr + IEEE80211_WEP_IV_LEN, hdrlen);
/* the HW only needs room for the IV, but not the actual IV */
if (info->control.hw_key &&
(info->control.hw_key->flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE))
return newhdr + hdrlen;
ieee80211_wep_get_iv(local, keylen, keyidx, newhdr + hdrlen);
return newhdr + hdrlen;
}
there's no check that the skb->len is actually >= hdrlen(), in which
case we would return an 'iv' that's outside of [skb->data..+len].
Then the
len = skb->len - (iv + IEEE80211_WEP_IV_LEN - skb->data);
subtraction could underflow and result in this issue, I guess.
But the stack dump is strange in that we appear to get here via
nl80211_tx_mgmt() which only accepts management frames, but
ieee80211_tx_h_select_key() at least for WEP will NULL out the key for
!data frames, and then we can't get into the encrypt functions. Data
frames are always built by the kernel itself, so shouldn't get into some
kind of weird "header is shorter than the frame" situation.
It's theoretically possible that this is a _different_ frame being
dequeued than was just enqueued, but that seems like quite a stretch
since we just immediately dequeue after the enqueue with the hwsim
implementation ... and I'm not sure where that frame would come from
anyway.
So right now I have no idea what's going on here, nothing seems right.
johannes
Powered by blists - more mailing lists