linux-kernel - Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for registered reboot

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ee0c4a7b-e3d1-1816-d5b3-e53ec3cf2e8f@oss.qualcomm.com>
Date: Mon, 5 Jan 2026 23:15:00 +0530
From: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
To: Bartosz Golaszewski <brgl@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-pm@...r.kernel.org, Bartosz Golaszewski <brgl@...ev.pl>,
        Sebastian Reichel <sre@...nel.org>,
        Bartosz Golaszewski <bgolasze@...cinc.com>,
        Bjorn Andersson <andersson@...nel.org>
Subject: Re: [PATCH v22 2/2] power: reset: reboot-mode: Expose sysfs for
 registered reboot_modes



On 1/2/2026 6:55 PM, Bartosz Golaszewski wrote:
> On Fri, 26 Dec 2025 19:56:34 +0100, Shivendra Pratap
> <shivendra.pratap@....qualcomm.com> said:
>> Currently, there is no standardized mechanism for userspace to discover
>> which reboot-modes are supported on a given platform. This limitation
>> forces tools and scripts to rely on hardcoded assumptions about the
>> supported reboot-modes.

[SNIP..]

>>
>> +struct sysfs_data {
> 
> Let's make this more descriptive? struct reboot_mode_sysfs_data?

Ack. thanks.

> 
>> +	const char *mode;
>> +	struct list_head list;
>> +};
>> +

[SNIP..]

>> +
>> +	reboot->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0,
>> +						   (void *)head, reboot->dev->driver->name);
> 
> No, why pass the list? You should create an instance of struct sysfs_data per
> device_create(). If it needs to contain a list, then let it contain a list but
> don't allocate the list_head, that's really unusual.
> 

ok. Will create struct reboot_mode_sysfs_data with a list head and
allocate it as data.

>> +

[SNIP..]

>>
>> +static inline void reboot_mode_unregister_device(struct reboot_mode_driver *reboot)
>> +{
>> +	struct sysfs_data *sysfs_info;
>> +	struct sysfs_data *next;
>> +	struct list_head *head;
>> +
>> +	head = dev_get_drvdata(reboot->reboot_mode_device);
>> +	device_unregister(reboot->reboot_mode_device);
>> +	reboot->reboot_mode_device = NULL;
>> +
>> +	if (head) {
>> +		list_for_each_entry_safe(sysfs_info, next, head, list) {
>> +			list_del(&sysfs_info->list);
>> +			kfree_const(sysfs_info->mode);
>> +			kfree(sysfs_info);
>> +		}
> 
> This loop is duplicated, can you please factor it out into a dedicated
> function?

The loop frees the sysfs data. You mean i should directly call
reboot_mode_unregister_device in error path of reboot_mode_create_device
as not to duplicate the loop?

> 

[SNIP..]

>> diff --git a/include/linux/reboot-mode.h b/include/linux/reboot-mode.h
>> index 4a2abb38d1d612ec0fdf05eb18c98b210f631b7f..b56783c32068096325f92445b9530d1856c4826c 100644
>> --- a/include/linux/reboot-mode.h
>> +++ b/include/linux/reboot-mode.h
>> @@ -5,6 +5,7 @@
>>  struct reboot_mode_driver {
>>  	struct device *dev;
>>  	struct list_head head;
>> +	struct device *reboot_mode_device;
> 
> Why can't this be part of struct (reboot_mode_)sysfs_data?
> 

If reboot_mode_device is kept in sysfs_data, we need a reference to free
it. Should I maintain reference for it in "reboot struct" and store 
sysfs_data pointer, so that it can be used to call device_unregister()?

Eg:
struct reboot 
{ 
..
..
  void *priv;
};

struct reboot_mode_sysfs_data {
    struct device *reboot_mode_device;
    struct list_head head;
};

---
data =  kzalloc(reboot_mode_sysfs_data);
reboot->priv = data;
--

thanks,
Shivendra