linux-kernel - Re: [PATCH v2 8/9] mailbox: mediatek: Add CMDQ secure mailbox driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 6 Nov 2023 06:53:02 +0000
From:   CK Hu (胡俊光) <ck.hu@...iatek.com>
To:     "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
        Jason-JH Lin (林睿祥) 
        <Jason-JH.Lin@...iatek.com>,
        "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Singo Chang (張興國) 
        <Singo.Chang@...iatek.com>,
        Johnson Wang (王聖鑫) 
        <Johnson.Wang@...iatek.com>,
        Jason-ch Chen (陳建豪) 
        <Jason-ch.Chen@...iatek.com>,
        Shawn Sung (宋孝謙) 
        <Shawn.Sung@...iatek.com>,
        Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
        "jkardatzke@...gle.com" <jkardatzke@...gle.com>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH v2 8/9] mailbox: mediatek: Add CMDQ secure mailbox driver

Hi, Jason:

On Mon, 2023-10-23 at 12:37 +0800, Jason-JH.Lin wrote:
> To support secure video path feature, GCE have to read/write
> registgers
> in the secure world. GCE will enable the secure access permission to
> the
> HW who wants to access the secure content buffer.
> 
> Add CMDQ secure mailbox driver to make CMDQ client user is able to
> sending their HW settings to the secure world. So that GCE can
> execute
> all instructions to configure HW in the secure world.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> ---
>  drivers/mailbox/Makefile                      |    2 +-
>  drivers/mailbox/mtk-cmdq-sec-mailbox.c        | 1102
> +++++++++++++++++
>  drivers/mailbox/mtk-cmdq-sec-tee.c            |  202 +++
>  include/linux/mailbox/mtk-cmdq-mailbox.h      |    2 +
>  .../linux/mailbox/mtk-cmdq-sec-iwc-common.h   |  293 +++++
>  include/linux/mailbox/mtk-cmdq-sec-mailbox.h  |   83 ++
>  include/linux/mailbox/mtk-cmdq-sec-tee.h      |   31 +
>  7 files changed, 1714 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mailbox/mtk-cmdq-sec-mailbox.c
>  create mode 100644 drivers/mailbox/mtk-cmdq-sec-tee.c
>  create mode 100644 include/linux/mailbox/mtk-cmdq-sec-iwc-common.h
>  create mode 100644 include/linux/mailbox/mtk-cmdq-sec-mailbox.h
>  create mode 100644 include/linux/mailbox/mtk-cmdq-sec-tee.h
> 
> 

[snip]

> +
> +int cmdq_sec_insert_backup_cookie(struct cmdq_pkt *pkt)
> +{
> +	struct cmdq_client *cl = (struct cmdq_client *)pkt->cl;
> +	struct cmdq_sec_thread *thread = ((struct mbox_chan *)(cl-
> >chan))->con_priv;
> +	struct cmdq_sec *cmdq = container_of(thread->chan->mbox, struct
> cmdq_sec, mbox);
> +	struct cmdq_operand left, right;
> +	dma_addr_t addr;
> +
> +	if (!thread->occupied || !cmdq->shared_mem)
> +		return -EFAULT;
> +
> +	pr_debug("%s %d: pkt:%p thread:%u gce:%#lx",
> +		 __func__, __LINE__, pkt, thread->idx, (unsigned
> long)cmdq->base_pa);
> +
> +	addr = (u32)(cmdq->base_pa + CMDQ_THR_BASE +
> +		CMDQ_THR_SIZE * thread->idx + CMDQ_THR_EXEC_CNT_PA);
> +
> +	cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX1, CMDQ_ADDR_HIGH(addr));
> +	cmdq_pkt_read_s(pkt, CMDQ_THR_SPR_IDX1, CMDQ_ADDR_LOW(addr),
> CMDQ_THR_SPR_IDX1);
> +
> +	left.reg = true;
> +	left.idx = CMDQ_THR_SPR_IDX1;
> +	right.reg = false;
> +	right.value = 1;
> +	cmdq_pkt_logic_command(pkt, CMDQ_LOGIC_ADD, CMDQ_THR_SPR_IDX1,
> &left, &right);
> +
> +	addr = cmdq->shared_mem->pa + CMDQ_SEC_SHARED_THR_CNT_OFFSET +
> +		thread->idx * sizeof(u32);
> +
> +	cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX2, CMDQ_ADDR_HIGH(addr));
> +	cmdq_pkt_write_s(pkt, CMDQ_THR_SPR_IDX2, CMDQ_ADDR_LOW(addr),
> CMDQ_THR_SPR_IDX1);

This is a sample code of reading secure data and writing to normal
world. I think you have already check address of cmdq_pkt_read_s() in
TEE. This is a sample code that hacker may try to read any secure
address:

	cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX1,
CMDQ_ADDR_HIGH(hack_address));
	cmdq_pkt_jump(pkt, read);
	cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX1, CMDQ_ADDR_HIGH(addr));
read:
	cmdq_pkt_read_s(pkt, CMDQ_THR_SPR_IDX1, CMDQ_ADDR_LOW(addr),
		CMDQ_THR_SPR_IDX1);

Please explain how you protect this hacker attack.

Regards,
CK

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(cmdq_sec_insert_backup_cookie);
> +