linux-kernel - Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid

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: <dd5d8e8ca634724ab4f77ac2ed8ab56956551bc3.camel@hadess.net>
Date: Thu, 13 Nov 2025 12:56:03 +0100
From: Bastien Nocera <hadess@...ess.net>
To: Benjamin Tissoires <bentiss@...nel.org>, Lucas Zampieri
	 <lzampier@...hat.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, Jiri Kosina
	 <jikos@...nel.org>, Sebastian Reichel <sre@...nel.org>, 
	linux-pm@...r.kernel.org
Subject: Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery

On Thu, 2025-11-13 at 11:47 +0100, Benjamin Tissoires wrote:
> On Nov 13 2025, Lucas Zampieri wrote:
> > Add struct hid_battery to encapsulate battery state per HID device.
> > This structure will allow tracking individual battery properties
> > including capacity, min/max values, report information, and status.
> > 
> > The structure includes a list node to enable support for multiple
> > batteries per device in subsequent patches.
> > 
> > This is a preparation step for transitioning from direct
> > power_supply
> > access to a more flexible battery management system.
> > 
> > Signed-off-by: Lucas Zampieri <lzampier@...hat.com>
> > ---
> >  include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
> 
> I know Bastien asked you to split out this into a separate commit,
> but I
> hate having a header change when noone uses it. It is painful for
> people
> needing to backport the further changes (imagine you are fixing a CVE
> without noticing it), as they also need to pull this one.

I usually find that a stand-alone "this can't possibly introduce bugs"
commit to be better (and it makes it easier to concentrate on smaller
amounts of content).

For the CVE use case, I don't really understand the problem. Either you
forget to backport the commit that added the type, in which case it
just doesn't compile, or you can backport the whole series (I would
hope that there's enough metadata in the kernel git to figure that out,
isn't there?).

In any case, your subsystem, your rules, just thought I'd mention why I
asked Lucas to split this up.

Cheers

> 
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index a4ddb94e3ee5..63422130de20 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -634,6 +634,36 @@ enum hid_battery_status {
> >  	HID_BATTERY_REPORTED,		/* Device sent unsolicited
> > battery strength report */
> >  };
> >  
> > +/**
> > + * struct hid_battery - represents a single battery power supply
> > + * @list: list node for linking into hid_device's battery list
> 
> For the first inclusion of the new struct, please drop the list
> field.
> This should go into the last patch when you actually make use of it.
> 
> > + * @dev: pointer to the parent hid_device
> > + * @ps: the power supply device
> > + * @capacity: current battery capacity
> > + * @min: minimum battery value
> > + * @max: maximum battery value
> > + * @report_type: type of report (HID_INPUT_REPORT,
> > HID_FEATURE_REPORT)
> > + * @report_id: report ID for this battery
> > + * @charge_status: current charge status
> > + * @status: battery status (unknown, queried, reported)
> > + * @avoid_query: if true, don't query battery (wait for device
> > reports)
> > + * @ratelimit_time: time for rate limiting battery updates
> > + */
> > +struct hid_battery {
> > +	struct list_head list;
> > +	struct hid_device *dev;
> > +	struct power_supply *ps;
> > +	__s32 capacity;
> > +	__s32 min;
> > +	__s32 max;
> > +	__s32 report_type;
> > +	__s32 report_id;
> > +	__s32 charge_status;
> > +	enum hid_battery_status status;
> > +	bool avoid_query;
> > +	ktime_t ratelimit_time;
> > +};
> > +
> >  struct hid_driver;
> >  struct hid_ll_driver;
> >  
> > -- 
> > 2.51.1
> > 
> 
> Cheers,
> Benjamin