Re: [PATCH 3/3] s390/dasd: Fix undersized format-check buffer

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing input validation allows an unprivileged user to trigger a massive memory allocation, leading to a stack trace or kernel panic (if panic_on_warn is enabled).
- [High] This problem wasn't introduced by this patch, but out-of-bounds array reads occur in the while loop parsing trailing zeroes in the format buffer, potentially leading to a kernel panic.
--

--- Patch [3]: [PATCH 3/3] s390/dasd: Fix undersized format-check buffer ---
commit fff320f435d672762ac313457acd2d29da83d443
Author: Stefan Haberland <sth@linux.ibm.com>

s390/dasd: Fix undersized format-check buffer

This patch fixes an issue where a 64-bit calculation for a buffer size was
silently truncated to a 32-bit integer. By using a size_t and checking for
multiplication overflows, it prevents undersized allocations that lead to
out-of-bounds writes.

Cc: stable@vger.kernel.org #4.7

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

[ ... ]

> @@ -3490,7 +3491,9 @@ static int dasd_eckd_check_device_format(struct dasd_device *base,
>  	rpt_exp = recs_per_track(&private->rdc_data, 0, cdata->expect.blksize);
>  
>  	trkcount = cdata->expect.stop_unit - cdata->expect.start_unit + 1;
> -	fmt_buffer_size = trkcount * rpt_max * sizeof(struct eckd_count);
> +	if (check_mul_overflow(trkcount, rpt_max, &fmt_buffer_size) ||
> +	    check_mul_overflow(fmt_buffer_size, sizeof(struct eckd_count), &fmt_buffer_size))
> +		return -EINVAL;
>  
>  	fmt_buffer = kzalloc(fmt_buffer_size, GFP_KERNEL | GFP_DMA);

[Severity: High]
This is a pre-existing issue, but since the track range start_unit and
stop_unit are user-controlled via the ioctl, could an unprivileged user
still trigger a massive memory allocation request here?

Without the __GFP_NOWARN flag, passing an extremely large size to kzalloc
will log an allocator warning, which can panic the kernel if panic_on_warn
is enabled.

Should we add __GFP_NOWARN to this allocation, or enforce a stricter upper
bound on trkcount?

[Severity: High]
This is also a pre-existing issue, but do we have an out-of-bounds array
read later on when evaluating the tracks from this same format buffer?

Looking at dasd_eckd_format_evaluate_tracks() called when processing
these tracks in transport mode:

drivers/s390/block/dasd_eckd.c:dasd_eckd_format_evaluate_tracks() {
    ...
            while (fmt_buffer[pos].record == 0 &&
                   fmt_buffer[pos].dl == 0) {
                if (pos++ > max_entries)
                    break;
            }
    ...
}

Since the loop condition evaluates fmt_buffer[pos] before checking if pos
exceeds max_entries, it seems we can read past the end of the allocated
fmt_buffer array.

Additionally, this out-of-bounds pos index is passed to
dasd_eckd_count_records(), resulting in another unconditional out-of-bounds
read:

drivers/s390/block/dasd_eckd.c:dasd_eckd_count_records() {
    ...
    head = fmt_buffer[start].head;
    ...
}

Could this trigger memory safety warnings or a kernel panic if the read
crosses into an unmapped page?

>  	if (!fmt_buffer)
>  		return -ENOMEM;

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