netdev - Re: [EXT] [PATCH] [net][atlantic] Fix buff_ring OOB in aq_ring_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Jul 2021 18:33:37 +0200
From:   Igor Russkikh <irusskikh@...vell.com>
To:     Zekun Shen <bruceshenzk@...il.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [EXT] [PATCH] [net][atlantic] Fix buff_ring OOB in
 aq_ring_rx_clean

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