mac80211_hwsim: correctly handle zero length frames

Started by Anirudh Rayabharam on Jun 10, 2021 · 3 messages from 2 authors · Last reply Jun 23, 2021

Subsystems (2)

Maintainers (2)

STALE1965d REVIEWED · 1 review, 0 by maintainers

Status: Reviewed.

1 review trailer.

open the first message on its own page

Message details and related content
syzbot, using KMSAN, has reported an uninit-value access in
hwsim_cloned_frame_received_nl(). This is happening because frame_data_len
is 0. The code doesn't detect this case and blindly tries to read the
frame's header.

Fix this by bailing out in case frame_data_len is 0.

Reported-by: <redacted>
Tested-by: <redacted>
Signed-off-by: Anirudh Rayabharam <redacted>
---
 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 51ce767eaf88..ccfe40313109 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3649,7 +3649,7 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	if (skb == NULL)
 		goto err;
 
-	if (frame_data_len > IEEE80211_MAX_DATA_LEN)
+	if (frame_data_len == 0 || frame_data_len > IEEE80211_MAX_DATA_LEN)
 		goto err;
 
 	/* Copy the data */
-- 
2.26.2
Message details and related content
On Thu, 2021-06-10 at 21:49 +0530, Anirudh Rayabharam wrote:
syzbot, using KMSAN, has reported an uninit-value access in
hwsim_cloned_frame_received_nl(). This is happening because frame_data_len
is 0. The code doesn't detect this case and blindly tries to read the
frame's header.

Fix this by bailing out in case frame_data_len is 0.
This really seems quite pointless - you should bail out if the frame is
too short for what we need to do, not just when it's 0.

johannes
Message details and related content
On Fri, Jun 18, 2021 at 11:36:16AM +0200, Johannes Berg wrote:
On Thu, 2021-06-10 at 21:49 +0530, Anirudh Rayabharam wrote:
quoted
syzbot, using KMSAN, has reported an uninit-value access in
hwsim_cloned_frame_received_nl(). This is happening because frame_data_len
is 0. The code doesn't detect this case and blindly tries to read the
frame's header.

Fix this by bailing out in case frame_data_len is 0.
This really seems quite pointless - you should bail out if the frame is
too short for what we need to do, not just when it's 0.
That makes sense. Do you happen to know what the min length of a valid
frame is? There doesn't seem to be constant defined for that already.

	- Anirudh
johannes
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help