Re: [PATCH 3/4] s390/dasd: Fix wrong free and refcount leak in build_check_tcw error path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] s390/dasd: inaccurate Fixes tag and crash description in commit
  message
--

commit b485d08acd9f888238bf37617a7cb2fffa1637cc
Author: Stefan Haberland <sth@linux.ibm.com>

> s390/dasd: Fix wrong free and refcount leak in build_check_tcw error path
>
> dasd_eckd_build_check_tcw() allocates its request with
> dasd_fmalloc_request(), but its out_err path frees it with
> dasd_sfree_request(). The latter uses a different pool and therefor,
> leading to invalid pointer arithmetic and a crash.
> Use the matching dasd_ffree_request() instead.
>
> The same error path also leaves start_priv->count incremented
> without ever decrementing it again.
> This leaks a permanent reference on every build failure.
> Decrement it before freeing the request.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260722103704.1709668-1-sth@linux.ibm.com
> Fixes: 8fd575200db5 ("s390/dasd: Add new ioctl BIODASDCHECKFMT")
> Cc: stable@vger.kernel.org #4.7
> Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>

[Severity: Low]
Are the Fixes tag, the #4.7 stable hint, and the crash description in the
commit message accurate?

Commit 8fd575200db5 ("s390/dasd: Add new ioctl BIODASDCHECKFMT") introduced
the missing start_priv->count decrement, but it paired dasd_smalloc_request()
with dasd_sfree_request().  Commit 5e2b17e712cf ("s390/dasd: Add dynamic
formatting support for ESE volumes") in v4.18 introduced
dasd_fmalloc_request() and dasd_ffree_request() and switched
dasd_eckd_build_check_tcw() to dasd_fmalloc_request(), so backporting this
patch to v4.7..v4.17 will fail to build, and this commit should also carry:

  Fixes: 5e2b17e712cf ("s390/dasd: Add dynamic formatting support for ESE volumes")

Also, since commit 9cebfced1324 ("s390/dasd: Add infrastructure for ESE
full-track write"), dasd_sfree_request() checks cqr->mem_chunk and frees
requests with a NULL mem_chunk from device->ese_chunks rather than crashing:

drivers/s390/block/dasd.c:dasd_sfree_request() {
    ...
	if (cqr->mem_chunk)
		dasd_free_chunk(&device->ccw_chunks, cqr->mem_chunk);
	else
		dasd_free_chunk(&device->ese_chunks, cqr);
    ...
}

So the invalid pointer arithmetic and crash only occur on kernels between
5e2b17e712cf ("s390/dasd: Add dynamic formatting support for ESE volumes")
and 9cebfced1324 ("s390/dasd: Add infrastructure for ESE full-track write").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112102608.1057852-1-sth@linux.ibm.com?part=3