Re: [PATCH v5 15/17] s390/dasd: Report ESE capability and format mode at device online

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `BIODASDRAS` ioctl crashes the kernel on DASD devices whose discipline does not implement `.is_ese`.
--

--- Patch [15]: [PATCH v5 15/17] s390/dasd: Report ESE capability and format mode at device online ---
commit a8b5bcde61b1c7e9effa84a4b07196f1129e6ac3
Author: Stefan Haberland <[email protected]>

s390/dasd: Report ESE capability and format mode at device online

This patch extends the device information line logged when a volume comes
online with the ESE hardware capability and the on-disk format mode. It
derives the format mode (full or on demand) from the on-disk format label.

> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 8f7ffbeb9a6ca..a7d874e79336e 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c

[ ... ]

This isn't a bug introduced by this patch, but while reviewing the ESE
support I noticed a pre-existing issue in the BIODASDRAS ioctl handling:

drivers/s390/block/dasd_ioctl.c:dasd_release_space() {
    if (!device->discipline->is_ese && !device->discipline->is_ese(device))
        return -ENOTSUPP;
    if (!device->discipline->release_space)
        return -ENOTSUPP;

[Severity: High]
Can this logic cause a kernel panic if a privileged user issues the
BIODASDRAS ioctl on a device whose discipline lacks the .is_ese callback?

If the .is_ese function pointer is NULL, the first half of the condition
evaluates to true. The && operator then proceeds to evaluate the right
side, which immediately dereferences the NULL function pointer.

Additionally, when the callback is present (such as on ECKD devices), does
the && operator skip the capability check entirely? Since the left side
evaluates to false when the pointer exists, the right side is never
executed.

Should this condition use || instead of && to properly check for both the
existence of the callback and its return value?

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