Re: [PATCH v7 02/19] s390/dasd: Propagate partial completion length across ERP recovery

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

Pre-existing issues:
- [Critical] dasd_eckd_ese_read() directly dereferences cqr->callback_data, which is NULL for ERP requests, causing a kernel panic.
--

--- Patch [2]: [PATCH v7 02/19] s390/dasd: Propagate partial completion length across ERP recovery ---
commit 5cfd110150006620d9ee9acaf83c89fea865281b
Author: Stefan Haberland <[email protected]>

s390/dasd: Propagate partial completion length across ERP recovery

This commit ensures that the partial completion length of a request is properly
carried over during error recovery procedure (ERP) post-actions. It addresses
an issue where partially completed requests would silently return zeroed data
because the processed bytes counter was dropped.

> dasd_default_erp_postaction() copies the timing and device state from
> the finished ERP request back to the original request but drops
> proc_bytes. A request that was partially completed, an ESE read of a
> not-yet-allocated track returns fewer bytes than requested, and then
> recovered through the ERP chain loses its partial-completion length.

[Severity: Critical]
This is a pre-existing issue, but since we are looking at ESE reads recovered
through the ERP chain, does the current error handling sequence lead to a NULL
pointer dereference?

When an ERP request encounters a No Record Found (NRF) error while reading
from an unallocated track, dasd_int_handler() processes it. While it uses
dasd_get_callback_data() to check the data direction, it appears to pass the
raw ERP request to ese_read():

drivers/s390/block/dasd.c:dasd_int_handler() {
    ...
    if (device->discipline->ese_read(cqr, irb))
    ...
}

Inside dasd_eckd_ese_read(), the code directly casts cqr->callback_data
instead of using the helper:

drivers/s390/block/dasd_eckd.c:dasd_eckd_ese_read() {
    ...
    req = (struct request *) cqr->callback_data;
    ...
    first_trk = first_blk = blk_rq_pos(req) >> block->s2b_shift;
    ...
}

Since ERP requests allocated via dasd_alloc_erp_request() do not set
callback_data, req becomes NULL and is immediately dereferenced by
blk_rq_pos(). Could this cause a kernel panic, and should dasd_eckd_ese_read()
be updated to use dasd_get_callback_data(cqr) instead?

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