Re: [PATCH v9 3/7] input: serio: Add driver for ASUS Transformer dock keyboard and touchpad

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Premature registration of the event notifier before callback initialization causes a race condition leading to a NULL pointer dereference.
- [High] Incorrect devres ordering leaves the notifier active after serio ports are destroyed, causing a Use-After-Free during teardown.
- [High] Incorrect bounds check on the EC packet length allows an out-of-bounds heap read and exposes uninitialized memory.
--

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

input: serio: Add driver for ASUS Transformer dock keyboard and touchpad

Add input driver for ASUS Transformer dock keyboard and touchpad. Modify
incoming keyboard data to send proper scancodes for keys that don't match
their position.

> diff --git a/drivers/input/serio/asus-transformer-ec-kbc.c b/drivers/input/serio/asus-transformer-ec-kbc.c
> new file mode 100644
> index 0000000000000..3ddfa9925b2b0
> --- /dev/null
> +++ b/drivers/input/serio/asus-transformer-ec-kbc.c

[ ... ]

> +static int asus_ec_kbc_notify(struct notifier_block *nb,
> +			      unsigned long action, void *data_)
> +{

[ ... ]

> +	/*
> +	 * The data[0] is the length of the packet including itself. The data[]
> +	 * buffer has to be at least 3 bytes (length + ctrl + 1 data byte) and
> +	 * must not exceed the EC buffer size.
> +	 */
> +	if (data[0] < 2 || data[0] > ASUSEC_ENTRY_BUFSIZE)
> +		return NOTIFY_BAD;
> +
> +	n = data[0] - 1;
> +	data += 2;

[ ... ]

> +	while (n--)
> +		serio_interrupt(priv->sdev[port_idx], *data++, 0);

[Severity: High]
If data[0] is ASUSEC_ENTRY_BUFSIZE (33), n becomes 32, and data is advanced
by 2. Will the loop read 32 bytes from data[2] up to data[33]?

Since the buffer is exactly 33 bytes long, could this read past the end of
the array or access uninitialized data?

> +
> +	return NOTIFY_OK;
> +}

[ ... ]

> +static int asus_ec_kbc_probe(struct platform_device *pdev)
> +{
> +	struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct asus_ec_kbc_data *priv;
> +	int error;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->ec = ec;
> +	priv->parent = to_i2c_client(pdev->dev.parent);
> +
> +	error = blocking_notifier_chain_register(&ec->notify_list, &priv->nb);

[Severity: High]
Is it safe to register the notifier here before priv->nb.notifier_call is
initialized?

If an interrupt fires immediately after registration, could the notifier core
attempt to call a NULL pointer?

> +	if (error)
> +		return dev_err_probe(&pdev->dev, error,
> +				     "failed to register blocking notifier chain");
> +
> +	error = devm_add_action_or_reset(&pdev->dev,
> +					 asus_ec_notifier_chain_unregister,
> +					 priv);

[Severity: High]
Since managed resources (devm) are released in LIFO order, will the serio
ports registered below be unregistered and freed before this notifier is
unregistered?

If an interrupt arrives during the unbind window or if the probe function
fails after port registration, could the notifier loop in asus_ec_kbc_notify()
dereference the freed serio port pointers?

> +	if (error)
> +		return error;
> +
> +	error = asus_ec_register_serio(pdev, 0, "Keyboard", 0);
> +	if (error)
> +		return error;
> +
> +	error = asus_ec_register_serio(pdev, 1, "Touchpad", I8042_CMD_AUX_SEND);
> +	if (error)
> +		return error;
> +
> +	priv->nb.notifier_call = asus_ec_kbc_notify;
> +
> +	return 0;
> +}

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