Re: [EXT] [PATCH] [net][atlantic] Fix buff_ring OOB in aq_ring_rx_clean

From: Igor Russkikh
Date: Mon Jul 12 2021 - 12:33:50 EST


Hi Zekun,

Thanks for looking into that.

> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index 24122ccda614..f915b4885831 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -365,6 +365,10 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
> if (!buff->is_eop) {
> buff_ = buff;
> do {
> + if (buff_->next >= self->size) {
> + err = -EIO;
> + goto err_exit;
> + }
> next_ = buff_->next,
> buff_ = &self->buff_ring[next_];
> is_rsc_completed =
>

>From code analysis, the only way how ->next could be overflowed - is a
hardware malfunction/data corruption.

Software driver logic can't lead to that field overflow.
I'm not sure how fuzzing can lead to that result.. Do you have any details?

Even if it can, then we should also do a similar check in `if (buff->is_eop)` case below,
since it also uses similar sequence to run through `next` pointers.

Thanks,
Igor