Re: [PATCH v9 2/9] scsi: ibmvfc: add NOOP command support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `ibmvfc_handle_scrq` function dereferences the untrusted `evt` pointer to access `evt->queue` before validating it, which will trigger a kernel panic if the hypervisor provides a non-NULL invalid correlation token.
--

commit e6c05534301788d9c2d48c5e067192600cb0bcda
Author: Dave Marquardt <davemarq@linux.ibm.com>

scsi: ibmvfc: add NOOP command support

Adds support for handling IBMVFC_NOOP format CRQ messages from the VIOS
partner and advertises the client capability. It also attempts to harden
message handling in ibmvfc_handle_scrq() by guarding against null events.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -4225,7 +4235,20 @@ static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost
>  	case IBMVFC_CRQ_XPORT_EVENT:
>  		return;
>  	default:
> -		dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid);
> +		dev_err(vhost->dev, "Got an invalid message type 0x%02x\n", crq->valid);
> +		return;
> +	}
> +
> +	if (crq->format == IBMVFC_NOOP) {
> +		if (vhost->state == IBMVFC_ACTIVE &&
> +		    !ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_NOOP_CMD))
> +			dev_err_ratelimited(vhost->dev,
> +					    "Received unexpected NOOP command from partner\n");
> +		return;
> +	}
> +
> +	if (unlikely(!evt)) {
> +		dev_err(vhost->dev, "Received null event\n");
>  		return;
>  	}

[Severity: High]
This is a pre-existing issue, but this patch attempts to fix token validation
by adding an if (!evt) check, which still leaves the kernel vulnerable to
malformed non-NULL tokens.

Because the event pointer is directly cast from an untrusted 64-bit value sent
over the channel, if the hypervisor provides a non-NULL, invalid correlation
token, doesn't ibmvfc_handle_scrq() still dereference the untrusted pointer
before validating it?

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_handle_scrq() {
    ...
    if (unlikely(!ibmvfc_valid_event(&evt->queue->evt_pool, evt))) {
    ...
}

Could this dereference of evt->queue trigger a kernel panic before the event
is proven valid?

To fix this safely, should ibmvfc_handle_scrq() accept the scrq instance from
its caller and validate against &scrq->evt_pool without dereferencing evt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-ibmvfc-fpin-support-v8-0-b27183b055af@linux.ibm.com?part=2