netdev - Re: [PATCH 1/2] airo: Fix possible info leak in AIROOLDIOCTL/SIOCDEVPRIVATE

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]

Message-Id: <20200123.110334.575852757631951738.davem@davemloft.net>
Date:   Thu, 23 Jan 2020 11:03:34 +0100 (CET)
From:   David Miller <davem@...emloft.net>
To:     mpe@...erman.id.au
Cc:     netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
        kvalo@...eaurora.org, linux-kernel@...r.kernel.org,
        security@...nel.org, ivansprundel@...ctive.com
Subject: Re: [PATCH 1/2] airo: Fix possible info leak in
 AIROOLDIOCTL/SIOCDEVPRIVATE

From: Michael Ellerman <mpe@...erman.id.au>
Date: Wed, 22 Jan 2020 15:07:27 +1100

> The driver for Cisco Aironet 4500 and 4800 series cards (airo.c),
> implements AIROOLDIOCTL/SIOCDEVPRIVATE in airo_ioctl().
> 
> The ioctl handler copies an aironet_ioctl struct from userspace, which
> includes a command and a length. Some of the commands are handled in
> readrids(), which kmalloc()'s a buffer of RIDSIZE (2048) bytes.
> 
> That buffer is then passed to PC4500_readrid(), which has two cases.
> The else case does some setup and then reads up to RIDSIZE bytes from
> the hardware into the kmalloc()'ed buffer.
> 
> Here len == RIDSIZE, pBuf is the kmalloc()'ed buffer:
> 
> 	// read the rid length field
> 	bap_read(ai, pBuf, 2, BAP1);
> 	// length for remaining part of rid
> 	len = min(len, (int)le16_to_cpu(*(__le16*)pBuf)) - 2;
> 	...
> 	// read remainder of the rid
> 	rc = bap_read(ai, ((__le16*)pBuf)+1, len, BAP1);
> 
> PC4500_readrid() then returns to readrids() which does:
> 
> 	len = comp->len;
> 	if (copy_to_user(comp->data, iobuf, min(len, (int)RIDSIZE))) {
> 
> Where comp->len is the user controlled length field.
> 
> So if the "rid length field" returned by the hardware is < 2048, and
> the user requests 2048 bytes in comp->len, we will leak the previous
> contents of the kmalloc()'ed buffer to userspace.
> 
> Fix it by kzalloc()'ing the buffer.
> 
> Found by Ilja by code inspection, not tested as I don't have the
> required hardware.
> 
> Reported-by: Ilja Van Sprundel <ivansprundel@...ctive.com>
> Signed-off-by: Michael Ellerman <mpe@...erman.id.au>

Applied and queued up for -stable.