| 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
| ||
|
Date: Sat, 13 Jun 2020 07:20:38 +0800
From: Chun-Kuang Hu <chunkuang.hu@...nel.org>
To: Neal Liu <neal.liu@...iatek.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
devicetree@...r.kernel.org,
wsd_upstream <wsd_upstream@...iatek.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@...ts.infradead.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/2] soc: mediatek: devapc: add devapc-mt6873 driver
Hi, Neal:
Neal Liu <neal.liu@...iatek.com> 於 2020年6月9日 週二 下午6:25寫道：
>
> MT6873 bus frabric provides TrustZone security support and data
> protection to prevent slaves from being accessed by unexpected
> masters.
> The security violations are logged and sent to the processor for
> further analysis or countermeasures.
>
> Any occurrence of security violation would raise an interrupt, and
> it will be handled by devapc-mt6873 driver. The violation
> information is printed in order to find the murderer.
>
> Signed-off-by: Neal Liu <neal.liu@...iatek.com>
[snip]
> +
> +/*
> + * sramrom_vio_handler - clean sramrom violation & print violation information
> + * for debugging.
> + */
> +static void sramrom_vio_handler(void)
> +{
> + const struct mtk_sramrom_sec_vio_desc *sramrom_vios;
> + struct mtk_devapc_vio_info *vio_info;
> + struct arm_smccc_res res;
> + size_t sramrom_vio_sta;
> + int sramrom_vio;
> + u32 rw;
> +
> + sramrom_vios = mtk_devapc_ctx->soc->sramrom_sec_vios;
> + vio_info = mtk_devapc_ctx->soc->vio_info;
> +
> + arm_smccc_smc(MTK_SIP_KERNEL_CLR_SRAMROM_VIO,
> + 0, 0, 0, 0, 0, 0, 0, &res);
> +
> + sramrom_vio = res.a0;
> + sramrom_vio_sta = res.a1;
> + vio_info->vio_addr = res.a2;
> +
> + if (sramrom_vio == SRAM_VIOLATION)
> + pr_info(PFX "%s, SRAM violation is triggered\n", __func__);
> + else if (sramrom_vio == ROM_VIOLATION)
> + pr_info(PFX "%s, ROM violation is triggered\n", __func__);
> + else
> + return;
> +
> + vio_info->master_id = (sramrom_vio_sta & sramrom_vios->vio_id_mask)
> + >> sramrom_vios->vio_id_shift;
> + vio_info->domain_id = (sramrom_vio_sta & sramrom_vios->vio_domain_mask)
> + >> sramrom_vios->vio_domain_shift;
> + rw = (sramrom_vio_sta & sramrom_vios->vio_rw_mask) >>
> + sramrom_vios->vio_rw_shift;
I think some information, such as master_id, would be get in
devapc_extract_vio_dbg(), you need not to get it here.
> +
> + if (rw)
> + vio_info->write = 1;
> + else
> + vio_info->read = 1;
> +
> + pr_info(PFX "%s: %s:0x%x, %s:0x%x, %s:%s, %s:0x%x\n",
> + __func__, "master_id", vio_info->master_id,
> + "domain_id", vio_info->domain_id,
> + "rw", rw ? "Write" : "Read",
> + "vio_addr", vio_info->vio_addr);
> +}
> +
[snip]
> +
> +/*
> + * devapc_violation_irq - the devapc Interrupt Service Routine (ISR) will dump
> + * violation information including which master violates
> + * access slave.
> + */
> +static irqreturn_t devapc_violation_irq(int irq_number, void *dev_id)
> +{
> + u32 slave_type_num = mtk_devapc_ctx->soc->slave_type_num;
Don't make mtk_devapc_ctx a global variable. You should allocate
instance of mtk_devapc_ctx in probe(), and pass mtk_devapc_ctx to
the last parameter of devm_request_irq(), and it would be the second
parameter of irq handler.
> + const struct mtk_device_info **device_info;
> + struct mtk_devapc_vio_info *vio_info;
> + int slave_type, vio_idx, index;
> + const char *vio_master;
> + unsigned long flags;
> + bool normal;
> + u8 perm;
> +
> + spin_lock_irqsave(&devapc_lock, flags);
> +
> + device_info = mtk_devapc_ctx->soc->device_info;
> + vio_info = mtk_devapc_ctx->soc->vio_info;
> + normal = false;
> + vio_idx = -1;
> + index = -1;
> +
> + /* There are multiple DEVAPC_PD */
> + for (slave_type = 0; slave_type < slave_type_num; slave_type++) {
> + if (!check_type2_vio_status(slave_type, &vio_idx, &index))
> + if (!mtk_devapc_dump_vio_dbg(slave_type, &vio_idx,
> + &index))
> + continue;
> +
> + /* Ensure that violation info are written before
> + * further operations
> + */
> + smp_mb();
> + normal = true;
> +
> + mask_module_irq(slave_type, vio_idx, true);
> +
> + if (clear_vio_status(slave_type, vio_idx))
> + pr_warn(PFX "%s, %s:0x%x, %s:0x%x\n",
> + "clear vio status failed",
> + "slave_type", slave_type,
> + "vio_index", vio_idx);
> +
> + perm = get_permission(slave_type, index, vio_info->domain_id);
> +
> + vio_master = mtk_devapc_ctx->soc->master_get
> + (vio_info->master_id,
> + vio_info->vio_addr,
> + slave_type,
> + vio_info->shift_sta_bit,
> + vio_info->domain_id);
> +
> + if (!vio_master) {
> + pr_warn(PFX "master_get failed\n");
> + vio_master = "UNKNOWN_MASTER";
> + }
> +
> + pr_info(PFX "%s - %s:0x%x, %s:0x%x, %s:0x%x, %s:0x%x\n",
> + "Violation", "slave_type", slave_type,
> + "sys_index",
> + device_info[slave_type][index].sys_index,
> + "ctrl_index",
> + device_info[slave_type][index].ctrl_index,
> + "vio_index",
> + device_info[slave_type][index].vio_index);
> +
> + pr_info(PFX "%s %s %s %s\n",
> + "Violation - master:", vio_master,
> + "access violation slave:",
> + device_info[slave_type][index].device);
> +
> + devapc_vio_reason(perm);
> +
> + devapc_extra_handler(slave_type, vio_master, vio_idx,
> + vio_info->vio_addr);
> +
> + mask_module_irq(slave_type, vio_idx, false);
> + }
> +
> + if (normal) {
> + spin_unlock_irqrestore(&devapc_lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + spin_unlock_irqrestore(&devapc_lock, flags);
> + return IRQ_HANDLED;
> +}
> +
[snip]
> +
> +int mtk_devapc_probe(struct platform_device *pdev, struct mtk_devapc_soc *soc)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + u32 slave_type_num;
> + int slave_type;
> + int ret;
> +
> + if (IS_ERR(node))
> + return -ENODEV;
> +
> + mtk_devapc_ctx->soc = soc;
> + slave_type_num = mtk_devapc_ctx->soc->slave_type_num;
> +
> + for (slave_type = 0; slave_type < slave_type_num; slave_type++) {
> + mtk_devapc_ctx->devapc_pd_base[slave_type] =
> + of_iomap(node, slave_type);
> + if (!mtk_devapc_ctx->devapc_pd_base[slave_type])
> + return -EINVAL;
> + }
> +
> + mtk_devapc_ctx->infracfg_base = of_iomap(node, slave_type_num + 1);
> + if (!mtk_devapc_ctx->infracfg_base)
> + return -EINVAL;
> +
> + mtk_devapc_ctx->devapc_irq = irq_of_parse_and_map(node, 0);
> + if (!mtk_devapc_ctx->devapc_irq)
> + return -EINVAL;
> +
> + /* CCF (Common Clock Framework) */
> + mtk_devapc_ctx->devapc_infra_clk = devm_clk_get(&pdev->dev,
> + "devapc-infra-clock");
> +
> + if (IS_ERR(mtk_devapc_ctx->devapc_infra_clk))
> + return -EINVAL;
> +
> + proc_create("devapc_dbg", 0664, NULL, &devapc_dbg_fops);
> +
> + if (clk_prepare_enable(mtk_devapc_ctx->devapc_infra_clk))
> + return -EINVAL;
> +
> + start_devapc();
> +
> + ret = devm_request_irq(&pdev->dev, mtk_devapc_ctx->devapc_irq,
> + (irq_handler_t)devapc_violation_irq,
> + IRQF_TRIGGER_NONE, "devapc", NULL);
> + if (ret) {
> + pr_err(PFX "request devapc irq failed, ret:%d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mtk_devapc_probe);
Why export probe function?
> +
> +int mtk_devapc_remove(struct platform_device *dev)
> +{
> + clk_disable_unprepare(mtk_devapc_ctx->devapc_infra_clk);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mtk_devapc_remove);
Ditto.
Regards,
Chun-Kuang.
Powered by blists - more mailing lists