Tony Nguyen <[email protected]> 于2025年11月18日周二 07:24写道:
>
>
> On 11/4/2025 12:28 AM, Guangshuo Li wrote:
> > In e1000_tbi_should_accept() we read the last byte of the frame via
> > 'data[length - 1]' to evaluate the TBI workaround. If the descriptor-
> > reported length is zero or larger than the actual RX buffer size, this
> > read goes out of bounds and can hit unrelated slab objects. The issue
> > is observed from the NAPI receive path (e1000_clean_rx_irq):
>
> ...
>
> > --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> > +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> > @@ -4090,6 +4090,12 @@ static bool e1000_tbi_should_accept(struct
> e1000_adapter *adapter,
> > u8 status, u8 errors,
> > u32 length, const u8 *data)
> > {
> > + /* Guard against OOB on data[length - 1] */
> > + if (unlikely(!length))
> > + return false;
> > + /* Upper bound: length must not exceed rx_buffer_len */
> > + if (unlikely(length > adapter->rx_buffer_len))
> > + return false;
>
> The change itself looks fine, however, the declarations should be at the
> beginning of the function so this should be moved to be after that.
>
> > struct e1000_hw *hw = &adapter->hw;
> > u8 last_byte = *(data + length - 1);
>
> Also, since last_byte uses length, this should be broken up and the
> assignment moved after the added checks.
>
> Thanks,
> Tony
>
Hi Tony, all,
>
Thanks for the review. As suggested by Tony, I’ll keep the declarations at
> the top and place the bounds checks before assigning last_byte. I’ll send
> a v2 with the following change:
> static bool e1000_tbi_should_accept(struct e1000_adapter *adapter,
> u8 status, u8 errors,
> u32 length, const u8 *data)
> {
> struct e1000_hw *hw = &adapter->hw;
> u8 last_byte;
>
> /* Guard against OOB on data[length - 1] */
> if (unlikely(!length))
> return false;
>
> /* Upper bound: length must not exceed rx_buffer_len */
> if (unlikely(length > adapter->rx_buffer_len))
> return false;
>
> last_byte = data[length - 1];
>
> /* existing logic follows ... */
> }
Please let me know if further adjustments are preferred.
> Best regards,
Guangshuo Li