linux-kernel - Re: [PATCH 1/2] Add manage_restart device attribute to scsi

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: <524f67bf-f4df-41bf-bf1b-5cd0a79649eb@kernel.org>
Date: Mon, 6 Oct 2025 09:58:17 +0900
From: Damien Le Moal <dlemoal@...nel.org>
To: Markus Probst <markus.probst@...teo.de>, Niklas Cassel
 <cassel@...nel.org>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>
Cc: linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] Add manage_restart device attribute to scsi_disk

On 10/6/25 04:06, Markus Probst wrote:
> there is already manage_shutdown, manage_system_start_stop and
> manage_runtime_start_stop device attributes that allows the high-level
> device driver (sd) manage the device power state, expect for the
> system_state SYSTEM_RESTART. With this device attribute, it is possible to
> let the high-level device driver (sd) manage the device power state for
> SYSTEM_RESTART too.
> 
> Signed-off-by: Markus Probst <markus.probst@...teo.de>
> ---
>  drivers/scsi/sd.c          | 35 ++++++++++++++++++++++++++++++++++-
>  include/scsi/scsi_device.h |  6 ++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5b8668accf8e..a3e9c2e9d9f4 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -318,6 +318,36 @@ static ssize_t manage_shutdown_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(manage_shutdown);
>  
> +static ssize_t manage_restart_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> +	struct scsi_device *sdp = sdkp->device;

sdp is not really needed.

> +
> +	return sysfs_emit(buf, "%u\n", sdp->manage_restart);
> +}
> +
> +
> +static ssize_t manage_restart_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> +	struct scsi_device *sdp = sdkp->device;

Same here.

> +	bool v;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (kstrtobool(buf, &v))
> +		return -EINVAL;
> +
> +	sdp->manage_restart = v;
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(manage_restart);
> +
>  static ssize_t
>  allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> @@ -654,6 +684,7 @@ static struct attribute *sd_disk_attrs[] = {
>  	&dev_attr_manage_system_start_stop.attr,
>  	&dev_attr_manage_runtime_start_stop.attr,
>  	&dev_attr_manage_shutdown.attr,
> +	&dev_attr_manage_restart.attr,
>  	&dev_attr_protection_type.attr,
>  	&dev_attr_protection_mode.attr,
>  	&dev_attr_app_tag_own.attr,
> @@ -4175,7 +4206,9 @@ static void sd_shutdown(struct device *dev)
>  	    (system_state == SYSTEM_POWER_OFF &&
>  	     sdkp->device->manage_shutdown) ||
>  	    (system_state == SYSTEM_RUNNING &&
> -	     sdkp->device->manage_runtime_start_stop)) {
> +	     sdkp->device->manage_runtime_start_stop) ||
> +	    (system_state == SYSTEM_RESTART &&
> +	     sdkp->device->manage_restart)) {
>  		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
>  		sd_start_stop_device(sdkp, 0);
>  	}
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 6d6500148c4b..c7e657ac8b6d 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -178,6 +178,12 @@ struct scsi_device {
>  	 */
>  	unsigned manage_shutdown:1;
>  
> +	/*
> +	 * If true, let the high-level device driver (sd) manage the device
> +	 * power state for system restart (reboot) operations.

What about cold boot ? Same is needed, no ? The name "manage_restart" is a bit
confusing since we already have manage_system_start_stop,
manage_runtime_start_stop and manage_shutdown. I do not have a better suggestion
though.

> +	 */
> +	unsigned manage_restart:1;
> +
>  	/*
>  	 * If set and if the device is runtime suspended, ask the high-level
>  	 * device driver (sd) to force a runtime resume of the device.


-- 
Damien Le Moal
Western Digital Research