| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b771d045-9b37-4fb3-8257-6c56a3942755@ti.com>
Date: Wed, 8 Jan 2025 14:19:34 +0530
From: Beleswar Prasad Padhi <b-padhi@...com>
To: <andersson@...nel.org>, <mathieu.poirier@...aro.org>
CC: <afd@...com>, <hnagalla@...com>, <u-kumar1@...com>,
<p.zabel@...gutronix.de>, <jan.kiszka@...mens.com>,
<christophe.jaillet@...adoo.fr>, <jkangas@...hat.com>,
<eballetbo@...hat.com>, <linux-remoteproc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 01/20] remoteproc: k3-m4: Prevent Mailbox level IPC
with detached core
On 03/01/25 15:42, Beleswar Padhi wrote:
> Inter-Processor Communication is facilitated through mailbox payloads,
> which typically contains the index of the triggered virtqueue having the
> actual data to be consumed, but the payload can also be used for trivial
> communication, like sending an echo message or notifying a crash etc.
> When the core is detached, the virtqueues are freed, and thus the Virtio
> level IPC is not functional. However, Mailbox IPC is still possible with
> trivial payloads.
>
> Therefore, introduce checks in k3_m4_rproc_kick() and
> k3_m4_rproc_mbox_callback() functions to return early without parsing
> the payload when core is detached, and is not undergoing an attach
> operation in IPC-only mode.
>
> Signed-off-by: Beleswar Padhi <b-padhi@...com>
> ---
> drivers/remoteproc/ti_k3_m4_remoteproc.c | 41 ++++++++++++++++++++----
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/remoteproc/ti_k3_m4_remoteproc.c
> index a16fb165fced..3201c3684a86 100644
> --- a/drivers/remoteproc/ti_k3_m4_remoteproc.c
> +++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c
> @@ -50,6 +50,7 @@ struct k3_m4_rproc_mem_data {
> /**
> * struct k3_m4_rproc - k3 remote processor driver structure
> * @dev: cached device pointer
> + * @rproc: remoteproc device handle
> * @mem: internal memory regions data
> * @num_mems: number of internal memory regions
> * @rmem: reserved memory regions data
> @@ -60,9 +61,11 @@ struct k3_m4_rproc_mem_data {
> * @ti_sci_id: TI-SCI device identifier
> * @mbox: mailbox channel handle
> * @client: mailbox client to request the mailbox channel
> + * @is_attach_ongoing: flag to indicate if IPC-only "attach()" is in progress
Note: This variable naming will be changed to 'is_attached' (for better
clarity) in the next revision. I wish to take more feedback for the
whole series before posting next revision.
Thanks,
Beleswar
> */
> struct k3_m4_rproc {
> struct device *dev;
> + struct rproc *rproc;
> struct k3_m4_rproc_mem *mem;
> int num_mems;
> struct k3_m4_rproc_mem *rmem;
> @@ -73,6 +76,7 @@ struct k3_m4_rproc {
> u32 ti_sci_id;
> struct mbox_chan *mbox;
> struct mbox_client client;
> + bool is_attach_ongoing;
> };
>
> /**
> @@ -93,8 +97,16 @@ static void k3_m4_rproc_mbox_callback(struct mbox_client *client, void *data)
> {
> struct device *dev = client->dev;
> struct rproc *rproc = dev_get_drvdata(dev);
> + struct k3_m4_rproc *kproc = rproc->priv;
> u32 msg = (u32)(uintptr_t)(data);
>
> + /*
> + * Do not forward messages from a detached core, except when the core
> + * is in the process of being attached in IPC-only mode.
> + */
> + if (!kproc->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED)
> + return;
> +
> dev_dbg(dev, "mbox msg: 0x%x\n", msg);
>
> switch (msg) {
> @@ -135,6 +147,12 @@ static void k3_m4_rproc_kick(struct rproc *rproc, int vqid)
> u32 msg = (u32)vqid;
> int ret;
>
> + /*
> + * Do not forward messages to a detached core, except when the core
> + * is in the process of being attached in IPC-only mode.
> + */
> + if (!kproc->is_attach_ongoing && kproc->rproc->state == RPROC_DETACHED)
> + return;
> /*
> * Send the index of the triggered virtqueue in the mailbox payload.
> * NOTE: msg is cast to uintptr_t to prevent compiler warnings when
> @@ -515,15 +533,19 @@ static int k3_m4_rproc_stop(struct rproc *rproc)
> /*
> * Attach to a running M4 remote processor (IPC-only mode)
> *
> - * The remote processor is already booted, so there is no need to issue any
> - * TI-SCI commands to boot the M4 core. This callback is used only in IPC-only
> - * mode.
> + * This rproc attach callback only needs to set the "is_attach_ongoing" flag to
> + * notify k3_m4_rproc_{kick/mbox_callback} functions that the core is in the
> + * process of getting attached in IPC-only mode. The remote processor is already
> + * booted, so there is no need to issue any TI-SCI commands to boot the M4 core.
> + * This callback is used only in IPC-only mode.
> */
> static int k3_m4_rproc_attach(struct rproc *rproc)
> {
> struct k3_m4_rproc *kproc = rproc->priv;
> int ret;
>
> + kproc->is_attach_ongoing = true;
> +
> ret = k3_m4_rproc_ping_mbox(kproc);
> if (ret)
> return ret;
> @@ -534,12 +556,18 @@ static int k3_m4_rproc_attach(struct rproc *rproc)
> /*
> * Detach from a running M4 remote processor (IPC-only mode)
> *
> - * This rproc detach callback performs the opposite operation to attach
> - * callback, the M4 core is not stopped and will be left to continue to
> - * run its booted firmware. This callback is invoked only in IPC-only mode.
> + * This rproc detach callback performs the opposite operation to attach callback
> + * and only needs to clear the "is_attach_ongoing" flag to ensure no mailbox
> + * messages are sent to or received from a detached core. The M4 core is not
> + * stopped and will be left to continue to run its booted firmware. This
> + * callback is invoked only in IPC-only mode.
> */
> static int k3_m4_rproc_detach(struct rproc *rproc)
> {
> + struct k3_m4_rproc *kproc = rproc->priv;
> +
> + kproc->is_attach_ongoing = false;
> +
> return 0;
> }
>
> @@ -577,6 +605,7 @@ static int k3_m4_rproc_probe(struct platform_device *pdev)
> rproc->has_iommu = false;
> rproc->recovery_disabled = true;
> kproc = rproc->priv;
> + kproc->rproc = rproc;
> kproc->dev = dev;
> platform_set_drvdata(pdev, rproc);
>
Powered by blists - more mailing lists