Re: [PATCH v10 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] This problem wasn't introduced by this patch, but it is a pre-existing high-severity issue in `ibmvfc_handle_scrq()`. The unvalidated pointer `evt` (derived directly from the hypervisor's untrusted `crq->ioba` payload) is dereferenced via `evt->queue` to determine which event pool to validate against. If the hypervisor provides a non-NULL garbage correlation token, this will trigger a kernel panic before the validation function can even execute.
--

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

scsi: ibmvfc: add NOOP command support

This commit adds support for handling IBMVFC_NOOP format CRQ messages from
the VIOS partner. It handles and ignores NOOP messages in both
ibmvfc_handle_crq() and ibmvfc_handle_scrq(), while also logging unexpected
NOOP commands. Additionally, it fixes a typo and guards against null event
pointers before processing CRQ responses.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 514fc87c10173..7ea64956653ef 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -4223,7 +4233,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 isn't a bug introduced by this patch, but is it possible for a malformed
CRQ message to crash the host in ibmvfc_handle_scrq() here?

If the hypervisor (VIOS) returns a CRQ response with a non-NULL garbage
correlation token in crq->ioba, it will be cast to struct ibmvfc_event *evt.
The !evt check will pass, but immediately after this block, the code calls:

	if (unlikely(!ibmvfc_valid_event(&evt->queue->evt_pool, evt))) {

This dereferences evt->queue to determine which event pool to validate against
before the pointer is actually validated. This could lead to an out-of-bounds
memory read and a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911054832.1311668-1-tyreld@linux.ibm.com?part=2