Re: [PATCH v7 09/19] 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] Removal of `trkcount` without an active replacement introduces a TOCTOU race allowing double-formatting and silent data corruption.
- [High] The new `collision` check in `test_and_set_format_track()` is performed outside `format_lock`, creating a TOCTOU race even when fully activated.
--

--- Patch [9]: [PATCH v7 09/19] s390/dasd: Add range-based format-track collision detection ---
Note: The format of this report has been altered to a free-form summary due
to recitation restrictions preventing direct quoting of the patch diff.

commit 4fd0dd7f48bc14f4366f229841daa2dd1074fb30
Author: Stefan Haberland <[email protected]>

s390/dasd: Add range-based format-track collision detection

This commit replaces the single per-device format_entry slot with an array
of 16 slots to allow multiple format requests in flight. It also introduces
range-based overlap checks and a new collision flag.

[Severity: Critical]
Does the removal of the trkcount check in test_and_set_format_track() in
drivers/s390/block/dasd_eckd.c introduce a window for silent data corruption
before the new collision mechanism is fully activated?

It appears that the old trkcount protection is removed, but the commit
message notes that the new collision check remains dormant until a later
patch routes the format requests through dasd_return_cqr_cb() in
drivers/s390/block/dasd.c. Currently, format requests still use
dasd_eckd_ese_format_cb(), meaning cqr->collision will not be set.

If a data request fails with an unformatted track error, initiates a format,
fails again, and is delayed in ERP, could a concurrent request format the
same track and write data to it? When the first request finally retries
test_and_set_format_track(), the format_list would be empty and the
collision flag would remain unset, potentially leading to a double-format
that erases the data written by the concurrent request. 

Is there a way to keep the trkcount protection active until the new
mechanism is fully wired up?

[Severity: High]
Is there a race condition in test_and_set_format_track() in
drivers/s390/block/dasd_eckd.c even when the collision mechanism becomes
fully active?

The read of cqr->collision is performed locklessly before acquiring the
format_lock. If a thread reads the collision flag as false, and immediately
afterward a softirq completing a format sets it to true and removes the
track from format_list, the first thread will then acquire format_lock.
Finding the format_list empty, it would proceed to incorrectly format the
track again.

Should the check of cqr->collision be moved inside the format_lock section,
or re-evaluated inside, so that it is atomic with the inspection of the
format_list?

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