linux-kernel - [PATCH] Initialization of read buffer for dib3000_read

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]

Message-ID: <0100018779eb40dc-cee9e39d-5d87-4733-83db-eca5218fcc8f-000000@email.amazonses.com>
Date:   Thu, 13 Apr 2023 09:21:13 +0000
From:   Kernel-Development <kdev@...benng.net>
To:     mchehab@...nel.org <mchehab@...nel.org>
Cc:     linux-media@...r.kernel.org 
        <linux-media@...r.kernel.org>,
        linux-kernel@...r.kernel.org 
        <linux-kernel@...r.kernel.org>,
        skhan@...uxfoundation.org 
        <skhan@...uxfoundation.org>,
        linux-kernel-mentees@...ts.linuxfoundation.org 
        <linux-kernel-mentees@...ts.linuxfoundation.org>,
        syzbot+c88fc0ebe0d5935c70da@...kaller.appspotmail.com 
        <syzbot+c88fc0ebe0d5935c70da@...kaller.appspotmail.com>,
        Kernel-Development <kdev@...benng.net>
Subject: [PATCH] Initialization of read buffer for dib3000_read_reg

This is a patch that fixes a bug:
KMSAN: uninit-value in dib3000mb_attach (2)

Local variable u8 rb[2] is not initialized as it is used as read buffer
for i2c_transfer(). It is expected that i2c_transfer() should fill in
the buffer before the target function returns rb's content. However
error handling of i2c_transfer is not done, and on occasions where the
read fails, uninitialized rb value will be returned.

The usage of this function, defined as macro rd() in
drivers/media/dvb-frontends/dib3000mb_priv,h, does not expect any error
to occur. Adding error handling here might involve significant code
changes.

Thus 0-initialization is done on rb. This might affect some logic on
error case as the use of the return value is used as boolean and flags.

Reported-by: syzbot+c88fc0ebe0d5935c70da@...kaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=2f4d19de8c9e9f0b9794e53ca54d68e0ffe9f068
Signed-off-by: (Ben) HokChun Ng <kdev@...benng.net>
---
 drivers/media/dvb-frontends/dib3000mb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib3000mb.c b/drivers/media/dvb-frontends/dib3000mb.c
index a6c2fc4586eb..0dd96656aaf4 100644
--- a/drivers/media/dvb-frontends/dib3000mb.c
+++ b/drivers/media/dvb-frontends/dib3000mb.c
@@ -50,15 +50,19 @@ MODULE_PARM_DESC(debug, "set debugging level (1=info,2=xfer,4=setfe,8=getfe (|-a
 
 static int dib3000_read_reg(struct dib3000_state *state, u16 reg)
 {
+	int errno;
 	u8 wb[] = { ((reg >> 8) | 0x80) & 0xff, reg & 0xff };
-	u8 rb[2];
+	u8 rb[2] = { 0, 0 };
 	struct i2c_msg msg[] = {
 		{ .addr = state->config.demod_address, .flags = 0,        .buf = wb, .len = 2 },
 		{ .addr = state->config.demod_address, .flags = I2C_M_RD, .buf = rb, .len = 2 },
 	};
 
-	if (i2c_transfer(state->i2c, msg, 2) != 2)
-		deb_i2c("i2c read error\n");
+	errno = i2c_transfer(state->i2c, msg, 2);
+	if (errno != 2) {
+		deb_i2c("i2c read error (errno: %d)\n", -errno);
+		return 0;
+	}
 
 	deb_i2c("reading i2c bus (reg: %5d 0x%04x, val: %5d 0x%04x)\n",reg,reg,
 			(rb[0] << 8) | rb[1],(rb[0] << 8) | rb[1]);
-- 
2.39.2