Page MenuHomeFreeBSD
Differential D11786

uath: fix varible types, add checks for descriptor / command header structure fields.
ClosedPublic
Actions

Authored by avos on Jul 30 2017, 10:49 PM.
Tags
None
Referenced Files
F103287453: D11786.diff
Sat, Nov 23, 2:29 AM
Unknown Object (File)
Fri, Nov 15, 7:10 AM
Unknown Object (File)
Wed, Nov 13, 2:23 PM
Unknown Object (File)
Fri, Nov 8, 1:37 AM
Unknown Object (File)
Thu, Oct 31, 3:42 PM
Unknown Object (File)
Wed, Oct 30, 6:17 PM
Unknown Object (File)
Sep 30 2024, 3:54 AM
Unknown Object (File)
Sep 13 2024, 2:59 PM
View All 48 Files
Subscribers
imp

Details

Reviewers
adrian
• hselasky
Commits
rS324144: uath(4): fix varible types, add missing checks for descriptor / command
Summary
  • Use 'uint32_t' to save be32toh(9) result.
  • Drop too short payload (to prevent 'chunklen' underflow).
  • Recheck Rx descriptor fields to prevent buffer over-read.
Test Plan

Untested.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

avos created this revision.Jul 30 2017, 10:49 PM
Herald added a subscriber: imp. · View Herald TranscriptJul 30 2017, 10:49 PM
• hselasky added inline comments.Jul 31 2017, 7:07 AM
sys/dev/usb/wlan/if_uath.c
2226

If hdr->len comes from HW, please verify that value is valid!

2231–2232

Missing upper range check?

2526–2527

chunklen should also be checked against actlen ???

2599–2612

Are the offsets and payloads within the USB buffer and below actlen ?

2640–2666

Missing upper and lower checks for desc->framelen !

avos updated this revision to Diff 31683.Aug 7 2017, 12:04 AM
avos retitled this revision from uath: fix type, add sanity check to uath: fix varible types, add checks for descriptor / command header structure fields..
avos edited the summary of this revision. (Show Details)

Address some possible buffer over-reads.

avos marked 3 inline comments as done.Aug 13 2017, 7:16 PM
avos added inline comments.
sys/dev/usb/wlan/if_uath.c
2226

Done in uath_intr_rx_callback()

avos updated this revision to Diff 32014.Aug 13 2017, 7:51 PM
avos marked an inline comment as done.
  • Add more 'framelen' variable checks.
  • Check hdr->len before using it to calculate dlen.
avos marked 3 inline comments as done.Aug 13 2017, 7:53 PM
• hselasky edited edge metadata.Aug 13 2017, 8:07 PM

I'll have a closer look at this tomorrow. Thank you!

• hselasky added inline comments.Aug 14 2017, 12:29 PM
sys/dev/usb/wlan/if_uath.c
2243–2244

Can you invert this code? It makes it more clear:

!(a && b) == ((!a) || (!b))

if (hdr->len < sizeof(*hdr) || hdr->len >= UATH_MAX_CMDSZ)

2362

if (hdr->len > (uint32_t)actlen)

2654

Add this check first? Or is it redundant?
actlen < sizeof(struct uath_chunk) ||

cast: actlen - sizeof(struct uath_chunk) to uint32_t:

framelen > (uint32_t)(actlen - sizeof(struct uath_chunk))

avos updated this revision to Diff 32264.Aug 20 2017, 10:38 AM
avos marked 3 inline comments as done.Aug 20 2017, 10:43 AM
avos added inline comments.
sys/dev/usb/wlan/if_uath.c
2654

Already checked at the top of the function ('if (actlen < (int)UATH_MIN_RXBUFSZ)')

avos marked 2 inline comments as done.Aug 20 2017, 10:44 AM
• hselasky accepted this revision.Aug 20 2017, 7:09 PM

Looks good.

This revision is now accepted and ready to land.Aug 20 2017, 7:09 PM
Closed by commit rS324144: uath(4): fix varible types, add missing checks for descriptor / command (authored by avos). · Explain WhySep 30 2017, 9:01 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
dev/
usb/
wlan/
122 lines

Diff 32264

View Options

sys/dev/usb/wlan/if_uath.c

Loading...