All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Stefan Haberland" <sth@linux.ibm.com>
Cc: "Vasily Gorbik" <gor@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	linux-s390@vger.kernel.org, "Heiko Carstens" <hca@linux.ibm.com>
Subject: Re: [PATCH v6 08/18] s390/dasd: Add range-based format-track collision detection
Date: Sat, 01 Aug 2026 11:36:34 +0000	[thread overview]
Message-ID: <20260801113634.959421F00AC4@smtp.kernel.org> (raw)
In-Reply-To: <20260801111008.3391031-9-sth@linux.ibm.com>

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 <sth@linux.ibm.com>
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/20260801111008.3391031-1-sth@linux.ibm.com?part=8

  reply	other threads:[~2026-08-01 11:36 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-01 11:09 [PATCH v6 00/18] s390/dasd: ESE thin-provisioning performance improvements Stefan Haberland
2026-08-01 11:09 ` [PATCH v6 01/18] s390/dasd: Do not complete a failed ESE read as successful Stefan Haberland
2026-08-01 11:27   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 02/18] s390/dasd: Propagate partial completion length across ERP recovery Stefan Haberland
2026-08-01 11:21   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 03/18] s390/dasd: Guard sysfs discipline callbacks against unallocated private data Stefan Haberland
2026-08-01 11:40   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 04/18] s390/dasd: Optimize max blocks per request for track alignment Stefan Haberland
2026-08-01 11:20   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 05/18] s390/dasd: Use GFP_KERNEL in dasd_alloc_device() Stefan Haberland
2026-08-01 11:21   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 06/18] s390/dasd: Add defines for the Extended Address Volume track address Stefan Haberland
2026-08-01 11:13   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 07/18] s390/dasd: Add infrastructure for ESE full-track write Stefan Haberland
2026-08-01 11:32   ` sashiko-bot
2026-08-01 11:09 ` [PATCH v6 08/18] s390/dasd: Add range-based format-track collision detection Stefan Haberland
2026-08-01 11:36   ` sashiko-bot [this message]
2026-08-01 11:09 ` [PATCH v6 09/18] s390/dasd: Extend prepare_itcw() to support WRITE_FULL_TRACK Stefan Haberland
2026-08-01 11:37   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 10/18] s390/dasd: Add dasd_eckd_build_cp_tpm_writefulltrack() Stefan Haberland
2026-08-01 11:29   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 11/18] s390/dasd: Use WRITE_FULL_TRACK in ESE format handler Stefan Haberland
2026-08-01 11:39   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 12/18] s390/dasd: Add full_track_bias to control fulltrack write mode Stefan Haberland
2026-08-01 11:27   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 13/18] s390/dasd: Derive adaptive ESE fulltrack heuristic from ft_bias Stefan Haberland
2026-08-01 11:46   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 14/18] s390/dasd: Stamp a format label into newly formatted volumes Stefan Haberland
2026-08-01 11:33   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 15/18] s390/dasd: Detect ESE volumes from the on-disk format label Stefan Haberland
2026-08-01 11:38   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 16/18] s390/dasd: Report ESE capability and format mode at device online Stefan Haberland
2026-08-01 11:45   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 17/18] s390/dasd: Re-enable discard support for ESE volumes Stefan Haberland
2026-08-01 11:45   ` sashiko-bot
2026-08-01 11:10 ` [PATCH v6 18/18] s390/dasd: Read cached unit address and LSS in the CCW build path Stefan Haberland
2026-08-01 11:49   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260801113634.959421F00AC4@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sth@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.