linux-kernel - [PATCH] media: cxd2880-spi: avoid out-of-bounds access warning

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-next>] [day] [month] [year] [list]

Date:   Tue, 13 Mar 2018 13:09:11 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Yasunari Takiguchi <Yasunari.Takiguchi@...y.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Martin Sebor <msebor@...il.com>,
        Toshihiko Matsumoto <Toshihiko.Matsumoto@...y.com>,
        Kota Yonezawa <Kota.Yonezawa@...y.com>,
        Satoshi Watanabe <Satoshi.C.Watanabe@...y.com>,
        Masayuki Yamamoto <Masayuki.Yamamoto@...y.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] media: cxd2880-spi: avoid out-of-bounds access warning

The -Warray-bounds warning in gcc-8 triggers for a newly added file:

drivers/media/spi/cxd2880-spi.c: In function 'cxd2880_write_reg':
drivers/media/spi/cxd2880-spi.c:111:3: error: 'memcpy' forming offset [133, 258] is out of the bounds [0, 132] of object 'send_data' with type 'u8[132]' {aka 'unsigned char[132]'} [-Werror=array-bounds]

The problem appears to be that we have two range checks in this function,
first comparing against BURST_WRITE_MAX (128) and then comparing against
a literal '255'. The logic checking the buffer size looks at the second
one and decides that this might be the actual maximum data length.

This is understandable behavior from the compiler, but the code is actually
safe. Since the first check is already shorter, we can remove the loop
and only leave that. To be on the safe side in case BURST_WRITE_MAX might
be increased, I'm leaving the check against U8_MAX.

Fixes: bd24fcddf6b8 ("media: cxd2880-spi: Add support for CXD2880 SPI interface")
Cc: Martin Sebor <msebor@...il.com>
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 drivers/media/spi/cxd2880-spi.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index 4df3bd312f48..011a37c2f272 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -88,7 +88,7 @@ static int cxd2880_write_reg(struct spi_device *spi,
 		pr_err("invalid arg\n");
 		return -EINVAL;
 	}
-	if (size > BURST_WRITE_MAX) {
+	if (size > BURST_WRITE_MAX || size > U8_MAX) {
 		pr_err("data size > WRITE_MAX\n");
 		return -EINVAL;
 	}
@@ -101,24 +101,14 @@ static int cxd2880_write_reg(struct spi_device *spi,
 	send_data[0] = 0x0e;
 	write_data_top = data;
 
-	while (size > 0) {
-		send_data[1] = sub_address;
-		if (size > 255)
-			send_data[2] = 255;
-		else
-			send_data[2] = (u8)size;
+	send_data[1] = sub_address;
+	send_data[2] = (u8)size;
 
-		memcpy(&send_data[3], write_data_top, send_data[2]);
+	memcpy(&send_data[3], write_data_top, send_data[2]);
 
-		ret = cxd2880_write_spi(spi, send_data, send_data[2] + 3);
-		if (ret) {
-			pr_err("write spi failed %d\n", ret);
-			break;
-		}
-		sub_address += send_data[2];
-		write_data_top += send_data[2];
-		size -= send_data[2];
-	}
+	ret = cxd2880_write_spi(spi, send_data, send_data[2] + 3);
+	if (ret)
+		pr_err("write spi failed %d\n", ret);
 
 	return ret;
 }
-- 
2.9.0