Re: [PATCH v7 15/19] s390/dasd: Stamp a format label into newly formatted volumes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel heap buffer overflow in `dasd_eckd_build_format()` when formatting multiple tracks with non-standard intensities (e.g., 0x01, 0x09, 0x04, or 0x0c).
--

--- Patch [15]: [PATCH v7 15/19] s390/dasd: Stamp a format label into newly formatted volumes ---
commit 13926f208a70b78985e240163ddd3726aa7b2dea
Author: Stefan Haberland <[email protected]>

s390/dasd: Stamp a format label into newly formatted volumes

When a CDL volume is formatted, write a small on-disk label so the format
can later be recognised by the kernel.

> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c

[ ... ]

> @@ -2819,6 +2852,10 @@ dasd_eckd_build_format(struct dasd_device *base, struct dasd_device *startdev,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	/* room for the label data that R4 carries in addition to its count */
> +	if (write_label)
> +		datasize += fdata->blksize;
> +
>  	fcp = dasd_fmalloc_request(DASD_ECKD_MAGIC, cplength, datasize, startdev);

[Severity: High]
This is a pre-existing issue, but can this lead to a kernel heap buffer
overflow when formatting multiple tracks with non-standard intensities?

In dasd_eckd_build_format(), when handling intensities like 0x01, 0x09, 0x04,
or 0x0c, the cplength and datasize calculations underestimate the required
memory if nr_tracks > 1.

For example, with intensity 0x04 (invalidate track) or 0x0c, cplength is
hardcoded to 3 in the switch block prior to this allocation. However, if a
user issues a BIODASDFMT ioctl with a track range resulting in nr_tracks > 1,
the subsequent track processing loop unconditionally generates CCWs for every
track:

for (j = 0; j < nr_tracks; j++) {
        ...
        if ((intensity & ~0x08) & 0x04) {
                ...
                ccw++;
        }
}

Because the initial allocation does not properly scale with the number of
tracks for these specific intensities, advancing the ccw and data pointers
appears to write past the allocated limits.

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