Re: [PATCH v6 16/18] 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] Lockless reading of partially updated `dasd_format_label` causes a data race and potential torn read in sysfs.
--

--- Patch [16]: [PATCH v6 16/18] s390/dasd: Report ESE capability and format mode at device online ---
commit 1bb0c0f3e976fb237f52d951feaeac8bfd58702e
Author: Stefan Haberland <[email protected]>

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

This commit 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 35a970e006fd0..643eb7882f9cd 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -2608,15 +2609,29 @@ static int dasd_eckd_end_analysis(struct dasd_block *block)
>  			  private->rdc_data.trk_per_cyl *
>  			  blk_per_trk);
>  
> +	/*
> +	 * 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 calling dasd_eckd_on_demand_format() locklessly result in a data race
and a potential torn read?

When reading from sysfs, or here in dasd_eckd_end_analysis(), we read the
label without locks:

drivers/s390/block/dasd_eckd.c:dasd_eckd_on_demand_format() {
    ...
	if (private->ese_label_valid)
		return !!(private->ese_label.flags & DASD_ESE_LABEL_F_QUICK);
    ...
}

However, other paths overwrite the label locklessly. This happens during
the format ioctl:

drivers/s390/block/dasd_eckd.c:dasd_eckd_fill_format_label() {
    ...
	private->ese_label = *label;
	private->ese_label_valid = true;
    ...
}

It also happens during device probe:

drivers/s390/block/dasd_eckd.c:dasd_eckd_read_format_label() {
    ...
		private->ese_label = *label;
		private->ese_label_valid = true;
    ...
}

If a user repeatedly reads the sysfs attribute (or this device online path
is executed) while concurrently running a format ioctl, could this cause a
torn read that returns a corrupted or uninitialized boolean value?

> +		fmt_str = ", on-demand format";
> +	else if (dasd_eckd_ese_capable(device))
> +		fmt_str = ", full format";

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