linux-kernel - Re: [PATCH v2 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support

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: <2025052902-dizzy-baggie-15ee@gregkh>
Date: Thu, 29 May 2025 06:40:05 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Marco Felsch <m.felsch@...gutronix.de>
Cc: Luis Chamberlain <mcgrof@...nel.org>,
	Russ Weight <russ.weight@...ux.dev>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Kamel Bouhara <kamel.bouhara@...tlin.com>,
	Marco Felsch <kernel@...gutronix.de>,
	Henrik Rydberg <rydberg@...math.org>,
	Danilo Krummrich <dakr@...hat.com>, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [PATCH v2 4/4] Input: Add TouchNetix aXiom I2C Touchscreen
 support

On Thu, May 29, 2025 at 12:08:45AM +0200, Marco Felsch wrote:
> +	if (!entry->info)
> +		WARN(1, "Unsupported usage u%x used, driver bug!", i);

You just crashed the system and caused all data to be lost if this is
ever hit :(

As you did detect this, please handle the error and recover.  It's a bit
rude for a single i2c driver to take down a whole system, right?

> +#define AXIOM_SIMPLE_FW_DEVICE_ATTR(attr)					\
> +	static ssize_t								\
> +	fw_ ## attr ## _show(struct device *dev,				\
> +			     struct device_attribute *_attr, char *buf)		\
> +	{									\
> +		struct i2c_client *i2c = to_i2c_client(dev);			\
> +		struct axiom_data *ts = i2c_get_clientdata(i2c);		\
> +										\
> +		return sprintf(buf, "%u\n", ts->fw_##attr);			\

sysfs_emit() please for all sysfs show functions.

> +	axiom_u42_get_touchslots(ts);
> +	if (!ts->num_slots && update_in_process) {
> +		input_free_device(input);
> +		/*
> +		 * Skip input device registration but don't throw an error to
> +		 * not abort the update since some FW updates require a
> +		 * following CFG update to re-initialize the touchslot handling.
> +		 */
> +		if (update_in_process) {
> +			dev_info(dev, "No touchslots found after FW or CFG update, skip registering input device\n");

Why is this info?  What can a user do with this?  Shouldn't this be a
dev_warn() call at the least?

> +			return 0;

You return success, but the device is NOT set up properly, how is that
going to work?

thanks,

greg k-h