Hi Bryan, On 1/17/2025 3:55 PM, Bryan O'Donoghue wrote: > On 17/01/2025 08:39, Vedang Nagar wrote: >> Below is the first read where dwords is being validated properly with the checks. >> dwords = *rd_ptr >> 2; >> >> Whereas the same address is being read for the second time: >> memcpy(pkt, rd_ptr, dwords << 2); >> >> For the second read the value is not validated which may get updated from the firmware >> leading to incorrect memcpy into the packet and may lead to OOB read access while accessing >> the packet. > > So you are saying that pkt points to memory that the firmware and host can simultaneously access. > > The question is - if the length value can change between one read and another read - how do you trust the _content_ of the packet ? Original content of the packet is validated while reading the packet in hfi_process_msg_packet function. Whereas the current change is just to validate the size of the packet to avoid the Out of bound read access. > > Surely the right thing to do is to take a _copy_ of the entire frame and act on that frame exclusively on the host side ? > > If I receive a frame, and read length X. > > Then I need to re-read that frame because length may now by X+3. > > This implies the _data_ in the frame has changed. Yes, the _data_ in the frame has changed and will get rejected while parsing that data. So I think it's okay to no read or copy the entire frame again. > > What exactly is the valid lifetime of this data from HFI RX interrupt ? There is no as such lifetime of the interrupt, but any rogue firmware can corrupt the data in the packet. Regards, Vedang Nagar > > --- > bod
