Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dave Marquardt" <davemarq@linux.ibm.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v14 04/11] scsi: ibmvfc: add basic FPIN support
Date: Thu, 15 Oct 2026 15:18:34 +0000	[thread overview]
Message-ID: <sashiko-outbox-173047@kernel.org> (raw)
In-Reply-To: <20261015-ibmvfc-fpin-support-v14-4-a0b063b8d825@linux.ibm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: missing flush of dynamically allocated system_wq items causes UAF
--

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

scsi: ibmvfc: add basic FPIN support

Implement support for a basic level of Fabric Performance Impact
Notifications (FPIN) in the ibmvfc driver to enable monitoring of
fabric congestion and link integrity events.

Add async event handler for IBMVFC_AE_FPIN events that offloads FPIN
processing via schedule_work(). Convert VIOS FPIN messages to standard
fc_els_fpin structures and pass them to fc_host_fpin_rcv() for processing
by the FC transport layer.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 0fe6b6f4260ba..871decab29856 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -3261,6 +3266,255 @@ static const char *ibmvfc_get_link_state(enum ibmvfc_ae_link_state state)
>  	return "";
>  }
>  
[ ... ]
> +VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
> +{
> +	struct ibmvfc_async_work *aw;
> +	struct ibmvfc_async_crq *crq;
> +	struct ibmvfc_target *tgt;
> +	struct ibmvfc_host *vhost;
> +	struct fc_els_fpin *fpin;
> +	unsigned long flags;
> +
> +	aw = container_of_const(work, struct ibmvfc_async_work, async_work_s);
> +	vhost = aw->vhost;
> +	crq = &aw->crq;
[ ... ]
> +	fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
> +	if (fpin) {
> +		fc_host_fpin_rcv(tgt->vhost->host,
> +				 sizeof(*fpin) + be32_to_cpu(fpin->desc_len),
> +				 (char *)fpin, 0);
> +		kfree(fpin);
> +	} else
> +		dev_err_ratelimited(vhost->dev, "FPIN event received, unable to process\n");
> +
> +	kref_put(&tgt->kref, ibmvfc_release_tgt);

[Severity: High]
Can this untracked async worker drop its reference and trigger the release
function on a destroyed mempool?

If ibmvfc_remove() has already completed and destroyed vhost->tgt_pool,
this final kref_put() triggers ibmvfc_release_tgt():

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_release_tgt() {
	...
	mempool_free(tgt, tgt->vhost->tgt_pool);
}

This accesses the destroyed target mempool, causing a Use-After-Free.

> + free:
> +	scsi_host_put(aw->vhost->host);
> +	kfree(aw);
> +}
> +EXPORT_SYMBOL_IF_KUNIT(ibmvfc_process_async_work);
> +
>  /**
>   * ibmvfc_handle_async - Handle an async event from the adapter
>   * @crq:	crq to process
[ ... ]
> @@ -3361,12 +3616,28 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
>  	case IBMVFC_AE_HALT:
>  		ibmvfc_link_down(vhost, IBMVFC_HALTED);
>  		break;
> +	case IBMVFC_AE_FPIN:
> +		if (vhost->state == IBMVFC_HOST_OFFLINE)
> +			break;
> +		aw = kzalloc_obj(*aw, GFP_ATOMIC);
> +		if (aw) {
> +			if (!scsi_host_get(vhost->host)) {
> +				kfree(aw);
> +				break;
> +			}
> +			INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
> +			aw->vhost = vhost;
> +			aw->crq = *crq;
> +			schedule_work(&aw->async_work_s);

[Severity: High]
Does this code risk a Use-After-Free during teardown?

Since aw is queued to the global system_wq without being tracked,
there doesn't seem to be a way to flush it in ibmvfc_remove().

If ibmvfc_remove() runs concurrently with this pending work item, it calls
ibmvfc_free_mem() which unconditionally destroys the target mempool:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_free_mem() {
	...
	mempool_destroy(vhost->tgt_pool);
	...
}

This could lead to a crash when ibmvfc_process_async_work() completes and
drops its reference.

Additionally, because it is untracked on the global workqueue, it can execute
after the module is unloaded, leading to a fatal instruction page fault.

> +		} else
> +			dev_err_ratelimited(vhost->dev,
> +					    "can't offload async CRQ to work queue\n");
> +		break;
>  	default:
>  		dev_err(vhost->dev, "Unknown async event received: %lld\n", crq->event);
>  		break;
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-ibmvfc-fpin-support-v14-0-a0b063b8d825@linux.ibm.com?part=4

  reply	other threads:[~2026-10-15 15:18 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15 15:06 [PATCH v14 00/11] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 01/11] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 02/11] scsi: ibmvfc: fix potential clobbering of rc after failed irq setup Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 03/11] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation Dave Marquardt via B4 Relay
2026-10-15 15:20   ` sashiko-bot
2026-10-15 16:15     ` Dave Marquardt
2026-10-15 15:07 ` [PATCH v14 04/11] scsi: ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
2026-10-15 15:18   ` sashiko-bot [this message]
2026-10-15 21:47     ` Dave Marquardt
2026-10-15 15:07 ` [PATCH v14 05/11] scsi: ibmvfc: add NOOP command support Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 06/11] scsi: ibmvfc: add FPIN extended flag and async sub-CRQ queue handle Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 07/11] scsi: ibmvfc: extend async event handlers for async sub-CRQ events Dave Marquardt via B4 Relay
2026-10-15 15:20   ` sashiko-bot
2026-10-15 15:07 ` [PATCH v14 08/11] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ Dave Marquardt via B4 Relay
2026-10-15 15:07 ` [PATCH v14 09/11] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ Dave Marquardt via B4 Relay
2026-10-15 15:21   ` sashiko-bot
2026-10-15 15:07 ` [PATCH v14 10/11] scsi: ibmvfc: register and use asynchronous sub CRQ for events Dave Marquardt via B4 Relay
2026-10-15 15:28   ` sashiko-bot
2026-10-15 15:07 ` [PATCH v14 11/11] scsi: ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-173047@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=davemarq@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox