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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAMRc=MedMkQTGYzSFeiCovKTb_3Mpy8sOa66usQxtu+yi46-2g@mail.gmail.com>
Date: Tue, 13 Jan 2026 15:59:34 +0100
From: Bartosz Golaszewski <brgl@...nel.org>
To: Shivendra Pratap <shivendra.pratap@....qualcomm.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	linux-pm@...r.kernel.org, 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 Mon, Jan 5, 2026 at 6:45 PM Shivendra Pratap
<shivendra.pratap@....qualcomm.com> wrote:
>
>
>
> 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.
>

Sorry for the delayed response.

> [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?
>

I was thinking about wrapping it in a dedicated function and calling
it here and in the error path in reboot_mode_create_device().

> >
>
> [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;
> };
>

You can use class_find_device(). Store the address of the associated
reboot_mode_driver in the private structure and compare by it in the
match callback.

Bart