Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Short I2C block reads return positive values or zero, violating API contract and causing false probe success or uninitialized variable usage.
--

commit 66708f4a4bcbbc5908e6fc11660dc174e28d7ee4
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

mfd: Add driver for ASUS Transformer embedded controller

Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
and TF600T, P1801-T and TF701T pad. This is a glue driver handling
detection and common operations for EC's functions.

> --- /dev/null
> +++ b/drivers/mfd/asus-transformer-ec.c

[ ... ]

> +int asus_dockram_access_ctl(struct i2c_client *client, u64 *out, u64 mask,
> +			    u64 xor)
> +{
> +	struct dockram_ec_data *ddata = i2c_get_clientdata(client);
> +	u8 *buf = ddata->ctl_buf;
> +	u64 val;
> +	int ret = 0;
> +
> +	guard(mutex)(&ddata->ctl_lock);
> +
> +	memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> +	ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> +					    ASUSEC_ENTRY_SIZE, buf);
> +	if (ret < ASUSEC_ENTRY_SIZE) {
> +		dev_err(&client->dev, "failed to access control buffer: %d\n",
> +			ret);
> +		return ret;

[Severity: High]
If i2c_smbus_read_i2c_block_data() returns a positive short read, will this
return a positive error code?

This positive value could propagate up to asus_ec_probe(). Returning a
positive value from probe causes __device_attach_driver() to interpret it
as a successful match and probe, leaving the device in an unbound, broken
state without a proper failure log.

Additionally, if it returns 0, the function will return success (0) without
populating output variables, potentially leading to uninitialized memory
usage in callers like asus_ec_susb_on_status().

> +	}
> +
> +	if (buf[0] != ASUSEC_CTL_SIZE) {

[ ... ]

> +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> +			    const char *name, const char **out)
> +{
> +	struct device *dev = &ddata->client->dev;
> +	u8 buf[ASUSEC_ENTRY_BUFSIZE];
> +	int ret;
> +
> +	memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> +	ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> +					    ASUSEC_ENTRY_SIZE, buf);
> +	if (ret < ASUSEC_ENTRY_SIZE)
> +		return ret;

[Severity: High]
Could this also return a positive short read or 0 to the caller?

Similar to asus_dockram_access_ctl(), this might propagate positive values
to the probe function or mask zero-byte read failures.

> +
> +	if (buf[0] > ASUSEC_ENTRY_SIZE) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528053203.9339-1-clamor95@gmail.com?part=2