Re: [PATCH v4 1/2] Input: axp20x-pek - add is_visible callback for sysfs attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver unconditionally operates on uninitialized IRQs (0) during suspend and resume when the input device is not registered, which can crash or hang the system.
--

commit edceaed72d31aa98549e1afb728e99699b609d10
Author: Steven Feng <steven@joint-cloud.com>

Input: axp20x-pek - add is_visible callback for sysfs attributes

This commit adds an is_visible callback to control the visibility of sysfs
attributes based on hardware capabilities. It hides the attributes for
devices without a startup_time configuration.

> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 24f9e9d893de3..c1a4e5ea1e001 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -193,7 +193,28 @@ static struct attribute *axp20x_attrs[] = {
>  	&dev_attr_shutdown.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(axp20x);
> +
> +static umode_t axp20x_attr_is_visible(struct kobject *kobj,
> +				      struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> +	if (!axp20x_pek->info->startup_time)
> +		return 0;
> +
> +	return attr->mode;
> +}

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
suspend and resume are handled when the input device is not registered.

If axp20x_pek_should_register_input() returns false (for example, on Cherry
Trail platforms with certain ACPI devices), axp20x_pek_probe_input_device()
is skipped. Since axp20x_pek is allocated with devm_kzalloc(), the irq_dbf
and irq_dbr fields remain 0.

During system suspend and resume, if device_may_wakeup(dev) is false, the
callbacks in axp20x_pek_suspend() and axp20x_pek_resume() unconditionally
execute:

        disable_irq(axp20x_pek->irq_dbf);
        disable_irq(axp20x_pek->irq_dbr);

and

        enable_irq(axp20x_pek->irq_dbf);
        enable_irq(axp20x_pek->irq_dbr);

Could this cause the driver to unconditionally call disable_irq(0) and
enable_irq(0)? On x86 architectures, tampering with IRQ 0 (typically the
legacy system timer) could cause severe system instability or kernel hangs
during the suspend and resume cycle.

> +
> +static const struct attribute_group axp20x_group = {
> +	.attrs = axp20x_attrs,
> +	.is_visible = axp20x_attr_is_visible,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603074727.613319-1-steven@joint-cloud.com?part=1