iwlwifi: Null deref in iwl_mld_reorder

iwlwifi: Null deref in iwl_mld_reorder

[Ben Greear]

Hello,

While chasing what appears to be a different problem, I started checking for
NULL in iwl_mld_sta_from_mac80211.  That showed a problem in the code below,
where mld_sta is assigned before sta is checked for null.

I guess the compiler optimized this somehow so that crashes are not actually
seen in this particular code path.  Also possible that it is somehow exacerbated
by some local patch in our tree.

But, probably best to assign mld_sta after the NULL sta check.


/* Returns true if the MPDU was buffered\dropped, false if it should be passed
  * to upper layer.
  */
enum iwl_mld_reorder_result
iwl_mld_reorder(struct iwl_mld *mld, struct napi_struct *napi,
                 int queue, struct ieee80211_sta *sta,
                 struct sk_buff *skb, struct iwl_rx_mpdu_desc *desc)
{
         struct ieee80211_hdr *hdr = (void *)skb_mac_header(skb);
         struct iwl_mld_baid_data *baid_data;
         struct iwl_mld_reorder_buffer *buffer;
         struct iwl_mld_reorder_buf_entry *entries;
         struct iwl_mld_sta *mld_sta = iwl_mld_sta_from_mac80211(sta);

assignment before checking NULL sta. ^^^

         struct iwl_mld_link_sta *mld_link_sta;
         u32 reorder = le32_to_cpu(desc->reorder_data);
         bool amsdu, last_subframe, is_old_sn, is_dup;
         u8 tid = ieee80211_get_tid(hdr);
         u8 baid;
         u16 nssn, sn;
         u32 sta_mask = 0;
         int index;
         u8 link_id;

         baid = u32_get_bits(reorder, IWL_RX_MPDU_REORDER_BAID_MASK);

         /* This also covers the case of receiving a Block Ack Request
          * outside a BA session; we'll pass it to mac80211 and that
          * then sends a delBA action frame.
          * This also covers pure monitor mode, in which case we won't
          * have any BA sessions.
          */
         if (baid == IWL_RX_REORDER_DATA_INVALID_BAID)
                 return IWL_MLD_PASS_SKB;

         /* no sta yet */
         if (WARN_ONCE(!sta,
                       "Got valid BAID without a valid station assigned\n"))
                 return IWL_MLD_PASS_SKB;

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: iwlwifi: Null deref in iwl_mld_reorder

[Johannes Berg]

On Wed, 2026-02-04 at 05:41 -0800, Ben Greear wrote:
> 
> I guess the compiler optimized this somehow so that crashes are not actually
> seen in this particular code path.  Also possible that it is somehow exacerbated
> by some local patch in our tree.
> 
> But, probably best to assign mld_sta after the NULL sta check.

Admittedly not great, but

>          struct iwl_mld_sta *mld_sta = iwl_mld_sta_from_mac80211(sta);

That's just a pointer calculation, ie. mld_sta = sta + offset, so
there's no actual dereference.

johannes

Re: iwlwifi: Null deref in iwl_mld_reorder

[Ben Greear]

On 2/4/26 07:32, Johannes Berg wrote:
> On Wed, 2026-02-04 at 05:41 -0800, Ben Greear wrote:
>>
>> I guess the compiler optimized this somehow so that crashes are not actually
>> seen in this particular code path.  Also possible that it is somehow exacerbated
>> by some local patch in our tree.
>>
>> But, probably best to assign mld_sta after the NULL sta check.
> 
> Admittedly not great, but
> 
>>           struct iwl_mld_sta *mld_sta = iwl_mld_sta_from_mac80211(sta);
> 
> That's just a pointer calculation, ie. mld_sta = sta + offset, so
> there's no actual dereference.

Ok, thanks for reviewing.

--Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com