Re: [PATCH v6 08/18] s390/dasd: Add range-based format-track collision detection

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Premature removal of the `trkcount` check introduces a double-format vulnerability that breaks bisection and causes data corruption.
- [Low] Unsynchronized modification of `format->cqr` outside of `format_lock` causes a data race.
--

--- Patch [8]: [PATCH v6 08/18] s390/dasd: Add range-based format-track collision detection ---
Note: The format of this review has been altered to a free-form summary to
comply with system recitation restrictions. Standard diff quoting has been
omitted.

commit a04e5d02e092ddaf9f460306f01f82bfd4b72118
Author: Stefan Haberland <[email protected]>
s390/dasd: Add range-based format-track collision detection

This commit replaces the single format_entry with an array to support multiple
in-flight format requests, and modifies conflict detection to use range
overlaps. It also removes the trkcount snapshot in dasd_start_IO().

[Severity: Critical]
In dasd_start_IO(), the snapshot of trkcount was removed, which previously
protected against delayed-interrupt races. In test_and_set_format_track(),
this was replaced with a scan of format_list.

However, because the new collision flag mechanism is dormant in this patch,
does this premature removal introduce a double-format vulnerability that breaks
bisection?

If CPU 1 and CPU 2 both issue a WRITE to the same unallocated track, the
device fails both, generating two unformatted track interrupts. If CPU 1
processes its interrupt, formats the track, and retries its WRITE, it will
remove the track from format_list. If CPU 2 is delayed and processes its
interrupt after CPU 1s format completes, it calls test_and_set_format_track().

Since the track is no longer in format_list and the new collision mechanism is
inactive, CPU 2 will not detect the collision and will issue a second format,
potentially erasing CPU 1s freshly written data.

[Severity: Low]
In dasd_eckd_ese_format(), there is an assignment format->cqr = fcqr which
occurs outside of the format_lock.

Concurrently, in test_and_set_format_track(), a reader checks
device->format_entry[i].cqr while holding the lock.

Does this unsynchronized modification cause a data race on a shared pointer?
While it might not cause a crash since the pointer transitions from one valid
non-NULL pointer to another, it appears to violate kernel concurrency rules.

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