Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jaidev Shastri" <jaidevshastri@vt.edu>
Cc: imx@lists.linux.dev, Frank.Li@kernel.org
Subject: Re: [PATCH] firmware: imx: sm: publish the protocol handle after the ops pointer
Date: Tue, 22 Sep 2026 00:40:39 +0000	[thread overview]
Message-ID: <20260922004039.BB35B1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260921-mb-imx-sm-v1-1-39f17526d253@vt.edu>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrency: discarded smp_load_acquire result creates TOCTOU double-fetch
--

commit bbaeb64e803a74302b103407c87f99c75c2cbd83
Author: Jaidev Shastri <jaidevshastri@vt.edu>

firmware: imx: sm: publish the protocol handle after the ops pointer

This commit resolves an un-ordered initialization issue by publishing the
protocol handle ph only after the ops pointer is set. It uses
smp_store_release() during probe and smp_load_acquire() in the exported
helpers to ensure memory consistency.

> --- a/drivers/firmware/imx/sm-cpu.c
> +++ b/drivers/firmware/imx/sm-cpu.c
[ ... ]
> @@ -26,7 +27,8 @@ EXPORT_SYMBOL(scmi_imx_cpu_reset_vector_set);
>  
>  int scmi_imx_cpu_start(u32 cpuid, bool start)
>  {
> -	if (!ph)
> +	/* Pairs with the smp_store_release() in the probe function. */
> +	if (!smp_load_acquire(&ph))
>  		return -EPROBE_DEFER;
>  
>  	if (start)
>  		return imx_cpu_ops->cpu_start(ph, cpuid, true);

[Severity: Medium]
Does discarding the result of smp_load_acquire(&ph) create a
Time-of-Check to Time-of-Use (TOCTOU) double-fetch race?

In scmi_imx_cpu_start() in drivers/firmware/imx/sm-cpu.c, the result of the
safe smp_load_acquire(&ph) fetch is only used for the NULL check. The global
ph is then read a second time from memory when it is passed into
imx_cpu_ops->cpu_start(ph, cpuid, true).

If ph were cleared concurrently (such as during driver unbind or probe error
rollback), this second load could fetch NULL and potentially cause a crash.

While this bug might currently be latent if the driver lacks a teardown path
that clears ph, this double-fetch usage bypasses the acquire semantics.

Could we store the acquired handle in a local variable and use that local
variable for both the NULL check and the function call?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-mb-imx-sm-v1-1-39f17526d253@vt.edu?part=1

      reply	other threads:[~2026-09-22  0:40 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  0:28 [PATCH] firmware: imx: sm: publish the protocol handle after the ops pointer Jaidev Shastri via B4 Relay
2026-09-22  0:40 ` sashiko-bot [this message]

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=20260922004039.BB35B1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=jaidevshastri@vt.edu \
    --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