linux-kernel - [PATCH 1/1] mfd: tps65911-comparator: Fix an off by one bug

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,  1 May 2018 10:45:53 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     dan.carpenter@...cle.com
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Lee Jones <lee.jones@...aro.org>
Subject: [PATCH 1/1] mfd: tps65911-comparator: Fix an off by one bug

The COMP1 and COMP2 elements are in 0 and 1 respectively so this code is
accessing the wrong elements and one space beyond the end of the array.

The "id" variable is never COMP (0) so that code can be removed.

Fixes: 6851ad3ab346 ("TPS65911: Comparator: Add comparator driver")
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Lee Jones <lee.jones@...aro.org>
---

History:

Dan was the originator of this patch and the author of the commit log,
but produced 2 code solutions which I wasn't happy with.  The first
submission [0] introduced a COMP device, which after a quick check of
the datasheet [1] appeared to be fictitious.  A subsequent submission
[2] conducted arithmetic in array indexes.

It is my belief that the correct solution is to roll which the
situation the hardware engineers presented us with and define COMP1
at position 0 and COMP2 at position 1 such that we can use the
simplest code possible to select them.

Dan wasn't happy to put his name to this, which I completely
understand.  Calling SOMETHING1 0 (zero) is a little unnatural.

However, since I have no shame, I offered to submit it.

[0] https://lkml.org/lkml/2018/4/19/449
[1] http://www.ti.com/lit/ds/symlink/tps65911.pdf (page 52)
[2] https://lkml.org/lkml/2018/4/20/204

drivers/mfd/tps65911-comparator.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/tps65911-comparator.c b/drivers/mfd/tps65911-comparator.c
index d223857fb4ad..33591767fb9b 100644
--- a/drivers/mfd/tps65911-comparator.c
+++ b/drivers/mfd/tps65911-comparator.c
@@ -22,9 +22,8 @@
 #include <linux/gpio.h>
 #include <linux/mfd/tps65910.h>

-#define COMP					0
-#define COMP1					1
-#define COMP2					2
+#define COMP1					0
+#define COMP2					1

 /* Comparator 1 voltage selection table in millivolts */
 static const u16 COMP_VSEL_TABLE[] = {
@@ -63,9 +62,6 @@ static int comp_threshold_set(struct tps65910 *tps65910, int id, int voltage)
 	int ret;
 	u8 index = 0, val;

-	if (id == COMP)
-		return 0;
-
 	while (curr_voltage < tps_comp.uV_max) {
 		curr_voltage = tps_comp.vsel_table[index];
 		if (curr_voltage >= voltage)
@@ -89,9 +85,6 @@ static int comp_threshold_get(struct tps65910 *tps65910, int id)
 	unsigned int val;
 	int ret;

-	if (id == COMP)
-		return 0;
-
 	ret = tps65910_reg_read(tps65910, tps_comp.reg, &val);
 	if (ret < 0)
 		return ret;
-- 
2.17.0