[PATCH resend] mfd: mt6370: add bounds checking to regmap_read/write functions

From: Dan Carpenter <dan.carpenter@oracle.com>
To: Lee Jones <lee@kernel.org>, ChiYuan Huang <cy_huang@richtek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	ChiaEn Wu <chiaen_wu@richtek.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	kernel-janitors@vger.kernel.org
Subject: [PATCH resend] mfd: mt6370: add bounds checking to regmap_read/write functions
Date: Mon, 24 Oct 2022 15:18:17 +0300	[thread overview]
Message-ID: <Y1aCiReTZDbPp/rS@kadam> (raw)

It looks like there are a potential out of bounds accesses in the
read/write() functions.  Also can "len" be negative?  Let's check for
that too.

Fixes: ab9905c5e38e ("mfd: mt6370: Add MediaTek MT6370 support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This came from static analysis, but then I couldn't figure out the next
day if it was actually required or not so we dropped it.  But then
ChiYuan Huang did some testing and it was required.

There was some debate around various style questions but honestly I'm
pretty happy with the style the way I wrote it.  I've written a long
blog on why "unsigned int" is good at the user space boundary but should
not be the default choice as a stack variable.

https://staticthinking.wordpress.com/2022/06/01/unsigned-int-i-is-stupid/

The other style issue was wether to use ARRAY_SIZE() or MT6370_MAX_I2C
and I just think ARRAY_SIZE() is more obvious to the reader.

 drivers/mfd/mt6370.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mfd/mt6370.c b/drivers/mfd/mt6370.c
index cf19cce2fdc0..fd5e1d6a0272 100644
--- a/drivers/mfd/mt6370.c
+++ b/drivers/mfd/mt6370.c
@@ -190,6 +190,9 @@ static int mt6370_regmap_read(void *context, const void *reg_buf,
 	bank_idx = u8_buf[0];
 	bank_addr = u8_buf[1];
 
+	if (bank_idx >= ARRAY_SIZE(info->i2c))
+		return -EINVAL;
+
 	ret = i2c_smbus_read_i2c_block_data(info->i2c[bank_idx], bank_addr,
 					    val_size, val_buf);
 	if (ret < 0)
@@ -211,6 +214,9 @@ static int mt6370_regmap_write(void *context, const void *data, size_t count)
 	bank_idx = u8_buf[0];
 	bank_addr = u8_buf[1];
 
+	if (len < 0 || bank_idx >= ARRAY_SIZE(info->i2c))
+		return -EINVAL;
+
 	return i2c_smbus_write_i2c_block_data(info->i2c[bank_idx], bank_addr,
 					      len, data + MT6370_MAX_ADDRLEN);
 }
-- 
2.35.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
