Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CANn89iKWzfAeK96iL_snfJ1jGnaWq0B5odZtC5o1_ccB8ECV9w@mail.gmail.com>
 <20200527184830.GA1164846@mwanda>
In-Reply-To: <20200527184830.GA1164846@mwanda>
From:   Eric Dumazet <edumazet@google.com>
Date:   Thu, 28 May 2020 08:55:07 -0700
Message-ID: <CANn89iJBBvMgGo930cxvhmk_dAt1Jh-AzyiiZQ_reKwzcaO38g@mail.gmail.com>
Subject: Re: [PATCH v3] airo: Fix read overflows sending packets
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     Kalle Valo <kvalo@codeaurora.org>, Hu Jiahui <kirin.say@gmail.com>,
        Security Officers <security@kernel.org>,
        linux-wireless@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Wed, May 27, 2020 at 11:48 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The problem is that we always copy a minimum of ETH_ZLEN (60) bytes from
> skb->data even when skb->len is less than ETH_ZLEN so it leads to a read
> overflow.
>
> The fix is to pad skb->data to at least ETH_ZLEN bytes.
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Hu Jiahui <kirin.say@gmail.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: remove an unnecessary if statement
>     increment the ->tx_dropped count on failure
>     fix found two more instances of the same bug.
>     fix typo in the "Cc: <stable@vger.kernel.org>" tag
> v3: I had thought that skb_padto() updated skb->len so that it would
>     always be more than ETH_ZLEN meaning that we could delete the checks
>     for smaller values:  "len = skb->len < ETH_ZLEN ? ETH_ZLEN : skb->len;"
>     But I was wrong and those are still required.
>
>  drivers/net/wireless/cisco/airo.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>

Reviewed-by: Eric Dumazet <edumazet@google.com>