| Message ID | 20191020011153.29383-1-Larry.Finger@lwfinger.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [V2] rtlwifi: rtl_pci: Fix problem of too small skb->len | expand |
Larry Finger <Larry.Finger@lwfinger.net> writes: > In commit 8020919a9b99 ("mac80211: Properly handle SKB with radiotap > only"), buffers whose length is too short cause a WARN_ON(1) to be > executed. This change exposed a fault in rtlwifi drivers, which is fixed > by increasing the length of the affected buffer before it is sent to > mac80211. With what frames, or in what scenarios, do you get these warnings? > Cc: Stable <stable@vger.kernel.org> # v5.0+ > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> > --- > V2 - added missing usage of new len > --- > Please Apply to 5.4 > --- > drivers/net/wireless/realtek/rtlwifi/pci.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c > index 6087ec7a90a6..3e9185162e51 100644 > --- a/drivers/net/wireless/realtek/rtlwifi/pci.c > +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c > @@ -692,12 +692,15 @@ static void _rtl_pci_rx_to_mac80211(struct ieee80211_hw *hw, > dev_kfree_skb_any(skb); > } else { > struct sk_buff *uskb = NULL; > + int len = skb->len; > > + if (unlikely(len <= FCS_LEN)) > + len = FCS_LEN + 2; I don't understand this change, I think this needs a comment in the code, or better yet a proper define documenting the meaning of the value. What does these two bytes contain? Or are you just working around the mac80211 warning by increasing the length with a random value you chose?
On 10/20/19 3:28 AM, Kalle Valo wrote: > Larry Finger <Larry.Finger@lwfinger.net> writes: > >> In commit 8020919a9b99 ("mac80211: Properly handle SKB with radiotap >> only"), buffers whose length is too short cause a WARN_ON(1) to be >> executed. This change exposed a fault in rtlwifi drivers, which is fixed >> by increasing the length of the affected buffer before it is sent to >> mac80211. > > With what frames, or in what scenarios, do you get these warnings? I am not sure how they happen, but the firmware reports a 3-byte packet, which leads to the warning. After looking at the code path again, a better approach would be to consider those short packets the same way that those with CRC or hardware errors and drop them. After more testing, I will send V3 using that approach. Larry
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c index 6087ec7a90a6..3e9185162e51 100644 --- a/drivers/net/wireless/realtek/rtlwifi/pci.c +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c @@ -692,12 +692,15 @@ static void _rtl_pci_rx_to_mac80211(struct ieee80211_hw *hw, dev_kfree_skb_any(skb); } else { struct sk_buff *uskb = NULL; + int len = skb->len; + if (unlikely(len <= FCS_LEN)) + len = FCS_LEN + 2; uskb = dev_alloc_skb(skb->len + 128); if (likely(uskb)) { memcpy(IEEE80211_SKB_RXCB(uskb), &rx_status, sizeof(rx_status)); - skb_put_data(uskb, skb->data, skb->len); + skb_put_data(uskb, skb->data, len); dev_kfree_skb_any(skb); ieee80211_rx_irqsafe(hw, uskb); } else {
In commit 8020919a9b99 ("mac80211: Properly handle SKB with radiotap only"), buffers whose length is too short cause a WARN_ON(1) to be executed. This change exposed a fault in rtlwifi drivers, which is fixed by increasing the length of the affected buffer before it is sent to mac80211. Cc: Stable <stable@vger.kernel.org> # v5.0+ Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> --- V2 - added missing usage of new len --- Please Apply to 5.4 --- drivers/net/wireless/realtek/rtlwifi/pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)