Devicetree
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: pankaj.gupta@oss.nxp.com
Cc: Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Frank Li <Frank.Li@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Pankaj Gupta <pankaj.gupta@nxp.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	sashiko-bot <sashiko-bot@kernel.org>
Subject: Re: [PATCH v40 5/7] firmware: imx: adds miscdev
Date: Fri, 21 Aug 2026 12:53:34 -0400	[thread overview]
Message-ID: <aoiCjv7jo7Z4Kjs6@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <20260822-imx-se-if-v40-5-74fbce2f2f95@nxp.com>

On Sat, Aug 22, 2026 at 12:06:12AM +0530, pankaj.gupta@oss.nxp.com wrote:
> From: Pankaj Gupta <pankaj.gupta@nxp.com>
>
> Adds the driver for communication interface to secure-enclave, that
> enables exchanging messages with NXP secure enclave HW IP(s)
> like EdgeLock Enclave, from:
> - User-Space Applications via character driver.
>
> ABI documentation for the NXP secure-enclave driver.
>
> User-space library using this driver:
> - i.MX Secure Enclave library:
>   -- URL: https://github.com/nxp-imx/imx-secure-enclave.git,
> - i.MX Secure Middle-Ware:
>   -- URL: https://github.com/nxp-imx/imx-smw.git
>
> Following checks are performed on the incoming msg-header,
> to block exchanging invalid arbitrary commands:
> - maximum allowed words,
> - check if command-tag & response-tag are valid
> - version,
> - command id validation check, to allow limited base-line API(s)
>   and restrict following:
>   - exchanging power management commands.
>   - reset requests.
>   - BBSM configuration requests.
>   - re-initializing the FW.
>   - RNG init
>   - CAAM resource release management
>   - SE's internal memory management.
> from user-space.
>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---

...
>
> +/*
> + * A number of ELE commands carry DMA physical addresses inside their message
> + * payload. se_val_cmd_addrs() range-checks each such address against the
> + * calling context's shared-memory window before the message reaches firmware.
> + *
> + * data[] index = message WORD index - 1, because the 4-byte se_msg_hdr is
> + * message WORD 0 and se_api_msg.data[0] is message WORD 1.
> + *
> + * struct se_cmd_addr_field describes one address embedded in the payload:
> + *   lsb_idx          - data[] index of the low 32 bits of the address
> + *   msb_idx          - data[] index of the high 32 bits, valid only when
> + *                      has_msb is set. Some base-API commands split the
> + *                      address into two words; FW-API commands do not.
> + *   has_msb          - true when the address occupies two words (lsb + msb)
> + *   flag_idx         - data[] index of the flag word that selects whether
> + *                      data[lsb_idx] is a DMA address or an integer key
> + *                      identifier; SE_CMD_ADDR_ALWAYS when the word is always
> + *                      a DMA address
> + *   flag_mask        - the selecting flag bit, already shifted to its position
> + *                      inside the 32-bit little-endian flag word
> + *   is_addr_when_set - true when the word is a DMA address if the flag bit is
> + *                      set; false when it is an address if the bit is clear
> + *                      (inverse polarity, e.g. VERIFY_SIGN OPAQUE_KEY)
> + *   size_idx         - data[] index of the word carrying the length in bytes
> + *                      of the buffer at this address. se_val_cmd_addrs() uses
> + *                      it to confirm the whole buffer [addr, addr + len) fits
> + *                      inside the shared-memory window, not just its start.
> + *                      SE_CMD_ADDR_NO_SIZE when the message carries no length
> + *                      for this buffer.
> + *   size_shift       - right shift applied to the size word before masking,
> + *                      for a length packed into the high half of a word
> + *   size_mask        - bitmask applied after the shift to extract the length
> + *                      from the message word (0xFFFFFFFF for a full 32-bit
> + *                      length, 0xFFFF for a u16, 0xFF for a u8). Used only
> + *                      when size_idx != SE_CMD_ADDR_NO_SIZE; zero otherwise.
> + *   buf_size         - literal byte count used when size_idx ==
> + *                      SE_CMD_ADDR_NO_SIZE and buf_size != 0: the whole
> + *                      buffer [addr, addr + buf_size) must fit inside the
> + *                      shared-memory window. Use this for buffers whose size
> + *                      is a firmware-defined constant not carried in the
> + *                      message. Zero means no end-bound check (start-address
> + *                      check only; see comments at each descriptor entry for
> + *                      the accepted exception rationale).
> + *                      When size_idx == SE_CMD_RCVR_ADDR_VAR_SIZE the size
> + *                      is taken from se_if_priv.cmd_rcvr_var_size, which the
> + *                      command-receiver path writes before calling
> + *                      se_val_cmd_addrs(). Use this for export-response
> + *                      buffers whose size was supplied in the preceding FW
> + *                      command and must be stored per SE interface.
> + */

Please use kdoc format to document field.

> +struct se_cmd_addr_field {
> +	u8 lsb_idx;
> +	u8 msb_idx;
> +	bool has_msb;
> +	u8 flag_idx;
> +	u32 flag_mask;
> +	bool is_addr_when_set;
> +	u8 size_idx;
> +	u8 size_shift;
> +	u32 size_mask;
> +	u32 buf_size;
> +};
> +
> +
>  static void se_if_probe_cleanup(void *plat_dev)
>  {
>  	struct platform_device *pdev = plat_dev;
> +	struct se_if_device_ctx *dev_ctx;
>  	struct device *dev = &pdev->dev;
>  	struct se_if_priv *priv;
>
> @@ -339,31 +709,148 @@ static void se_if_probe_cleanup(void *plat_dev)
>  	if (!priv)
>  		return;
>
> +	/*
> +	 * Announce teardown, then wake any in-flight waiter. going_away makes
> +	 * ele_msg_send_rcv() bail out instead of arming a new transaction and
> +	 * lets ele_msg_rcv() tell a teardown-forced completion apart from a
> +	 * real response; it must be set before complete_all().
> +	 *
> +	 * Set it under clbk_rx_lock, not se_if_cmd_lock: se_if_cmd_lock is held
> +	 * across the whole blocking transaction, so taking it here would stall
> +	 * unbind for a full receive-timeout. clbk_rx_lock is the short spinlock
> +	 * ele_msg_send_rcv() holds while arming, so this closes the lost-wakeup
> +	 * window - the sender either sees going_away and bails before arming, or
> +	 * armed first and this store (and complete_all()) is ordered after its
> +	 * reinit_completion() - and supplies the ordering the relaxed atomics do
> +	 * not.
> +	 */
> +	scoped_guard(spinlock_irqsave, &priv->waiting_rsp_clbk_hdl.clbk_rx_lock)
> +		atomic_set(&priv->going_away, 1);
> +	/*
> +	 * Wake the waiter before iterating the device-context list. It sleeps on
> +	 * this completion holding dev_ctx->fops_lock, which cleanup_dev_ctx()
> +	 * below also takes, so completing first avoids an unbind hang. Runs
> +	 * outside clbk_rx_lock; the going_away store above already orders it
> +	 * against the arming path.
> +	 */
> +	complete_all(&priv->waiting_rsp_clbk_hdl.done);
> +
> +	/*
> +	 * Mark the private device context as cleanup_done first.
> +	 * This prevents new device contexts from being created in open().
> +	 */
> +	if (priv->priv_dev_ctx) {
> +		/*
> +		 * Mark cleanup_done under fops_lock so that se_if_fops_open(),
> +		 * which checks cleanup_done while holding fops_lock, cannot
> +		 * race past this and add a new device context after teardown.
> +		 */
> +		scoped_guard(mutex, &priv->priv_dev_ctx->fops_lock)
> +			priv->priv_dev_ctx->cleanup_done = true;
> +
> +		if (priv->open_gate) {
> +			scoped_guard(mutex, &priv->open_gate->lock) {
> +				priv->open_gate->dying = true;
> +				priv->open_gate->priv = NULL;
> +			}
> +		}
> +
> +		/*
> +		 * misc_register() is deferred to the end of probe, so the
> +		 * device may have a miscdev set up but never registered if
> +		 * probe failed before se_if_misc_register(). Only deregister
> +		 * when registration actually succeeded.
> +		 */
> +		if (priv->open_gate && priv->open_gate->registered &&
> +		    priv->priv_dev_ctx->miscdev)
> +			misc_deregister(priv->priv_dev_ctx->miscdev);
> +	}
> +
> +	while (true) {
> +		dev_ctx = NULL;
> +
> +		scoped_guard(mutex, &priv->modify_lock) {
> +			if (list_empty(&priv->dev_ctx_list))
> +				goto out_done;
> +
> +			dev_ctx = list_first_entry(&priv->dev_ctx_list,
> +						   struct se_if_device_ctx, link);
> +
> +			/* pin this context so close() cannot free it under us */
> +			kref_get(&dev_ctx->refcount);
> +			dlink_dev_ctx(dev_ctx);
> +		}
> +
> +		/*
> +		 * Local cleanup outside the global lock avoids ABBA deadlock
> +		 * with paths that already take dev_ctx->fops_lock first.
> +		 */
> +		cleanup_dev_ctx(dev_ctx, false);
> +		kref_put(&dev_ctx->refcount, se_if_dev_ctx_release);
> +	}
> +out_done:
> +
> +	/*
> +	 * Drain any in-flight synchronous sender before releasing the mailbox
> +	 * channels. ele_msg_send_rcv() holds se_if_cmd_lock across the entire
> +	 * transaction, including ele_msg_send()'s mbox_send_message() on
> +	 * priv->tx_chan. going_away is checked and the transaction armed under
> +	 * clbk_rx_lock, but the mbox_send_message() itself runs after that
> +	 * spinlock is dropped, so a sender that passed the going_away check
> +	 * just before teardown set it could still be about to touch tx_chan
> +	 * when we free it here - a use-after-free in the mailbox layer.
> +	 *
> +	 * Acquire and immediately release se_if_cmd_lock as a barrier: it waits
> +	 * for such a sender to finish its transaction and drop the lock. This
> +	 * cannot stall unbind for a full receive timeout - going_away is
> +	 * already set and complete_all() has already woken any waiter, so an
> +	 * in-flight transaction only unwinds to -ENODEV before releasing the
> +	 * lock. It also cannot deadlock: the dev_ctx_list loop above has
> +	 * finished (teardown holds no se_if_cmd_lock of its own here) and a
> +	 * racing userspace close, which takes fops_lock then se_if_cmd_lock,
> +	 * bails out of ele_msg_send_rcv() with -ENODEV without waiting. After
> +	 * this barrier no sender can enter or remain inside mbox_send_message(),
> +	 * so freeing the channels below cannot race it.
> +	 */
> +	scoped_guard(mutex, &priv->se_if_cmd_lock) {
> +		;

look like doesn't work., maybe you need set busy heer, make sure no
new request queued.

	or move
free mbox_free_channel() here. and set priv->rx_chan to NULL.

> +	}
> +
> +	/*
> +	 * Free the rx mailbox channel before cancelling fw_busy_work.
> +	 * se_if_rx_callback() runs from the rx channel and can schedule
> +	 * fw_busy_work when a late response arrives. If the channel were still
> +	 * live after cancel_work_sync(), a callback could re-arm the work and
> +	 * later dereference priv after it has been freed. Releasing the rx
> +	 * channel first guarantees no further callbacks, so the subsequent
> +	 * cancel_work_sync() is final.
> +	 */
>  	if (priv->rx_chan)
>  		mbox_free_channel(priv->rx_chan);
>  	if (priv->tx_chan)
>  		mbox_free_channel(priv->tx_chan);
>
>  	/*
> -	 * Being device managed buffer, no need to free the buffer allocated
> -	 * in se probe to store encrypted IMEM.
> +	 * A timed-out synchronous command may have retained a dev_ctx through
> +	 * priv->fw_busy_dev_ctx even after the fd was closed and the context was
> +	 * removed from dev_ctx_list. If no late response arrived, release that
> +	 * retained context during driver teardown.
> +	 *
> +	 * se_clear_fw_busy() is idempotent and internally checks
> +	 * priv->fw_busy_dev_ctx under fw_busy_lock.
>  	 */
> +	se_clear_fw_busy(priv);
> +	cancel_work_sync(&priv->fw_busy_work);

Most like cancel_work_sync should be before free mbox chan.

>
>  	/*
> -	 * No need to check, if reserved memory is allocated
> -	 * before calling for its release. Or clearing the
> -	 * un-set bit.
> +	 * Being device managed buffer, no need to free the buffer allocated
> +	 * in se probe to store encrypted IMEM.
>  	 */
> -	of_reserved_mem_device_release(dev);
>
>  	dev_set_drvdata(dev, NULL);
>
> -	if (priv->priv_dev_ctx) {
> -		kfree(priv->priv_dev_ctx->devname);
> -		kfree(priv->priv_dev_ctx);
> -	}
> -
> -	kfree(priv);
> +	/* Drop the initial reference - priv will be freed when last fd closes */
> +	kref_put(&priv->refcount, se_if_priv_release);
>  }
>
>  static int se_if_probe(struct platform_device *pdev)
> @@ -386,15 +873,30 @@ static int se_if_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>
>  	priv->dev = dev;
> +	/*
> +	 * Pin the parent device for the lifetime of priv. A file descriptor may
> +	 * stay open after the device is unbound; close() then still passes
> +	 * priv->dev to dma_free_coherent()/dev_warn(). Without this reference
> +	 * the struct device could be freed while priv->dev still points at it,
> +	 * so the reference is dropped in se_if_priv_release() via put_device().
> +	 */
> +	get_device(priv->dev);
> +	kref_init(&priv->refcount);
>  	priv->if_defs = &if_node->if_defs;
>  	dev_set_drvdata(dev, priv);
>
>  	mutex_init(&priv->se_if_cmd_lock);
> +	mutex_init(&priv->modify_lock);

In probe funciton, should use devm_mutex_init()

>  	spin_lock_init(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock);
>  	spin_lock_init(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock);
>  	atomic_set(&priv->fw_busy, 0);
> +	spin_lock_init(&priv->fw_busy_lock);
> +	priv->fw_busy_dev_ctx = NULL;
> +	INIT_WORK(&priv->fw_busy_work, se_fw_busy_work);
> +
>  	init_completion(&priv->waiting_rsp_clbk_hdl.done);
>  	init_completion(&priv->cmd_receiver_clbk_hdl.done);
> +	INIT_LIST_HEAD(&priv->dev_ctx_list);
>
>  	ret = devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev);
>  	if (ret)
> @@ -460,7 +962,7 @@ static int se_if_probe(struct platform_device *pdev)
>  		load_fw->imem_mgmt = true;
>  	}
>
> -	ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx);
> +	ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx, &se_if_fops);
>  	if (ret)
>  		return dev_err_probe(dev, ret,
>  				     "Failed[0x%x] to create device contexts.",
> @@ -472,12 +974,1178 @@ static int se_if_probe(struct platform_device *pdev)
>  			return dev_err_probe(dev, ret, "Failed to fetch SoC Info.");
>  	}
>
> +	/*
> +	 * All probe-time initialization is complete; expose the
> +	 * interface to userspace last so that an open()/ioctl cannot
> +	 * race against a not-yet-initialized device.
> +	 */
> +	ret = se_if_misc_register(priv);
> +	if (ret)
> +		return ret;
> +
>  	dev_info(dev, "i.MX secure-enclave: %s0 interface to firmware, configured.",
>  		 get_se_if_name(priv->if_defs->se_if_type));
>
>  	return ret;
>  }
>
> +/*
> + * Expose the interface to userspace. Deferred until the end of probe so
> + * the device node only becomes openable after SoC info has been fetched
> + * and, on SoCs with IMEM management, the encrypted-IMEM buffer has been
> + * allocated. This prevents userspace from opening the node and issuing
> + * commands against a partially initialized interface.
> + */
> +static int se_if_misc_register(struct se_if_priv *priv)
> +{
> +	int ret;
> +
> +	ret = misc_register(priv->priv_dev_ctx->miscdev);
> +	if (ret)
> +		return dev_err_probe(priv->dev, ret,
> +				     "Failed to register misc device.");
> +
> +	priv->open_gate->registered = true;
> +
> +	return 0;
> +}
> +
> +static void se_if_priv_release(struct kref *kref)
> +{
> +	struct se_if_priv *priv = container_of(kref, struct se_if_priv, refcount);
> +
> +	/* Free priv_dev_ctx if it exists */
> +	if (priv->priv_dev_ctx) {
> +		/*
> +		 * miscdev storage belongs to open_gate, not directly to
> +		 * priv_dev_ctx. The gate should already have been detached
> +		 * from priv during teardown.
> +		 *
> +		 * Reclaim the internal context's shared memory directly here
> +		 * instead of through cleanup_dev_ctx(). Teardown already set
> +		 * cleanup_done on priv_dev_ctx, so cleanup_dev_ctx() would
> +		 * short-circuit and leak the host descriptors and the coherent
> +		 * buffer. By this point the device is fully unbound; if this
> +		 * context ever armed the firmware-busy breaker, se_clear_fw_busy()
> +		 * has already run with reclaim=false and freed the host
> +		 * descriptors, emptied the pool list and cleared
> +		 * non_secure_mem.ptr. A reclaim=true pass here is therefore both
> +		 * safe and idempotent: it releases the buffers for a normal
> +		 * context and is a no-op for the abandoned firmware-busy one.
> +		 */
> +		scoped_guard(mutex, &priv->priv_dev_ctx->fops_lock)
> +			cleanup_se_shared_mem(priv->priv_dev_ctx, true);
> +
> +		kfree(priv->priv_dev_ctx->devname);
> +		kfree(priv->priv_dev_ctx);
> +		priv->priv_dev_ctx = NULL;
> +	}
> +	/*
> +	 * No need to check, if reserved memory is allocated
> +	 * before calling for its release. Or clearing the
> +	 * un-set bit.
> +	 */
> +	of_reserved_mem_device_release(priv->dev);
> +
> +	/*
> +	 * Be defensive: if teardown did not already drop the device-owned
> +	 * gate reference for some reason, release it here.
> +	 */
> +	if (priv->open_gate) {
> +		se_if_open_gate_put(priv->open_gate);
> +		priv->open_gate = NULL;
> +	}
> +
> +	/*
> +	 * Drop the reference on priv->dev taken in se_if_probe(). The device was
> +	 * pinned so that a file descriptor closed after device unbind can still
> +	 * safely pass priv->dev to dma_free_coherent()/dev_warn().
> +	 */
> +	put_device(priv->dev);
> +
> +	/* Free any remaining resources that weren't devm-managed */
> +	kfree(priv);
> +}
> +
> +static void se_if_dev_ctx_release(struct kref *kref)
> +{
> +	struct se_if_device_ctx *dev_ctx =
> +		container_of(kref, struct se_if_device_ctx, refcount);
> +	struct se_if_priv *priv = dev_ctx->priv;
> +
> +	kfree(dev_ctx);
> +
> +	/* drop the priv reference owned by this device context */
> +	kref_put(&priv->refcount, se_if_priv_release);
> +}
> +
> +static void se_clear_fw_busy(struct se_if_priv *priv)
> +{
> +	struct se_if_device_ctx *dev_ctx = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->fw_busy_lock, flags);
> +	dev_ctx = priv->fw_busy_dev_ctx;
> +	priv->fw_busy_dev_ctx = NULL;
> +	atomic_set(&priv->fw_busy, 0);
> +	spin_unlock_irqrestore(&priv->fw_busy_lock, flags);
> +
> +	if (!dev_ctx)
> +		return;
> +
> +	/*
> +	 * The circuit breaker is cleared from two places, which need opposite
> +	 * memory-reclaim policies:
> +	 *
> +	 *   1. se_fw_busy_work(): a late firmware response actually arrived.
> +	 *      going_away is not set and the enclave has finished with the
> +	 *      buffer, so a full reclaim (reclaim=true) is safe. This includes
> +	 *      priv_dev_ctx, which has no close() path between a timeout and
> +	 *      module unload: cleanup_done is only set at unbind, so the
> +	 *      previous "else if (cleanup_done)" would permanently leak the
> +	 *      128 KB shared-memory slot for priv_dev_ctx after a late response.
> +	 *      Reclaim unconditionally here instead; for userspace contexts
> +	 *      se_dev_ctx_shared_mem_cleanup() is idempotent when pos is
> +	 *      already reset by the normal close() path.
> +	 *
> +	 *   2. se_if_probe_cleanup(): teardown. going_away is set and no
> +	 *      response has been confirmed, so the enclave may still be
> +	 *      DMA-writing into the shared buffer. Freeing it here would be a
> +	 *      DMA-after-free. Pass reclaim=false so cleanup_se_shared_mem()
> +	 *      frees only the host-side descriptors and deliberately leaks the
> +	 *      DMA buffer that the enclave might still touch.
> +	 */
> +	scoped_guard(mutex, &dev_ctx->fops_lock) {
> +		if (atomic_read(&priv->going_away)) {
> +			/*
> +			 * Fatal, but deliberately non-panic: the enclave is
> +			 * unresponsive at unbind with a transaction still in
> +			 * flight. Both the coherent staging buffer and any
> +			 * gen_pool buffers this context owns are abandoned
> +			 * (host descriptors freed, DMA-visible memory leaked)
> +			 * to avoid a DMA-after-free while the enclave may still
> +			 * be writing. Emit one headline error here rather than
> +			 * per-buffer so the count of faulted contexts is clear.
> +			 * Do not use WARN/BUG: this path is recoverable and
> +			 * panic_on_warn kernels must not be brought down by it.
> +			 */
> +			dev_err(priv->dev,
> +				"%s: FATAL: enclave stuck at unbind, DMA leaked.\n",
> +				dev_ctx->devname);
> +			cleanup_se_shared_mem(dev_ctx, false);
> +		} else {
> +			/*
> +			 * Late response arrived after going_away is clear.
> +			 * fw_busy has already been cleared atomically above;
> +			 * reclaim the shared-memory slot now. For priv_dev_ctx
> +			 * this is the only reclaim site (no close() path). For
> +			 * userspace contexts cleanup_se_shared_mem() is a
> +			 * safe no-op if the close() path already reset pos.
> +			 */
> +			cleanup_se_shared_mem(dev_ctx, true);
> +		}
> +	}
> +
> +	kref_put(&dev_ctx->refcount, se_if_dev_ctx_release);
> +}
> +
> +void unset_dev_ctx_as_command_receiver(struct se_if_device_ctx *dev_ctx)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_api_msg *old_rx_msg = NULL;
> +	struct se_clbk_handle *se_clbk_hdl;
> +	unsigned long flags;
> +
> +	lockdep_assert_held(&priv->modify_lock);
> +
> +	se_clbk_hdl = &priv->cmd_receiver_clbk_hdl;
> +
> +	if (se_clbk_hdl->dev_ctx == dev_ctx) {
> +		spin_lock_irqsave(&se_clbk_hdl->clbk_rx_lock, flags);
> +		old_rx_msg = se_clbk_hdl->rx_msg;
> +		se_clbk_hdl->dev_ctx = NULL;
> +		se_clbk_hdl->rx_msg = NULL;
> +		se_clbk_hdl->rx_msg_sz = 0;
> +		spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
> +
> +		kfree(old_rx_msg);
> +		complete_all(&se_clbk_hdl->done);
> +	}
> +}
> +
> +int set_dev_ctx_as_command_receiver(struct se_if_device_ctx *dev_ctx, bool is_ioctl)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_api_msg *new_rx_msg = NULL;
> +	struct se_clbk_handle *se_clbk_hdl;
> +	unsigned long flags;
> +
> +	se_clbk_hdl = &priv->cmd_receiver_clbk_hdl;
> +	guard(mutex)(&priv->modify_lock);
> +	if (se_clbk_hdl->dev_ctx == dev_ctx)
> +		return 0;
> +
> +	if (se_clbk_hdl->dev_ctx)
> +		return -EBUSY;
> +
> +	if (!dev_ctx->strg_hdl)
> +		return -EINVAL;
> +
> +	if (is_ioctl)
> +		return -ENOMEM;
> +
> +	if (!se_clbk_hdl->rx_msg) {
> +		new_rx_msg = kzalloc(MAX_NVM_MSG_LEN, GFP_KERNEL);
> +		if (!new_rx_msg)
> +			return -ENOMEM;
> +	}
> +	spin_lock_irqsave(&se_clbk_hdl->clbk_rx_lock, flags);
> +	if (new_rx_msg)
> +		se_clbk_hdl->rx_msg = new_rx_msg;
> +	reinit_completion(&se_clbk_hdl->done);
> +	se_clbk_hdl->rx_msg_sz = MAX_NVM_MSG_LEN;
> +	se_clbk_hdl->dev_ctx = dev_ctx;
> +	dev_ctx->rcv_msg_timeout_jiffies = MAX_SCHEDULE_TIMEOUT;
> +	spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
> +
> +	return 0;
> +}
> +
> +static void dlink_dev_ctx(struct se_if_device_ctx *dev_ctx)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +
> +	unset_dev_ctx_as_command_receiver(dev_ctx);
> +
> +	if (!list_empty(&dev_ctx->link)) {
> +		list_del_init(&dev_ctx->link);
> +		priv->active_devctx_count--;
> +	}
> +}
> +
> +bool se_is_fw_busy_ctx(struct se_if_device_ctx *dev_ctx)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	unsigned long flags;
> +	bool match;
> +
> +	spin_lock_irqsave(&priv->fw_busy_lock, flags);
> +	match = priv->fw_busy_dev_ctx == dev_ctx;
> +	spin_unlock_irqrestore(&priv->fw_busy_lock, flags);
> +
> +	return match;
> +}
> +
> +static void cleanup_dev_ctx(struct se_if_device_ctx *dev_ctx, bool is_fclose)
> +{
> +	bool already_done;
> +
> +	scoped_guard(mutex, &dev_ctx->fops_lock) {
> +		already_done = dev_ctx->cleanup_done;
> +		if (!already_done) {
> +			/*
> +			 * Ask FW to drop this context's session and storage so
> +			 * the kernel and FW stay in sync. Done here, under this
> +			 * context's fops_lock only (not the global modify_lock),
> +			 * because both close requests block on a firmware
> +			 * round-trip; issuing them while modify_lock was held
> +			 * would stall every other context for the FW timeout.
> +			 *
> +			 * Skip the round-trips once the FW path is marked busy.
> +			 * fw_busy is armed when a synchronous transaction times
> +			 * out; while it is set ele_msg_send_rcv() rejects further
> +			 * commands with -EBUSY without waiting. It is only cleared
> +			 * by se_clear_fw_busy(), which during unbind runs once
> +			 * after this loop (or earlier from fw_busy_work only if a
> +			 * genuine late FW response arrives). On a hung FW no late
> +			 * response comes, so the breaker stays set for the rest of
> +			 * the loop and the remaining closes would just return
> +			 * -EBUSY and log spurious "failed to close" errors. Skip
> +			 * them and emit a single warning instead.
> +			 */
> +			if (atomic_read(&dev_ctx->priv->fw_busy)) {
> +				if (dev_ctx->strg_hdl || dev_ctx->sess_hdl)
> +					dev_warn(dev_ctx->priv->dev,
> +						 "%s: skipping session/storage close, FW is busy\n",
> +						 dev_ctx->devname);
> +			} else {
> +				/*
> +				 * Pick the context that carries the close messages.
> +				 *
> +				 * fclose (is_fclose): a userspace close() may race
> +				 * driver unbind. Send on the caller's own dev_ctx so
> +				 * ele_msg_send_rcv()'s going_away check rejects the
> +				 * transmission with -ENODEV if unbind has begun (and
> +				 * may have freed priv->tx_chan), instead of touching a
> +				 * freed mailbox channel.
> +				 *
> +				 * Teardown (!is_fclose): going_away is already set, but
> +				 * priv->tx_chan is still live at this point in
> +				 * se_if_probe_cleanup(). Send on priv_dev_ctx, the only
> +				 * context ele_msg_send_rcv() lets through going_away for
> +				 * teardown-close messages, so the kernel can still
> +				 * resynchronise session/storage state with FW.
> +				 */
> +				struct se_if_device_ctx *tx_ctx = is_fclose ? dev_ctx :
> +							dev_ctx->priv->priv_dev_ctx;
> +
> +				if (dev_ctx->strg_hdl && se_close_storage(tx_ctx,
> +									  dev_ctx->strg_hdl))
> +					dev_err(dev_ctx->priv->dev, "failed to close storage.\n");
> +				if (dev_ctx->sess_hdl && se_close_session(tx_ctx,
> +									  dev_ctx->sess_hdl))
> +					dev_err(dev_ctx->priv->dev, "failed to close session.\n");
> +			}
> +			/*
> +			 * fw_busy is caused by one timed-out synchronous transaction.
> +			 * Only that transaction's dev_ctx may still have coherent
> +			 * memory referenced by FW. Do not skip cleanup for unrelated
> +			 * contexts while fw_busy is set.
> +			 */
> +			if (se_is_fw_busy_ctx(dev_ctx))
> +				dev_warn(dev_ctx->priv->dev,
> +					 "%s: deferring shared memory cleanup while FW is busy\n",
> +					 dev_ctx->devname);
> +			else
> +				cleanup_se_shared_mem(dev_ctx, true);
> +
> +			kfree(dev_ctx->devname);
> +			dev_ctx->devname = NULL;
> +			dev_ctx->cleanup_done = true;
> +		}
> +	}
> +
> +	if (is_fclose)
> +		kref_put(&dev_ctx->refcount, se_if_dev_ctx_release);
> +}
> +
> +static void dlink_n_cleanup_dev_ctx(struct se_if_device_ctx *dev_ctx, bool is_fclose)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +
> +	if (is_fclose) {
> +		scoped_guard(mutex, &priv->modify_lock)
> +			dlink_dev_ctx(dev_ctx);
> +	}
> +
> +	cleanup_dev_ctx(dev_ctx, is_fclose);
> +}
> +
> +static int init_device_context(struct se_if_priv *priv, int ch_id,
> +			       struct se_if_device_ctx **new_dev_ctx)
> +{
> +	struct se_if_device_ctx *dev_ctx;
> +	int ret = 0;
> +
> +	dev_ctx = kzalloc_obj(*dev_ctx, GFP_KERNEL);
> +
> +	if (!dev_ctx)
> +		return -ENOMEM;
> +
> +	dev_ctx->devname = kasprintf(GFP_KERNEL, "%s0_ch%d",
> +				     get_se_if_name(priv->if_defs->se_if_type),
> +				     ch_id);
> +	if (!dev_ctx->devname) {
> +		kfree(dev_ctx);
> +		return -ENOMEM;
> +	}
> +
> +	mutex_init(&dev_ctx->fops_lock);
> +	kref_init(&dev_ctx->refcount);
> +	dev_ctx->priv = priv;
> +	dev_ctx->cleanup_done = false;
> +	INIT_LIST_HEAD(&dev_ctx->link);
> +	set_se_rcv_msg_timeout(dev_ctx, SE_RCV_MSG_LONG_TIMEOUT_MS);
> +	*new_dev_ctx = dev_ctx;
> +
> +	ret = init_se_shared_mem(dev_ctx);
> +	if (ret < 0) {
> +		kfree(dev_ctx->devname);
> +		kfree(dev_ctx);
> +		*new_dev_ctx = NULL;
> +
> +		return ret;
> +	}
> +
> +	/* Take a reference to priv for this device context */
> +	kref_get(&priv->refcount);
> +
> +	scoped_guard(mutex, &priv->modify_lock) {
> +		list_add_tail(&dev_ctx->link, &priv->dev_ctx_list);
> +		priv->active_devctx_count++;
> +	}
> +
> +	return ret;
> +}
> +
> +static int se_ioctl_cmd_snd_rcv_cleanup(struct se_if_device_ctx *dev_ctx, void __user *uarg,
> +					struct se_ioctl_cmd_snd_rcv_rsp_info *cmd_snd_rcv_rsp_info)
> +{
> +	/* shared memory is allocated before this IOCTL */
> +	se_dev_ctx_shared_mem_cleanup(dev_ctx);
> +
> +	if (cmd_snd_rcv_rsp_info->rx_buf_sz &&
> +	    copy_to_user(uarg, cmd_snd_rcv_rsp_info, sizeof(*cmd_snd_rcv_rsp_info))) {
> +		dev_err(dev_ctx->priv->dev, "%s: Failed to copy cmd_snd_rcv_rsp_info to user.",
> +			dev_ctx->devname);
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx *dev_ctx,
> +					    void __user *uarg)
> +{
> +	struct se_ioctl_cmd_snd_rcv_rsp_info cmd_snd_rcv_rsp_info = {0};
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	int rsp_status_err = 0;
> +	int cleanup_err = 0;
> +	int err = 0;
> +
> +	if (copy_from_user(&cmd_snd_rcv_rsp_info, uarg,
> +			   sizeof(cmd_snd_rcv_rsp_info))) {
> +		dev_err(priv->dev,
> +			"%s: Failed to copy cmd_snd_rcv_rsp_info from user.",
> +			dev_ctx->devname);
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return -EFAULT;
> +	}
> +
> +	if (cmd_snd_rcv_rsp_info.tx_buf_sz < SE_MU_HDR_SZ ||
> +	    cmd_snd_rcv_rsp_info.tx_buf_sz > MAX_ALLOWED_TX_MSG_SZ) {
> +		dev_err(priv->dev, "%s: User buffer too small/large(%d < %d)",
> +			dev_ctx->devname, cmd_snd_rcv_rsp_info.tx_buf_sz,
> +			cmd_snd_rcv_rsp_info.tx_buf_sz < SE_MU_HDR_SZ ? SE_MU_HDR_SZ :
> +								MAX_ALLOWED_TX_MSG_SZ);
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return -ENOSPC;
> +	}
> +
> +	struct se_api_msg *tx_msg __free(kfree) =
> +		memdup_user(u64_to_user_ptr(cmd_snd_rcv_rsp_info.tx_buf),
> +			    cmd_snd_rcv_rsp_info.tx_buf_sz);
> +	if (IS_ERR(tx_msg)) {
> +		err = PTR_ERR(tx_msg);
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return err;
> +	}
> +
> +	err = se_chk_tx_cmd_msg_hdr(dev_ctx, &tx_msg->header,
> +				    cmd_snd_rcv_rsp_info.tx_buf_sz,
> +				    cmd_snd_rcv_rsp_info.rx_buf_sz);
> +	if (err) {
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return err;
> +	}
> +
> +	if (cmd_snd_rcv_rsp_info.rx_buf_sz < SE_MU_HDR_SZ ||
> +	    cmd_snd_rcv_rsp_info.rx_buf_sz > MAX_ALLOWED_RX_MSG_SZ) {
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return -EINVAL;
> +	}
> +
> +	if (tx_msg->header.tag != priv->if_defs->cmd_tag) {
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return -EINVAL;
> +	}
> +
> +	if (tx_msg->header.ver == priv->if_defs->fw_api_ver &&
> +	    get_load_fw_instance(priv)->is_fw_tobe_loaded) {
> +		err = se_load_firmware(priv);
> +		if (err) {
> +			dev_err(priv->dev, "Could not send msg as FW is not loaded.");
> +			se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +			return -EPERM;
> +		}
> +	}
> +
> +	struct se_api_msg *rx_msg __free(kfree) =
> +		kzalloc(cmd_snd_rcv_rsp_info.rx_buf_sz, GFP_KERNEL);
> +	if (!rx_msg) {
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +		return -ENOMEM;
> +	}
> +
> +	err = ele_msg_send_rcv(dev_ctx, tx_msg, cmd_snd_rcv_rsp_info.tx_buf_sz,
> +			       rx_msg, cmd_snd_rcv_rsp_info.rx_buf_sz);
> +	if (err < 0) {
> +		/*
> +		 * -ERESTARTSYS here means the wait was interrupted by a signal
> +		 * after the command had already been handed to - and executed
> +		 * by - the firmware, with its response delivered into rx_msg
> +		 * (ele_msg_send_rcv() converts only a positive, i.e. successfully
> +		 * received, result to -ERESTARTSYS). If that response carried a
> +		 * freshly allocated session/storage handle, record it now via
> +		 * fw_api_specific_ops(): the handle is already live in firmware,
> +		 * so leaving it untracked would stop cleanup_dev_ctx() from ever
> +		 * closing it and leak the firmware resource. Validate the
> +		 * delivered response first, using its own declared length bounded
> +		 * by the caller's buffer, so a truncated or malformed reply is
> +		 * not acted upon.
> +		 */
> +		if (err == -ERESTARTSYS) {
> +			u32 rsp_sz = rx_msg->header.size << 2;
> +
> +			if (rsp_sz && rsp_sz <= cmd_snd_rcv_rsp_info.rx_buf_sz &&
> +			    !se_val_rsp_hdr_n_status(priv, rx_msg,
> +						     tx_msg->header.command, rsp_sz,
> +						     tx_msg->header.ver ==
> +						     priv->if_defs->base_api_ver)) {
> +				se_dev_ctx_cpy_out_data(dev_ctx);
> +				fw_api_specific_ops(dev_ctx, rx_msg, true);
> +			}
> +			/*
> +			 * Returning -ERESTARTSYS would let the VFS transparently restart
> +			 * the ioctl, which would re-run the command with the just
> +			 * cleaned-up (zeroed) shared input buffers. Report -EINTR instead
> +			 * so the syscall is not auto-restarted; userspace enters its
> +			 * signal handler and can decide whether to reissue the command.
> +			 */
> +			err = -EINTR;
> +		}
> +
> +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +
> +		return err;
> +	}
> +
> +	/*
> +	 * ele_msg_send_rcv() returns a positive received-message size on
> +	 * success. Returning that raw size as the ioctl result would make a
> +	 * successful transaction look like a positive (non-zero) return value
> +	 * to userspace. Record the actual received size in rx_buf_sz for the
> +	 * response copied back to userspace, then normalise err to 0 so the
> +	 * ioctl reports plain success; the firmware status is conveyed to
> +	 * userspace inside the response buffer itself.
> +	 */
> +	cmd_snd_rcv_rsp_info.rx_buf_sz = err;
> +	err = 0;
> +
> +	dev_dbg(priv->dev, "%s: %s %s.", dev_ctx->devname, __func__,
> +		"message received, start transmit to user");
> +
> +	rsp_status_err =
> +		se_val_rsp_hdr_n_status(priv, rx_msg, tx_msg->header.command,
> +					cmd_snd_rcv_rsp_info.rx_buf_sz,
> +					tx_msg->header.ver == priv->if_defs->base_api_ver);
> +
> +	if (!rsp_status_err) {
> +		/*
> +		 * The response is well formed and fully fits the caller's
> +		 * buffer, so any FW-allocated session/storage handle it carries
> +		 * (data[1]) has been delivered. Record it now, before the
> +		 * copy-out steps below. The FW has already committed the handle;
> +		 * running fw_api_specific_ops() only after a successful
> +		 * se_dev_ctx_cpy_out_data()/copy_to_user() would leave the
> +		 * handle untracked - and so never closed on teardown, leaking it
> +		 * in FW - whenever the caller supplied a bad output pointer.
> +		 */
> +		fw_api_specific_ops(dev_ctx, rx_msg, false);
> +
> +		err = se_dev_ctx_cpy_out_data(dev_ctx);
> +		if (err < 0) {
> +			se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +			return err;
> +		}
> +	}
> +
> +	/* Copy data from the buffer */
> +	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4, rx_msg,
> +			     cmd_snd_rcv_rsp_info.rx_buf_sz, false);
> +
> +	if (copy_to_user(u64_to_user_ptr(cmd_snd_rcv_rsp_info.rx_buf), rx_msg,
> +			 cmd_snd_rcv_rsp_info.rx_buf_sz)) {
> +		dev_err(priv->dev, "%s: Failed to copy to user.", dev_ctx->devname);
> +		err = -EFAULT;
> +	}
> +
> +	cleanup_err = se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
> +
> +	if (cleanup_err && !err)
> +		err = cleanup_err;
> +
> +	return err;
> +}
> +
> +static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
> +				void __user *uarg)
> +{
> +	struct se_if_priv *priv = dev_ctx->priv;
> +	struct se_ioctl_get_if_info if_info;
> +	struct se_if_node *if_node;
> +	int err = 0;
> +
> +	if_node = container_of(priv->if_defs, typeof(*if_node), if_defs);
> +
> +	if_info.se_if_id = 0;
> +	if_info.interrupt_idx = 0;
> +	if_info.tz = 0;
> +	if_info.did = 0;
> +	if_info.cmd_tag = priv->if_defs->cmd_tag;
> +	if_info.rsp_tag = priv->if_defs->rsp_tag;
> +	if_info.success_tag = priv->if_defs->success_tag;
> +	if_info.base_api_ver = priv->if_defs->base_api_ver;
> +	if_info.fw_api_ver = priv->if_defs->fw_api_ver;
> +
> +	dev_dbg(priv->dev, "%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x].",
> +		dev_ctx->devname, if_info.se_if_id, if_info.interrupt_idx, if_info.tz,
> +		if_info.did);
> +
> +	if (copy_to_user(uarg, &if_info, sizeof(if_info))) {
> +		dev_err(priv->dev, "%s: Failed to copy mu info to user.",
> +			dev_ctx->devname);
> +		err = -EFAULT;
> +	}
> +
> +	return err;
> +}
> +
> +static void rollback_shared_mem_pos(struct se_if_device_ctx *dev_ctx, u32 length)
> +{
> +	struct se_shared_mem *shared_mem = NULL;
> +
> +	shared_mem = &dev_ctx->se_shared_mem_mgmt.non_secure_mem;
> +
> +	if (WARN_ON_ONCE(length > shared_mem->pos)) {
> +		shared_mem->pos = 0;
> +		return;
> +	}
> +
> +	shared_mem->pos -= length;
> +}
> +
> +int get_shared_mem_slot(struct se_if_device_ctx *dev_ctx,
> +			u32 *length, dma_addr_t *ele_dma_addr, void **ptr)
> +{
> +	struct se_shared_mem *shared_mem = NULL;
> +	bool is_fw_busy_dev_ctx;
> +	size_t aligned_len = 0;
> +	u32 pos;
> +
> +	/*
> +	 * If this context is the one that caused a firmware timeout the shared
> +	 * DMA buffers may still be actively read/written by the firmware.
> +	 */
> +	is_fw_busy_dev_ctx = se_is_fw_busy_ctx(dev_ctx);
> +	if (is_fw_busy_dev_ctx)
> +		return -EBUSY;
> +
> +	aligned_len = round_up((size_t)*length, 8);
> +	if (aligned_len < *length) {
> +		dev_err(dev_ctx->priv->dev, "%s: Invalid buffer length.",
> +			dev_ctx->devname);
> +		return -EINVAL;
> +	}
> +
> +	/* No specific requirement for this buffer. */
> +	shared_mem = &dev_ctx->se_shared_mem_mgmt.non_secure_mem;
> +
> +	/* Check there is enough space in the shared memory. */
> +	dev_dbg(dev_ctx->priv->dev, "%s: req_size = %zd, max_size= %d, curr_pos = %d",
> +		dev_ctx->devname, aligned_len, shared_mem->size,
> +		shared_mem->pos);
> +
> +	if (shared_mem->size < shared_mem->pos ||
> +	    aligned_len > (shared_mem->size - shared_mem->pos)) {
> +		dev_err(dev_ctx->priv->dev, "%s: Not enough space in shared memory.",
> +			dev_ctx->devname);
> +		return -ENOMEM;
> +	}
> +
> +	/* Allocate space in shared memory. 8 bytes aligned. */
> +	pos = shared_mem->pos;
> +	shared_mem->pos += aligned_len;
> +	*ele_dma_addr = (u64)shared_mem->dma_addr + pos;
> +	*ptr = shared_mem->ptr + pos;
> +	*length = aligned_len;
> +
> +	memset(shared_mem->ptr + pos, 0, aligned_len);
> +
> +	return 0;
> +}
> +
> +/*
> + * Copy a buffer of data to/from the user and return the address to use in
> + * messages
> + */
> +static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
> +					void __user *uarg)
> +{
> +	struct se_ioctl_setup_iobuf io = {0};
> +	struct se_buf_desc *b_desc = NULL;
> +	void *dma_buf_ptr = NULL;
> +	dma_addr_t ele_dma_addr;
> +	u32 aligned_len = 0;
> +	int err = 0;
> +
> +	if (copy_from_user(&io, uarg, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev, "%s: Failed copy iobuf config from user.",
> +			dev_ctx->devname);
> +		return -EFAULT;
> +	}
> +
> +	dev_dbg(dev_ctx->priv->dev, "%s: io [buf: %p(%d) flag: %x].", dev_ctx->devname,
> +		u64_to_user_ptr(io.user_buf), io.length, io.flags);
> +
> +	if (io.length == 0 || !io.user_buf) {
> +		/*
> +		 * Accept NULL pointers since some buffers are optional
> +		 * in FW commands. In this case we should return 0 as
> +		 * pointer to be embedded into the message.
> +		 * Skip all data copy part of code below.
> +		 */
> +		io.ele_addr = 0;
> +		goto copy;
> +	}
> +
> +	aligned_len = io.length;
> +	err = get_shared_mem_slot(dev_ctx, &aligned_len, &ele_dma_addr, &dma_buf_ptr);
> +	if (err)
> +		return err;
> +
> +	io.ele_addr = ele_dma_addr;
> +	if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
> +	    (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
> +		/*
> +		 * buffer is input:
> +		 * copy data from user space to this allocated buffer.
> +		 */
> +		if (copy_from_user(dma_buf_ptr, u64_to_user_ptr(io.user_buf),
> +				   io.length)) {
> +			dev_err(dev_ctx->priv->dev,
> +				"%s: Failed copy data to shared memory.",
> +				dev_ctx->devname);
> +			err = -EFAULT;
> +			goto rollback;
> +		}
> +	}
> +
> +	b_desc = add_b_desc_to_pending_list(dma_buf_ptr, &io, dev_ctx);
> +	if (IS_ERR(b_desc)) {
> +		err = PTR_ERR(b_desc);
> +		dev_err(dev_ctx->priv->dev, "%s: Failed to allocate/link b_desc.",
> +			dev_ctx->devname);
> +		goto rollback;
> +	}
> +
> +copy:
> +	/* Provide the EdgeLock Enclave address to user space only if success.*/
> +	if (copy_to_user(uarg, &io, sizeof(io))) {
> +		dev_err(dev_ctx->priv->dev, "%s: Failed to copy iobuff setup to user.",
> +			dev_ctx->devname);
> +		err = -EFAULT;
> +		goto rollback;
> +	}
> +	return err;
> +
> +rollback:
> +	if (!IS_ERR_OR_NULL(b_desc)) {
> +		list_del(&b_desc->link);
> +		kfree(b_desc);
> +	}
> +
> +	if (dma_buf_ptr && aligned_len) {
> +		memset(dma_buf_ptr, 0, aligned_len);
> +		rollback_shared_mem_pos(dev_ctx, aligned_len);
> +	}
> +
> +	return err;
> +}
> +
> +/* IOCTL to provide SoC information */
> +static int se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
> +					    void __user *uarg)
> +{
> +	struct se_ioctl_get_soc_info soc_info;
> +	int err = -EINVAL;
> +
> +	soc_info.soc_id = get_se_soc_id(dev_ctx->priv);
> +	soc_info.soc_rev = var_se_info.soc_rev;
> +
> +	err = copy_to_user(uarg, (u8 *)(&soc_info), sizeof(soc_info));
> +	if (err) {
> +		dev_err(dev_ctx->priv->dev, "%s: Failed to copy soc info to user.",
> +			dev_ctx->devname);
> +		err = -EFAULT;
> +	}
> +
> +	return err;
> +}
> +
> +/*
> + * File operations for user-space
> + */
> +
> +/* Write a message to the MU. */
> +static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
> +				size_t size, loff_t *ppos)
> +{
> +	struct se_if_device_ctx *dev_ctx = fp->private_data;
> +	struct se_if_priv *priv;
> +	int err;
> +
> +	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &dev_ctx->fops_lock) {
> +		if (dev_ctx->cleanup_done)
> +			return -ENODEV;
> +
> +		priv = dev_ctx->priv;
> +
> +		dev_dbg(priv->dev, "%s: write from buf (%p)%zu, ppos=%lld.", dev_ctx->devname,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +		if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
> +			se_dev_ctx_shared_mem_cleanup(dev_ctx);
> +			return -EINVAL;
> +		}
> +
> +		if (size < SE_MU_HDR_SZ || size > MAX_ALLOWED_TX_MSG_SZ) {
> +			dev_err(priv->dev, "%s: User buffer too small/large(%zu < %d)",
> +				dev_ctx->devname, size,
> +				size < SE_MU_HDR_SZ ? SE_MU_HDR_SZ :
> +								MAX_ALLOWED_TX_MSG_SZ);
> +			return -ENOSPC;
> +		}
> +
> +		struct se_api_msg *tx_msg __free(kfree) = memdup_user(buf, size);
> +		if (IS_ERR(tx_msg))
> +			return PTR_ERR(tx_msg);
> +
> +		err = se_chk_tx_rsp_msg_hdr(dev_ctx, &tx_msg->header, size);
> +		if (err)
> +			return err;
> +
> +		print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
> +				     tx_msg, size, false);
> +
> +		err = ele_msg_send(dev_ctx, tx_msg, size);
> +
> +		return err;
> +	}
> +}
> +
> +/*
> + * Read a message from the MU.
> + * Blocking until a message is available.
> + */
> +static ssize_t se_if_fops_read(struct file *fp, char __user *buf, size_t size,
> +			       loff_t *ppos)
> +{
> +	struct se_if_device_ctx *dev_ctx = fp->private_data;
> +	u8 rx_msg_snap[MAX_NVM_MSG_LEN] = {};
> +	struct se_if_priv *priv;
> +	unsigned long flags;
> +	size_t copy_len;
> +	int err;
> +
> +	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &dev_ctx->fops_lock) {
> +		priv = dev_ctx->priv;
> +
> +		if (dev_ctx->cleanup_done)
> +			return -ENODEV;
> +
> +		dev_dbg(priv->dev, "%s: read to buf %p(%zu), ppos=%lld.", dev_ctx->devname,
> +			buf, size, ((ppos) ? *ppos : 0));
> +
> +		mutex_lock(&priv->modify_lock);
> +		if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
> +			mutex_unlock(&priv->modify_lock);
> +			se_dev_ctx_shared_mem_cleanup(dev_ctx);
> +			return -EINVAL;
> +		}
> +		mutex_unlock(&priv->modify_lock);
> +	}
> +
> +	err = ele_msg_rcv(dev_ctx, &priv->cmd_receiver_clbk_hdl);
> +	if (err < 0) {
> +		if (err != -ERESTARTSYS)
> +			dev_err(priv->dev,
> +				"%s: Er[0x%x]: Signal Interrupted. Current act-dev-ctx count: %d.",
> +				dev_ctx->devname, err, dev_ctx->priv->active_devctx_count);
> +		return err;
> +	}
> +
> +	/*
> +	 * Reacquire fops_lock before touching any dev_ctx state (pending lists,
> +	 * rx_msg) after the blocking wait. fops_lock was dropped before calling
> +	 * ele_msg_rcv(). If cleanup_dev_ctx() ran concurrently it could have
> +	 * freed the DMA buffers and the pending lists, leading to UAF and list
> +	 * corruption. Re-checking cleanup_done under fops_lock prevents that.
> +	 */
> +	mutex_lock(&dev_ctx->fops_lock);
> +
> +	if (dev_ctx->cleanup_done) {
> +		mutex_unlock(&dev_ctx->fops_lock);
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * Snapshot the full rx_msg buffer under modify_lock + clbk_rx_lock.
> +	 * Always copy the full received message (not just copy_len bytes) so
> +	 * fw_api_specific_ops() reads complete data words. copy_len is the
> +	 * amount the caller asked for and is used only for copy_to_user().
> +	 * fw_api_specific_ops() must be called OUTSIDE modify_lock: for
> +	 * ELE_STORAGE_OPEN_REQ it calls set_dev_ctx_as_command_receiver(),
> +	 * which takes modify_lock itself. Calling it while modify_lock is
> +	 * already held would deadlock.
> +	 */
> +	scoped_guard(mutex, &priv->modify_lock) {
> +		spin_lock_irqsave(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> +		if (priv->cmd_receiver_clbk_hdl.dev_ctx != dev_ctx ||
> +		    !priv->cmd_receiver_clbk_hdl.rx_msg ||
> +		    !priv->cmd_receiver_clbk_hdl.rx_msg_sz) {
> +			spin_unlock_irqrestore(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> +			mutex_unlock(&dev_ctx->fops_lock);
> +			return -ENODEV;
> +		}
> +		/*
> +		 * Snapshot the whole received message, not just copy_len bytes.
> +		 * fw_api_specific_ops() reads data words (e.g. strg_hdl at
> +		 * data[1]) that may lie beyond the userspace read() size.
> +		 * Truncating the copy here would zero-pad those words and cause
> +		 * fw_api_specific_ops() to record a zero handle, losing it.
> +		 */
> +		copy_len = min(size, (size_t)priv->cmd_receiver_clbk_hdl.rx_msg_sz);
> +		memcpy(rx_msg_snap, priv->cmd_receiver_clbk_hdl.rx_msg,
> +		       priv->cmd_receiver_clbk_hdl.rx_msg_sz);
> +		priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
> +		spin_unlock_irqrestore(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock, flags);
> +
> +		/* We may need to copy the output data to user before
> +		 * delivering the completion message.
> +		 */
> +		err = se_dev_ctx_cpy_out_data(dev_ctx);
> +		if (err < 0) {
> +			se_dev_ctx_shared_mem_cleanup(dev_ctx);
> +			mutex_unlock(&dev_ctx->fops_lock);
> +			return err;
> +		}
> +	}
> +
> +	/* fw_api_specific_ops() runs outside modify_lock; see comment above. */
> +	print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
> +			     rx_msg_snap, copy_len, false);
> +
> +	cmd_receiver_specific_ops(dev_ctx, (struct se_api_msg *)rx_msg_snap);
> +	err = copy_len;
> +	if (copy_to_user(buf, rx_msg_snap, copy_len))
> +		err = -EFAULT;
> +
> +	se_dev_ctx_shared_mem_cleanup(dev_ctx);
> +	mutex_unlock(&dev_ctx->fops_lock);
> +
> +	return err;
> +}
> +
> +/* Open a character device. */
> +static int se_if_fops_open(struct inode *nd, struct file *fp)
> +{
> +	struct miscdevice *miscdev = fp->private_data;
> +	struct se_if_open_gate *gate;
> +	struct se_if_device_ctx *misc_dev_ctx;
> +	struct se_if_device_ctx *dev_ctx;
> +	struct se_if_priv *priv;
> +	int err = 0;
> +
> +	gate = container_of(miscdev, struct se_if_open_gate, miscdev);
> +
> +	if (!se_if_open_gate_get(gate))
> +		return -ENODEV;
> +
> +	if (mutex_lock_interruptible(&gate->lock)) {
> +		se_if_open_gate_put(gate);
> +		return -ERESTARTSYS;

		err = -ERESTARTSYS;
		then goto
> +	}
> +
> +	if (gate->dying || !gate->priv ||
> +	    !kref_get_unless_zero(&gate->priv->refcount)) {
> +		err = -ENODEV;
> +		goto out_unlock_gate;
> +	}
> +
> +	priv = gate->priv;
> +	mutex_unlock(&gate->lock);
> +
> +	misc_dev_ctx = priv->priv_dev_ctx;
> +
> +	if (mutex_lock_interruptible(&misc_dev_ctx->fops_lock)) {
> +		err = -ERESTARTSYS;
> +		goto out_put_priv;
> +	}

Is ACQUIRE(mutex_intr, l)(&misc_dev_ctx->fops_lock); make code simpler?

> +
> +	if (misc_dev_ctx->cleanup_done) {
> +		err = -ENODEV;
> +		goto out_unlock_misc;
> +	}
> +
> +	priv->dev_ctx_mono_count++;
> +	err = init_device_context(priv, priv->dev_ctx_mono_count, &dev_ctx);
> +	if (err) {
> +		dev_err(priv->dev, "Failed[0x%x] to create dev-ctx.", err);
> +		goto out_unlock_misc;
> +	}
> +
> +	fp->private_data = dev_ctx;
> +
> +out_unlock_misc:
> +	mutex_unlock(&misc_dev_ctx->fops_lock);
> +out_put_priv:
> +	kref_put(&priv->refcount, se_if_priv_release);
> +	se_if_open_gate_put(gate);
> +	return err;


> +out_unlock_gate:
> +	mutex_unlock(&gate->lock);
> +	se_if_open_gate_put(gate);
> +	return err;

why need two set error branch.

Frank

  parent reply	other threads:[~2026-08-21 16:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21 18:36 [PATCH v40 0/7] firmware: imx: driver for NXP secure-enclave pankaj.gupta
2026-08-21 18:36 ` [PATCH v40 1/7] Documentation/firmware: add imx/se to other_interfaces pankaj.gupta
2026-08-21 18:36 ` [PATCH v40 2/7] dt-bindings: arm: fsl: add imx-se-fw binding doc pankaj.gupta
2026-08-21 18:36 ` [PATCH v40 3/7] firmware: imx: add driver for NXP EdgeLock Enclave pankaj.gupta
2026-08-21 13:21   ` sashiko-bot
2026-08-21 16:15   ` Frank Li
2026-08-21 18:36 ` [PATCH v40 4/7] firmware: imx: device context dedicated to priv pankaj.gupta
2026-08-21 15:56   ` Frank Li
2026-08-21 18:36 ` [PATCH v40 5/7] firmware: imx: adds miscdev pankaj.gupta
2026-08-21 13:23   ` sashiko-bot
2026-08-21 15:13     ` Pankaj Gupta (OSS)
2026-08-21 15:17       ` Frank Li
2026-08-21 16:53   ` Frank Li [this message]
2026-08-21 18:36 ` [PATCH v40 6/7] arm64: dts: imx8ulp: add secure enclave node pankaj.gupta
2026-08-21 18:36 ` [PATCH v40 7/7] arm64: dts: imx8ulp: add reserved memory for EdgeLock Enclave pankaj.gupta

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=aoiCjv7jo7Z4Kjs6@lizhi-Precision-Tower-5810 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@nxp.com \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pankaj.gupta@nxp.com \
    --cc=pankaj.gupta@oss.nxp.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=sashiko-bot@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /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