RE: [PATCH v32 5/7] firmware: imx: adds miscdev

Thanks for the thorough review of v32 5/7. Eight [High] comments; my
disposition is below. Five point at issues and are FIXED in the
follow-up commit; three are NO-CHANGE (two intended-by-design, one false
positive). The three NO-CHANGE items - and in particular the -EINTR one
that has been raised on several revisions - are explained in full so the
rationale is on the record and does not need to be re-litigated each round.

> - [High] Implicit padding in `struct se_ioctl_cmd_snd_rcv_rsp_info` causes an
> ABI mismatch between 32-bit userspace and 64-bit kernel.
Fixed.

> - [High] The driver returns -EBUSY when file operations are interrupted by a
> signal, violating standard POSIX syscall semantics.
Fixed.

> - [High] If a signal interrupts `se_if_fops_read()` after it successfully receives a
> message, the message is stranded and the firmware protocol permanently
> deadlocks.
Fixed.

> - [High] A successful firmware transaction is reported to userspace as
> interrupted (`-EINTR`) if a signal is pending, discarding the successful response
> payload.
No Changes. By Design.

> - [High] A TOCTOU race between `ele_msg_send_rcv` and
> `se_if_probe_cleanup` causes a lost wakeup, resulting in a 1.3-hour unbind
> hang.
Fixed.

> - [High] Kernel crash in the error path of `init_misc_device_context` due to
> accessing an uninitialized `dev_ctx->priv` pointer.
Fixed.

> - [High] Unbounded DMA memory allocation in the device `open` path allows
> unprivileged users to exhaust contiguous DMA memory.
No change. Bounded pool.

> - [High] The dynamically allocated `se_if_open_gate` structure can be accessed
> after it is freed if a concurrent `open()` races with device teardown.
Not an issue. False positive.


> > +++ b/drivers/firmware/imx/ele_common.c
> [ ... ]
> > @@ -188,10 +258,21 @@ int ele_msg_send_rcv(struct se_if_device_ctx
> > *dev_ctx, void *tx_msg,
> >
> >  	guard(mutex)(&priv->se_if_cmd_lock);
> >
> > +	/*
> > +	 * Teardown has begun: do not arm a new transaction. A thread that
> was
> > +	 * blocked on se_if_cmd_lock while se_if_probe_cleanup() ran its
> > +	 * complete_all() would otherwise miss that wake-up, re-arm the
> > +	 * completion below and wait the full timeout, while unbind blocks
> > +	 * indefinitely on this thread's fops_lock. Bail out instead.
> > +	 */
> > +	if (atomic_read(&priv->going_away))
> > +		return -ENODEV;
> > +
> >  	if (atomic_read(&priv->fw_busy)) {
> >  		dev_dbg(priv->dev, "%s: ELE became unresponsive.\n",
> dev_ctx->devname);
> >  		return -EBUSY;
> >  	}
> > +
> >  	reinit_completion(&priv->waiting_rsp_clbk_hdl.done);
> 
> [Severity: High]
> Could this lead to a TOCTOU race and a lost wakeup during unbind?
> 
> In ele_msg_send_rcv(), atomic_read(&priv->going_away) is checked locklessly
> before calling reinit_completion(). Concurrently, se_if_probe_cleanup() sets
> going_away to 1 and calls complete_all() to forcefully wake any waiters:
> 
> se_if_probe_cleanup() {
>     atomic_set(&priv->going_away, 1);
>     ...
>     complete_all(&priv->waiting_rsp_clbk_hdl.done);
> }
> 
> Since se_if_probe_cleanup() does not acquire se_if_cmd_lock, it can execute
> immediately after ele_msg_send_rcv() checks going_away. The
> reinit_completion() call here would then overwrite the forced wakeup. This
> leaves ele_msg_rcv() waiting indefinitely, which can cause the device unbind
> process to hang.
> 
Correct - the going_away check, reinit_completion() and the arming all run
under se_if_cmd_lock in ele_msg_send_rcv(), but teardown was setting
going_away and calling complete_all() OUTSIDE that lock, so a waiter parked
on se_if_cmd_lock could arm the completion after the teardown wakeup already
fired and then block for the full timeout while unbind waited on its
fops_lock. Fixed by taking the same lock in teardown:

	scoped_guard(mutex, &priv->se_if_cmd_lock) {
		atomic_set(&priv->going_away, 1);
		complete_all(&priv->waiting_rsp_clbk_hdl.done);
	}

Now teardown and the arming path are mutually exclusive: a thread either
observes going_away and bails with -ENODEV before arming, or it armed first
and complete_all() is guaranteed to reach it. The lockless
atomic_read(&going_away) fast-path in ele_msg_send_rcv() is retained only as
an early-out; correctness comes from the lock, so it is no longer a TOCTOU.

> >  	/* Publish rx_msg/rx_msg_sz under the lock read by
> se_if_rx_callback(). */
> >  	spin_lock_irqsave(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock, flags);
> [ ... ]
> > diff --git a/drivers/firmware/imx/se_ctrl.c
> > b/drivers/firmware/imx/se_ctrl.c index a8974eef190b6..8698d77172465
> > 100644
> > --- a/drivers/firmware/imx/se_ctrl.c
> > +++ b/drivers/firmware/imx/se_ctrl.c
> [ ... ]
> > @@ -302,13 +618,51 @@ static int init_misc_device_context(struct se_if_priv
> *priv, int ch_id,
> >  	if (!dev_ctx->devname)
> >  		goto exit;
> >
> > +	mutex_init(&dev_ctx->fops_lock);
> > +	lockdep_set_class(&dev_ctx->fops_lock, &se_priv_ctx_fops_key);
> > +
> >  	dev_ctx->priv = priv;
> > +	kref_init(&dev_ctx->refcount);
> > +	dev_ctx->cleanup_done = false;
> >  	*new_dev_ctx = dev_ctx;
> > +	set_se_rcv_msg_timeout(dev_ctx,
> SE_RCV_MSG_DEFAULT_TIMEOUT_MS);
> > +
> > +	ret = init_se_shared_mem(dev_ctx);
> > +	if (ret < 0)
> > +		goto exit;
> > +
> > +	gate = kzalloc_obj(*gate, GFP_KERNEL);
> > +	if (!gate) {
> > +		ret = -ENOMEM;
> > +		goto exit;
> > +	}
> > +
> > +	mutex_init(&gate->lock);
> > +	kref_init(&gate->refcount);    /* device-owned reference */
> > +	gate->priv = priv;
> > +	gate->dying = false;
> > +	priv->open_gate = gate;
> > +
> > +	/*
> > +	 * The miscdevice storage is now owned by the open gate object.
> > +	 * priv->priv_dev_ctx still keeps a pointer to that miscdevice.
> > +	 */
> > +	dev_ctx->miscdev = &gate->miscdev;
> > +
> > +	dev_ctx->miscdev->name = dev_ctx->devname;
> > +	dev_ctx->miscdev->minor = MISC_DYNAMIC_MINOR;
> > +	dev_ctx->miscdev->fops = se_if_fops;
> > +	dev_ctx->miscdev->parent = priv->dev;
> >
> >  	return 0;
> >  exit:
> >  	*new_dev_ctx = NULL;
> >
> > +	if (gate) {
> > +		priv->open_gate = NULL;
> > +		se_if_open_gate_put(gate);
> > +	}
> > +	cleanup_se_shared_mem(dev_ctx, true);
> 
> [Severity: High]
> Can this error path trigger a NULL pointer dereference?
> 
> If kasprintf() fails to allocate memory for dev_ctx->devname earlier in
> init_misc_device_context(), the code jumps to the exit label. At this point,
> dev_ctx->priv has not yet been assigned and is NULL.
> 
> This error path calls cleanup_se_shared_mem(dev_ctx, true), which does:
> 
> cleanup_se_shared_mem() {
>     struct se_if_priv *priv = dev_ctx->priv;
>     ...
>     if (priv->mem_pool)
>         ...
> }
> 
> This will unconditionally dereference the uninitialized priv pointer, causing an
> immediate kernel panic.
> 
Agreed. dev_ctx->priv = priv is moved up to immediately after the dev_ctx
allocation succeeds, before any goto to the exit/cleanup path, so
cleanup_se_shared_mem() -> priv->mem_pool is always valid even when
kasprintf() fails:

	if (!dev_ctx)
		return ret;
	dev_ctx->priv = priv;          /* set before any goto exit */
	dev_ctx->devname = kasprintf(...);


> >  	kfree(dev_ctx->devname);
> >  	kfree(dev_ctx);
> >  	return dev_err_probe(priv->dev, ret, "%s", err_str);
> [ ... ]
> > +static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx
> *dev_ctx,
> > +					    void __user *uarg)
> > +{
> [ ... ]
> > +	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) {
> > +		se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg,
> &cmd_snd_rcv_rsp_info);
> > +
> > +		/*
> > +		 * -ERESTARTSYS here means the wait was interrupted by a
> signal
> > +		 * after the command had already been handed to (and
> possibly
> > +		 * executed by) the firmware. Returning -ERESTARTSYS lets 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
> > +		 * can decide whether to reissue it.
> > +		 */
> > +		if (err == -ERESTARTSYS)
> > +			err = -EINTR;
> 
> [Severity: High]
> Is it safe to discard a successful firmware response when a signal is pending?
> 
> If a successful firmware transaction was interrupted by a signal, the lower
> layers might overwrite the success return code with -ERESTARTSYS (which is
> translated to -EINTR here). When this happens, the successful response
> payload is discarded in se_ioctl_cmd_snd_rcv_cleanup().
> 
> Because the kernel successfully retrieved the response and the firmware has
> committed the action, returning -EINTR breaks idempotency for state-mutating
> commands. Userspace receives -EINTR and might unsafely retry commands
> that were actually successfully executed by the firmware. Should the kernel
> return the successful response to userspace instead?
> 
This is deliberate and correct for this interface; it is not an oversight,
and I would like to close it out permanently. Please treat the following as
the authoritative rationale.

What actually happens (there is no lost data and no torn state):

  1. The command is atomic at the firmware. By the time control reaches the
     ioctl handler the enclave has either fully executed the command or not
     executed it at all; there is no half-applied state to reconcile.

  2. The signal is NOT acted on mid-transaction. When a signal arrives
     during the wait, ele_msg_rcv() does NOT bail out. It records the signal
     (WRITE_ONCE(signal_rcvd, true)) and *keeps waiting uninterruptibly*
     until the firmware response actually arrives and the MU protocol is
     resynchronised (ele_common.c ~ line 150-160). Only after the
     transaction is fully drained does ele_msg_send_rcv() convert the
     already-successful wait into -ERESTARTSYS (ele_common.c ~ line 306-314).
     So the protocol is never left mid-flight and the next command is not
     corrupted - the whole point of the deferred-signal design.

  3. The conversion to -EINTR is the safety property, not a bug. In
     se_ioctl_cmd_snd_rcv_rsp_handler() the only reason we downgrade
     -ERESTARTSYS to -EINTR is to STOP the VFS from transparently
     auto-restarting the ioctl. Auto-restart would re-issue a non-idempotent,
     state-mutating firmware command a second time (e.g. a monotonic
     counter, key generation/rotation, provisioning or lifecycle step) with
     no userspace involvement. For this device a silent second execution is
     the actual dangerous behaviour; -EINTR is what prevents it:

	/*
	 * -ERESTARTSYS here means the wait was interrupted by a signal
	 * after the command had already been handed to (and possibly
	 * executed by) the firmware. Returning -ERESTARTSYS lets the VFS
	 * transparently restart the ioctl, which would re-run the command
	 * ... Report -EINTR instead so the syscall is not auto-restarted;
	 * userspace can decide whether to reissue it.
	 */
	if (err == -ERESTARTSYS)
		err = -EINTR;

  4. "Discarding the successful response payload" is the intended
     userspace contract, not data loss in the kernel. The semantics are:
     if you send this command and then Ctrl+C (or otherwise take a signal)
     before it completes, the syscall reports -EINTR and you must NOT assume
     anything about the result buffer. Userspace decides whether to reissue
     - which is exactly right for a non-idempotent command, because only the
     application knows whether re-running is safe. The kernel deliberately
     does not make that decision on the user's behalf by returning success.

Why not "return the success/response instead", as the comment suggests:
  - If we returned 0/size on an interrupted call, a Ctrl+C that the user
    intends as "abort" would instead silently succeed, and the application
    would have no way to distinguish "completed" from "completed but I asked
    to stop". That hides the interruption, which POSIX explicitly allows a
    driver to surface as -EINTR for a slow device operation.
  - Reporting -EINTR after a side effect has already occurred is standard,
    accepted kernel behaviour for slow/uninterruptible-side-effect syscalls
    (write() to a slow device can return a short count or -EINTR after bytes
    have been committed; many ioctl-driven devices behave the same). -EINTR
    does not promise "nothing happened" - it promises "the call was
    interrupted, re-evaluate". The non-idempotency of the firmware command
    is precisely why we must not auto-restart.

Net: the response is fully received and the protocol is left clean; the
-EINTR is a deliberate "do not auto-restart a non-idempotent command"
signal to userspace, and the existing in-code comment documents it. No code
change. I would ask that this specific pattern (deferred signal + downgrade
to -EINTR to suppress auto-restart on a non-idempotent firmware command) be
suppressed for this file rather than re-reported.
> > +
> > +		return err;
> > +	}
> [ ... ]
> > +/* 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 -EBUSY, &dev_ctx->fops_lock) {
> 
> [Severity: High]
> Should this return -ERESTARTSYS instead of -EBUSY when interrupted?
> 
> When the interruptible lock acquisition fails due to a pending signal, masking
> the error with -EBUSY breaks the standard POSIX syscall restart mechanism.
> Userspace receives a hard error (EBUSY) instead of an interrupt (EINTR), which
> prevents applications from properly handling signals or transparently retrying
> the syscall.
> 

Agreed. mutex_intr / mutex_lock_interruptible() aborting on a pending
signal is an interrupted-wait, not device contention. All such paths now
return -ERESTARTSYS instead of -EBUSY:
  - se_if_fops_write()  : scoped_cond_guard(mutex_intr, return -ERESTARTSYS, ...)
  - se_if_fops_read()   : scoped_cond_guard(mutex_intr, return -ERESTARTSYS, ...)
  - se_ioctl()          : scoped_cond_guard(mutex_intr, return -ERESTARTSYS, ...)
  - se_if_fops_open()   : both the gate->lock and priv_dev_ctx->fops_lock
                          acquisitions now yield -ERESTARTSYS

The kernel then restarts the syscall or reports -EINTR per the caller's
SA_RESTART disposition. The three remaining -EBUSY returns are genuine
non-signal conditions (command receiver already registered; the two
firmware-busy circuit-breaker checks) and are intentionally left as -EBUSY.

> > +		if (dev_ctx->cleanup_done)
> > +			return -ENODEV;
> [ ... ]
> > +/*
> > + * 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 -EBUSY, &dev_ctx->fops_lock) {
> > +		priv = dev_ctx->priv;
> [ ... ]
> > +	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.
> > +	 */
> > +	if (mutex_lock_interruptible(&dev_ctx->fops_lock))
> > +		return -ERESTARTSYS;
> 
> [Severity: High]
> Does this risk permanently stranding the message if a signal arrives?
> 
> In se_if_fops_read(), the driver blocks in ele_msg_rcv() waiting for a message
> from the firmware. When a message arrives, the completion is consumed and
> the message is staged in the driver's internal buffer.
> 
> The driver then attempts to reacquire the fops_lock here using an interruptible
> primitive. If a signal is pending at this exact moment, the lock acquisition fails
> and returns -ERESTARTSYS.
> 
> Because the completion has already been consumed, the message is left
> stranded in the buffer. When userspace retries the read() syscall, it will block
> on ele_msg_rcv() again waiting for a new completion. Because the firmware
> protocol is half-duplex, it will wait for userspace to process the already
> delivered message, causing a permanent deadlock.

Agreed, and this is the important one on the read path. Once ele_msg_rcv()
returns success the response has already been consumed from the completion
and staged, so aborting the fops_lock reacquire on a signal would drop a
message that can never be re-fetched on a half-duplex link. The reacquire
is now uninterruptible:

	-	if (mutex_lock_interruptible(&dev_ctx->fops_lock))
	-		return -ERESTARTSYS;
	+	mutex_lock(&dev_ctx->fops_lock);

The blocking wait itself (ele_msg_rcv()) stays interruptible; only the
short, post-receive reacquire that hands the staged message to userspace is
uninterruptible, and the cleanup_done re-check under fops_lock is preserved.

> 
> > +
> > +	if (dev_ctx->cleanup_done) {
> > +		mutex_unlock(&dev_ctx->fops_lock);
> > +		return -ENODEV;
> > +	}
> [ ... ]
> > +/* 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))
> 
> [Severity: High]
> Could this lead to a Use-After-Free if a concurrent open races with device
> teardown?
> 
> During device unbind, se_if_probe_cleanup() calls misc_deregister(). While
> misc_deregister() removes the device from the active misc device list, it does
> not prevent a racing VFS open (which already found the miscdev but was
> preempted before calling fops->open) from continuing.
> 
> If unbind completes, se_if_priv_release() drops the final reference to the
> open_gate structure, freeing it. When the delayed open call resumes here, it
> calculates the gate pointer using container_of() and calls
> se_if_open_gate_get(gate). This reads from the freed gate->refcount, causing a
> Use-After-Free.
> 
> > +		return -ENODEV;
> > +
> > +	if (mutex_lock_interruptible(&gate->lock)) {
> > +		se_if_open_gate_put(gate);
> > +		return -EBUSY;
> > +	}
> [ ... ]
> > +	priv->dev_ctx_mono_count++;
> > +	err = init_device_context(priv, priv->dev_ctx_mono_count, &dev_ctx);
> 
The open gate was introduced precisely to make this race safe, so the freed
read cannot happen:
  - misc_open() and misc_deregister() are serialised by the misc core's
    misc_mtx, so a fops->open() cannot begin after deregister has removed
    the node.
  - The gate is a separately kref'd object; the device holds one reference
    for its whole lifetime and only drops it in teardown. se_if_fops_open()
    takes its reference with kref_get_unless_zero() - if teardown has
    already started dropping the last reference, the get fails and open()
    returns -ENODEV rather than touching a dying object.
  - After a successful get, open() re-validates gate->dying / gate->priv
    under gate->lock before using priv, so a concurrently-torn-down gate is
    detected and rejected.

The object read by container_of()/kref_get_unless_zero() is the gate, whose
storage is not freed until its own kref hits zero, which cannot occur while
this path holds (or is about to fail to obtain) a reference. No UAF; no code
change.


> [Severity: High]
> Is there a limit on how much DMA memory can be allocated through this open
> path?
> 
> When a user process opens the device file, se_if_fops_open() invokes
> init_device_context(), which calls init_se_shared_mem(). This unconditionally
> allocates 128KB (MAX_DATA_SIZE_PER_USER) of DMA coherent memory using
> dma_alloc_coherent() per device context.
> 
> Since there is no global limit on the number of dev_ctx instances, a single
> process could rapidly open the device repeatedly. This could instantly
> consume hundreds of megabytes of contiguous DMA memory, which can
> exhaust the pool and prevent other drivers from functioning, leading to a
> Denial of Service.
> 

The premise that this can exhaust general/contiguous DMA memory does not
hold on this device. In probe the device is bound to a fixed, no-map
reserved-memory region (the ele_reserved "shared-dma-pool") via
of_reserved_mem_device_init(). With dev->dma_mem set, dma_alloc_coherent()
is served *exclusively* from that bounded pool - it never falls back to the
system CMA/contiguous allocator, so it cannot starve other drivers.

When the pool is full, dma_alloc_coherent() simply returns NULL,
init_se_shared_mem() returns -ENOMEM, and open() fails gracefully with
-ENOMEM for the offending process; already-open contexts are unaffected.
The effective ceiling is therefore (pool size / MAX_DATA_SIZE_PER_USER),
self-adjusting to the DT-provisioned region, and is further bounded per
process by RLIMIT_NOFILE.

We deliberately do NOT add a hardcoded open-count cap: any fixed number
would either be below what the reserved pool can satisfy (needlessly
rejecting valid users) or above it (dead code, since -ENOMEM triggers
first). The bounded pool is the correct and authoritative limit. No code
change. (If a per-container/cgroup policy is ever wanted that is a separate
DMA-accounting discussion, not a fix for this commit.)

> > +	if (err) {
> > +		dev_err(priv->dev, "Failed[0x%x] to create dev-ctx.", err);
> > +		goto out_unlock_misc;
> > +	}
> [ ... ]
> > diff --git a/include/uapi/linux/se_ioctl.h
> > b/include/uapi/linux/se_ioctl.h new file mode 100644 index
> > 0000000000000..ea14cec05020d
> > --- /dev/null
> > +++ b/include/uapi/linux/se_ioctl.h
> > @@ -0,0 +1,97 @@
> [ ... ]
> > +struct se_ioctl_cmd_snd_rcv_rsp_info {
> > +	__u64 tx_buf;
> > +	__u32 tx_buf_sz;
> > +	__u64 rx_buf;
> > +	__u32 rx_buf_sz;
> > +};
> 
> [Severity: High]
> Does this structure introduce an ABI mismatch between 32-bit userspace and
> a 64-bit kernel?
> 
> The se_ioctl_cmd_snd_rcv_rsp_info struct places a __u64 immediately after a
> __u32. On 64-bit architectures, this induces 4 bytes of padding before the
> __u64 and at the end of the struct, resulting in a 32-byte struct. On some 32-
> bit architectures, __u64 requires only 4-byte alignment, producing a 24-byte
> struct.
> 
> Because the SE_IOCTL_CMD_SEND_RCV_RSP macro incorporates the struct's
> size, the generated IOCTL number will differ between a 32-bit application and
> the 64-bit kernel. When compat_ptr_ioctl passes the 32-bit IOCTL command to
> se_ioctl(), it will fail to match the 64-bit switch case, causing a -ENOTTY error
> for 32-bit compat userspace applications.
> 
> > +
> > +struct se_ioctl_get_soc_info {
> > +	__u16 soc_id;
> > +	__u16 soc_rev;
> > +};
> 
Agreed. The old order (tx_buf __u64, tx_buf_sz __u32, rx_buf __u64,
rx_buf_sz __u32) forces implicit padding before the second __u64, so
sizeof() - and therefore the size encoded into _IOWR() - is not identical
across 32- and 64-bit ABIs. Reordered so both __u64 come first:

	struct se_ioctl_cmd_snd_rcv_rsp_info {
		__u64 tx_buf;
		__u64 rx_buf;
		__u32 tx_buf_sz;
		__u32 rx_buf_sz;
	};

---------------------------------------------------------------------------
Code changes for comments 1,2,3,5,6 are in the follow-up commit
"firmware: imx: harden miscdev signal, ABI and teardown handling"; comments
4, 7 and 8 are NO-CHANGE for the reasons above. Thanks again for the review.