linux-kernel - Re: [PATCH] power: supply: bq27xxx: fix error return in case of no bq27000 hdq battery

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

Message-ID: <20250821220552.2cb701f9@akair>
Date: Thu, 21 Aug 2025 22:05:52 +0200
From: Andreas Kemnade <andreas@...nade.info>
To: "H. Nikolaus Schaller" <hns@...delico.com>
Cc: Sebastian Reichel <sre@...nel.org>, Jerry Lv <Jerry.Lv@...s.com>, Pali
 Rohár <pali@...nel.org>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, letux-kernel@...nphoenux.org,
 stable@...r.kernel.org, kernel@...a-handheld.com
Subject: Re: [PATCH] power: supply: bq27xxx: fix error return in case of no
 bq27000 hdq battery

Am Thu, 21 Aug 2025 20:54:41 +0200
schrieb "H. Nikolaus Schaller" <hns@...delico.com>:

> > Am 21.08.2025 um 20:15 schrieb Andreas Kemnade <andreas@...nade.info>:
> > 
> > Hi,
> > 
> > Am Mon, 21 Jul 2025 14:46:09 +0200
> > schrieb "H. Nikolaus Schaller" <hns@...delico.com>:
> >   
> >> Since commit
> >> 
> >> commit f16d9fb6cf03 ("power: supply: bq27xxx: Retrieve again when busy")
> >> 
> >> the console log of some devices with hdq but no bq27000 battery
> >> (like the Pandaboard) is flooded with messages like:
> >> 
> >> [   34.247833] power_supply bq27000-battery: driver failed to report 'status' property: -1
> >> 
> >> as soon as user-space is finding a /sys entry and trying to read the
> >> "status" property.
> >> 
> >> It turns out that the offending commit changes the logic to now return the
> >> value of cache.flags if it is <0. This is likely under the assumption that
> >> it is an error number. In normal errors from bq27xxx_read() this is indeed
> >> the case.
> >> 
> >> But there is special code to detect if no bq27000 is installed or accessible
> >> through hdq/1wire and wants to report this. In that case, the cache.flags
> >> are set (historically) to constant -1 which did make reading properties
> >> return -ENODEV. So everything appeared to be fine before the return value was
> >> fixed. Now the -1 is returned as -ENOPERM instead of -ENODEV, triggering the
> >> error condition in power_supply_format_property() which then floods the
> >> console log.
> >> 
> >> So we change the detection of missing bq27000 battery to simply set
> >> 
> >> cache.flags = -ENODEV
> >> 
> >> instead of -1.
> >>   
> > This all is a bit inconsistent, the offending commit makes it worse. 
> > Normally devices appear only in /sys if they exist. Regarding stuff in
> > /sys/class/power_supply, input power supplies might be there or not,
> > but there you can argument that the entry in /sys/class/power_supply
> > only means that there is a connector for connecting a supply.  
> 
> Indeed. If there is an optional bq27000 hdq battery the entry exists.
> 
Which is the condition that there is an optional bq27000 battery?
w1 might be enabled for other reasons. The bq27000 is not the only w1
chip in the world. BTW: I have removed the battery from my macbook and
there is no battery entry in /sys/class/power_supply. Same with another
laptop.

> > But having the battery entry everywhere looks like waste. If would
> > expect the existence of a battery bay in the device where the common
> > battery is one with a bq27xxx.  
> 
> I think the flaw you are mentioning is a completely diffent one. It comes from that
> the 1-wire or hdq interface of some omap processors is enabled in the .dtsi by default
> instead of disabling it like other interfaces (e.g. mcbsp1). E.g. for omap3 hdqw1w:
> 
> https://elixir.bootlin.com/linux/v6.16.1/source/arch/arm/boot/dts/ti/omap/omap3.dtsi#L502
> 
> And we should have the dts for the boards enable it only if the hdq interface is really
> in use and there is a chance that a bq27000 can be connected. In that case the full
> /sys entry is prepared but returns -ENODEV if the battery is missing, which is then
> exactly the right error return (instead of -EPERM triggering the console message).
>

And why do you think bq27000 should behave different than
max1721x_battery or ds2780_battery or ds2781_battery? If I enable the
drivers there is no additional battery in /sys/class/power_supply! Why
should everyone have a bq27000 in /sys/class/power_supply if the driver
is enabled and w1 is used for something? I wonder if the -ENODEV should
be catched earlier.

Regards,
Andreas