Re: [PATCH 17/19] s390/dasd: Report ESE capability and format mode at device online

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing memory barrier when publishing the on-disk format label, allowing lockless sysfs readers to observe uninitialized data.
--

--- Patch [17]: [PATCH 17/19] s390/dasd: Report ESE capability and format mode at device online ---
commit 248d6eb45e332487f9255bf060a4d057f9939181
Author: Stefan Haberland <[email protected]>

s390/dasd: Report ESE capability and format mode at device online

This patch extends the device information line logged when a volume comes
online with the ESE hardware capability and the on-disk format mode. The
format mode is derived from the on-disk format label.

> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index d5854ed0076e3..3c4fcfb1558de 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -2609,15 +2610,29 @@ static int dasd_eckd_end_analysis(struct dasd_block *block)
[ ... ]
> +	/*
> +	 * Report the ESE hardware capability and the format mode. The mode
> +	 * comes from dasd_eckd_on_demand_format() (the on-disk label, or the
> +	 * ESE state when no label is present), matching the on_demand_formatting
> +	 * sysfs attribute.
> +	 */
> +	ese_str = dasd_eckd_ese_capable(device) ? ", ESE" : "";
> +	fmt_str = "";
> +	if (dasd_eckd_on_demand_format(device))

[Severity: Medium]
Could this lockless read of ese_label_valid in dasd_eckd_on_demand_format()
observe uninitialized or torn data from ese_label due to compiler reordering?

Looking at where the label is populated, such as in
dasd_eckd_read_format_label():

		private->ese_label = *label;
		private->ese_label_valid = true;

and in dasd_eckd_fill_format_label():

	private->ese_label = *label;
	private->ese_label_valid = true;

The valid flag is set to true immediately after the struct assignment without a
write memory barrier. In dasd_eckd_on_demand_format(), there is also no
acquire barrier:

	if (private->ese_label_valid)
		return !!(private->ese_label.flags & DASD_ESE_LABEL_F_QUICK);

Since dasd_eckd_on_demand_format() is accessible via sysfs attributes like
on_demand_formatting concurrently while the device is coming online or
formatting, could this missing barrier cause sysfs to transiently return
incorrect values (0 instead of 1)?

> +		fmt_str = ", on-demand format";
> +	else if (dasd_eckd_ese_capable(device))
> +		fmt_str = ", full format";
>  
>  	dev_info(&device->cdev->dev,
> -		 "DASD with %u KB/block, %lu KB total size, %u KB/track, "
> -		 "%s\n", (block->bp_block >> 10),
> +		 "DASD with %u KB/block, %lu KB total size, %u KB/track, %s%s%s\n",
> +		 (block->bp_block >> 10),
>  		 (((unsigned long) private->real_cyl *
>  		   private->rdc_data.trk_per_cyl *
>  		   blk_per_trk * (block->bp_block >> 9)) >> 1),
>  		 ((blk_per_trk * block->bp_block) >> 10),
>  		 private->uses_cdl ?
> -		 "compatible disk layout" : "linux disk layout");
> +		 "compatible disk layout" : "linux disk layout",
> +		 ese_str, fmt_str);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17