Re: [PATCH v7] HID: pulsar: add driver for Pulsar gaming mice

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The driver leaks uninitialized kernel stack memory to userspace and allows a deterministic kernel crash because `battery_get_property` returns `1` without initializing the property value.
--

commit 6987787f50bcfdb5c82e17e354bba7b71293f806
Author: Nikolas Koesling <nikolas@koesling.info>

HID: pulsar: add driver for Pulsar gaming mice

Add a HID driver for Pulsar wireless gaming mice (X2 V2, X2H, X2A,
Xlite V3). The driver exposes battery level, voltage, and charging
status through the power supply framework.

[ ... ]
> diff --git a/drivers/hid/hid-pulsar.c b/drivers/hid/hid-pulsar.c
> new file mode 100644
> index 0000000000000..a6ff6e436d407
> --- /dev/null
> +++ b/drivers/hid/hid-pulsar.c
[ ... ]
> +static int read_power(struct pulsar_data *drvdata)
> +{
[ ... ]
> +	if (need_status) {
> +		ret = read_status(drvdata);
> +		if (ret < 0) {
[ ... ]
> +			goto unlock;
> +		}
> +
> +		battery->last_status = now;
> +
> +		if (!ret) {
> +			battery->available = false;
> +			battery->refresh = false;
> +			goto unlock;
> +		}
> +
> +		/* device just became available, force power read */
> +		if (!battery->available)
> +			need_power = true;
> +	}
> +
> +	if (!need_power)
> +		goto unlock;
[ ... ]
> +unlock:
> +	up_write(&drvdata->lock_bat);
> +	return ret;
> +}

[Severity: Critical]
When read_status() succeeds and the device is available, it returns 1. If 
need_power is false (e.g., between 5 and 60 seconds after a previous read), 
read_power() jumps to unlock and returns this 1.

Does this unintentionally return 1 on success instead of 0?

> +
> +static int battery_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct pulsar_data *drvdata;
> +	int ret;
> +
> +	drvdata = power_supply_get_drvdata(psy);
> +
> +	ret = read_power(drvdata);
> +	if (ret)
> +		return ret;

[Severity: Critical]
Will this leak uninitialized kernel stack memory or cause a kernel crash?

If read_power() returns 1 as described above, this branch returns 1 
immediately, bypassing the switch statement that initializes the property 
value. 

The power supply core (e.g., in power_supply_format_property) only considers 
strictly negative values as errors. It treats 1 as success and will proceed 
to read the uninitialized union value. For integer properties, this leaks 
stack memory to userspace. For string properties like 
POWER_SUPPLY_PROP_MODEL_NAME, it dereferences the garbage stack pointer in 
val->strval, which can cause a direct kernel panic.

> +
> +	down_read(&drvdata->lock_bat);
> +
> +	switch (psp) {
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902161350.22233-1-nikolas@koesling.info?part=1