linux-kernel - Re: [PATCH v3 1/1] hwmon: (sht3x) read out sensor serial number

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]

Date: Mon, 1 Jan 2024 23:18:21 +0100
From: Stefan Gloor <code@...fan-gloor.ch>
To: Guenter Roeck <linux@...ck-us.net>
Cc: jdelvare@...e.com, corbet@....net, linux-hwmon@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] hwmon: (sht3x) read out sensor serial number

On Sun, Dec 31, 2023 at 10:32:56AM -0800, Guenter Roeck wrote:
> This creates i2c<bus>-<address>/serial_number when the device is instantiated.
> That debugfs entry is not removed when the device is removed, only when the
> driver is unloaded. This means that de-instantiating the device will leave
> stray debugfs directories and files behind until the driver is unloaded.
> 
> We had this before, and I understand that you claimed that this doesn't happen.
> To get me to believe you, you'll have to provide a log of
> 
> - instantiating the driver 
> - Showing the debufs tree
> - de-instantiating the driver
> - Showing the debugfs tree
> 
> ... but even then I'll want to be able to test it myself. Not sure if I
> have an eval board, but either case that will take some time. Frankly,
> I don't understand why you refuse to remove
> i2c<bus>-<address>/serial_number on device removal.
> 
> Guenter
> 

Hi Guenter,

Thank you for your patience. As this is my first patch set for Linux I still
need to learn a lot.

You are right. I was confused about driver instantiation and driver
loading/unloading. The i2cX-XX directory needs to be removed explicitly.

If I understood correctly, the following changes should achieve this:

+static void sht3x_remove(struct i2c_client *client)
+{
+	struct sht3x_data *data;
+
+	data = dev_get_drvdata(&client->dev);
+	debugfs_remove_recursive(data->sensor_dir);
+}
+
 static struct i2c_driver sht3x_i2c_driver = {
 	.driver.name = "sht3x",
 	.probe       = sht3x_probe,
+	.remove      = sht3x_remove,
 	.id_table    = sht3x_ids,
 };

Of course data->sensor_dir needs to be set to the i2X-XX directory when it is
created.

If there is nothing obviously wrong with it I'll submit v4 shortly.

Best,
Stefan