Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 26 Apr 2022 18:00:38 +0200 | Subject | Re: [EXT] Re: [PATCH 0/5] net: atlantic: more fuzzing fixes | From | Igor Russkikh <> |
| |
Hi Grant,
Sorry for the delay, I was on vacation. Thanks for working on this.
I'm adding here Dmitrii, to help me review the patches. Dmitrii, here is a full series:
https://patchwork.kernel.org/project/netdevbpf/cover/20220418231746.2464800-1-grundler@chromium.org/
Grant, I've reviewed and also quite OK with patches 1-4.
For patch 5 - why do you think we need these extra comparisons with software head/tail? From what I see in logic, only the size limiting check is enough there..
Other extra checks are tricky and non intuitive..
Regards, Igor
On 4/21/2022 9:53 PM, Grant Grundler wrote: > External Email > > ---------------------------------------------------------------------- > Igor, > Will you have a chance to comment on this in the near future? > Should someone else review/integrate these patches? > > I'm asking since I've seen no comments in the past three days. > > cheers, > grant > > > On Mon, Apr 18, 2022 at 4:17 PM Grant Grundler <grundler@chromium.org> > wrote: >> >> The Chrome OS fuzzing team posted a "Fuzzing" report for atlantic driver >> in Q4 2021 using Chrome OS v5.4 kernel and "Cable Matters >> Thunderbolt 3 to 10 Gb Ethernet" (b0 version): >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_document_d_e_2PACX-2D1vT4oCGNhhy-5FAuUqpu6NGnW0N9HF-5Fjxf2kS7raOpOlNRqJNiTHAtjiHRthXYSeXIRTgfeVvsEt0qK9qK_pub&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=3kUjVPjrPMvlbd3rzgP63W0eewvCq4D-kzQRqaXHOqU&m=QoxR8WoQQ-hpWu_tThQydP3-6zkRWACvRmj_7aY1qo2FG6DdPdI86vAYrfKQFMHX&s=620jqeSvQrGg6aotI35cWwQpjaL94s7TFeFh2cYSyvA&e= >> >> It essentially describes four problems: >> 1) validate rxd_wb->next_desc_ptr before populating buff->next >> 2) "frag[0] not initialized" case in aq_ring_rx_clean() >> 3) limit iterations handling fragments in aq_ring_rx_clean() >> 4) validate hw_head_ in hw_atl_b0_hw_ring_tx_head_update() >> >> I've added one "clean up" contribution: >> "net: atlantic: reduce scope of is_rsc_complete" >> >> I tested the "original" patches using chromeos-v5.4 kernel branch: >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__chromium-2Dreview.googlesource.com_q_hashtag-3Apcinet-2Datlantic-2D2022q1-2B-28status-3Aopen-2520OR-2520status-3Amerged-29&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=3kUjVPjrPMvlbd3rzgP63W0eewvCq4D-kzQRqaXHOqU&m=QoxR8WoQQ-hpWu_tThQydP3-6zkRWACvRmj_7aY1qo2FG6DdPdI86vAYrfKQFMHX&s=1a1YwJqrY-be2oDgGAG5oOyZDnqIok_2p5G-N8djo2I&e= >> >> The fuzzing team will retest using the chromeos-v5.4 patches and the b0 >> HW. >> >> I've forward ported those patches to 5.18-rc2 and compiled them but am >> currently unable to test them on 5.18-rc2 kernel (logistics problems). >> >> I'm confident in all but the last patch: >> "net: atlantic: verify hw_head_ is reasonable" >> >> Please verify I'm not confusing how ring->sw_head and ring->sw_tail >> are used in hw_atl_b0_hw_ring_tx_head_update(). >> >> Credit largely goes to Chrome OS Fuzzing team members: >> Aashay Shringarpure, Yi Chou, Shervin Oloumi >> >> cheers, >> grant
| ![\](/images/icornerr.gif) |