linux-kernel - Re: [lm-sensors] [PATCH] hwmon: (ina2xx) Change register cache to signed

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140608203007.GA6265@roeck-us.net>
Date:	Sun, 8 Jun 2014 13:30:07 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Fabio Baltieri <fabio.baltieri@...il.com>
Cc:	linux-kernel@...r.kernel.org, Lothar Felten <l-felten@...com>,
	lm-sensors@...sensors.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (ina2xx) Change register cache to
 signed

On Sun, Jun 08, 2014 at 01:16:00PM -0700, Guenter Roeck wrote:
> On Sat, Jun 07, 2014 at 09:47:01PM +0100, Fabio Baltieri wrote:
> > All devices supported by the ina2xx driver are bidirectional and reports
> > the measured value as a signed 16 bit, but the current driver
> > implementation caches the number as an u16, leading to an incorrect sign
> > extension when reporting to the userspace in ina2xx_get_value().
> > 
> > This patch fixes the problem by using a s16 instead, and has been tested
> > on an INA219.
> > 
> > Signed-off-by: Fabio Baltieri <fabio.baltieri@...il.com>
> 
> Applied.
> 
Actually, no, this won't work. The statement above is only correct for current
and shunt voltage measurements, but not for power measurements and not for bus
voltage measurements. Changing the register to s16 won't help; conversion needs
to be done in ina2xx_get_value() for shunt voltage and current measurement only.
Otherwise we just move the bug from current/shunt voltage measurements to
power / bus voltage measurements.

Even more interesting, the power is supposed to be the product of Bus voltage
and current, and the latter can be negative. However, the power register
description does not suggest that the upper bit would be a sign bit. So there
is some discrepancy in the datasheet, and we'll need some real-world data to
understand if the upper power bit is signed or not.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/