All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Mukesh Ojha" <mukesh.ojha@oss.qualcomm.com>
Cc: dri-devel@lists.freedesktop.org, media-ci@linuxtv.org
Subject: Re: [PATCH 1/8] remoteproc: qcom_wcnss: migrate to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()
Date: Sat, 26 Sep 2026 15:59:32 +0000	[thread overview]
Message-ID: <20260926155932.D28DD1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260925155737.4161358-2-mukesh.ojha@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] remoteproc: qcom_wcnss: DMA memory leak of metadata in wcnss_load()
- [High] remoteproc: qcom_wcnss: missing pas_ctx->use_tzmem = true prevents required shmbridge creation
--

commit 77b1a378f038e55ff2a11032d9b6f243214fbb7f
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Date:   Fri Sep 25 15:57:30 2026 +0000

    remoteproc: qcom_wcnss: migrate to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

This commit migrates the Qualcomm WCNSS remoteproc driver to use 
qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset(). It allocates a 
PAS context during probe to support TrustZone authentication with shmbridge 
preparation.

> diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> index c856a92af43ce..01f9076aa0b17 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c

[ ... ]

> @@ -157,9 +158,7 @@ static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
>  	struct qcom_wcnss *wcnss = rproc->priv;
>  	int ret;
>  
> -	ret = qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID,
> -			    (__force void *)wcnss->mem_region, wcnss->mem_phys,
> -			    wcnss->mem_size, &wcnss->mem_reloc);
> +	ret = qcom_mdt_pas_load(wcnss->pas_ctx, fw, rproc->firmware, &wcnss->mem_reloc);

[Severity: High]
Does this introduce a DMA memory leak of the metadata buffer?

By passing wcnss->pas_ctx to qcom_mdt_pas_load(), the allocated metadata
buffer is retained in wcnss->pas_ctx->ptr. In the previous qcom_mdt_load()
implementation, passing a NULL context signaled the backend to free the
buffer immediately.

Since the remoteproc operations structure (wcnss_ops) lacks an .unprepare
hook, there doesn't seem to be a corresponding call to
qcom_pas_metadata_release(wcnss->pas_ctx) when the device is shut down.

Will this persistently leak DMA memory each time the remoteproc is started
and stopped via sysfs?

>  	if (ret)
>  		return ret;

[ ... ]

> @@ -600,6 +599,11 @@ static int wcnss_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	wcnss->pas_ctx = devm_qcom_pas_context_alloc(wcnss->dev, WCNSS_PAS_ID,
> +						     wcnss->mem_phys, wcnss->mem_size);

[Severity: High]
The commit message explicitly states the goal is to include the shmbridge
prepare step required before TrustZone authentication. However, does this
actually enable the shmbridge?

The newly allocated wcnss->pas_ctx is zero-initialized by
devm_qcom_pas_context_alloc(), so pas_ctx->use_tzmem defaults to false. When
use_tzmem is false, qcom_scm_pas_prepare_and_auth_reset() explicitly skips
creating the shmbridge and falls back to the legacy behavior.

Should wcnss->pas_ctx->use_tzmem be set to true here to fulfill the stated
security objective?

> +	if (IS_ERR(wcnss->pas_ctx))
> +		return PTR_ERR(wcnss->pas_ctx);
> +
>  	wcnss->pmu_cfg = mmio + data->pmu_offset;
>  	wcnss->spare_out = mmio + data->spare_offset;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925155737.4161358-1-mukesh.ojha@oss.qualcomm.com?part=1

  reply	other threads:[~2026-09-26 15:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 15:57 [PATCH 0/8] Migrate PAS clients to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset() Mukesh Ojha
2026-09-25 15:57 ` [PATCH 1/8] remoteproc: qcom_wcnss: migrate " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot [this message]
2026-09-25 15:57 ` [PATCH 2/8] drm/msm/adreno: migrate zap shader loading " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 3/8] media: iris: migrate firmware " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 4/8] media: venus: " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 5/8] net: ipa: " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 6/8] wifi: ath12k: " Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 7/8] soc: qcom: mdt_loader: remove qcom_mdt_load() Mukesh Ojha
2026-09-26 15:59   ` sashiko-bot
2026-09-25 15:57 ` [PATCH 8/8] firmware: qcom: pas: remove qcom_pas_auth_and_reset() Mukesh Ojha

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=20260926155932.D28DD1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=media-ci@linuxtv.org \
    --cc=mukesh.ojha@oss.qualcomm.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.