| 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: Fri, 11 Nov 2022 17:18:31 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Patrick Delaunay <patrick.delaunay@...s.st.com>,
Alexandre TORGUE <alexandre.torgue@...s.st.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>
Cc: Fabrice GASNIER <fabrice.gasnier@...s.st.com>,
Etienne CARRIERE <etienne.carriere@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH v2 2/3] nvmem: stm32: add OP-TEE support for STM32MP13x
On 10/11/2022 15:45, Patrick Delaunay wrote:
> For boot with OP-TEE on STM32MP13, the communication with the secure
> world no more use STMicroelectronics SMC but communication with the
> BSEC TA, for data access (read/write) or lock operation:
> - all the request are sent to OP-TEE trusted application,
> - for upper OTP with ECC protection and with word programming only
> each OTP are permanently locked when programmed to avoid ECC error
> on the second write operation
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@...s.st.com>
> ---
>
> Changes in v2:
> - rebase series on linux-next/master
> - minor update after V1 revue
> - add missing sentinel in stm32_romem_of_match()
> - reorder function and remove prototypes for stm32_bsec_pta... functions
> - change stm32_bsec_pta_find to static
> - add return value in dev_err()
> - cleanups some comments, which can be on one line
> - remove test on priv->ctx in stm32_bsec_pta_remove
> - add missing tee_shm_free(shm) in stm32_bsec_pta_write() when
> tee_shm_get_va failed
> - return error in stm32_bsec_pta_find when devm_add_action_or_reset failed
> - handle driver_register error in stm32_romem_init
>
> drivers/nvmem/stm32-romem.c | 445 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 441 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvmem/stm32-romem.c b/drivers/nvmem/stm32-romem.c
> index d1d03c2ad081..0a0e29d09b67 100644
> --- a/drivers/nvmem/stm32-romem.c
> +++ b/drivers/nvmem/stm32-romem.c
> @@ -11,6 +11,7 @@
> #include <linux/module.h>
> #include <linux/nvmem-provider.h>
> #include <linux/of_device.h>
> +#include <linux/tee_drv.h>
>
> /* BSEC secure service access from non-secure */
> #define STM32_SMC_BSEC 0x82001003
> @@ -25,14 +26,401 @@
> struct stm32_romem_cfg {
> int size;
> u8 lower;
> + bool ta;
> };
>
> struct stm32_romem_priv {
> void __iomem *base;
> struct nvmem_config cfg;
> u8 lower;
> + struct device *ta;
> };
>
> +#if IS_ENABLED(CONFIG_OPTEE)
> +/*
...
> +
> +static const struct tee_client_device_id stm32_bsec_id_table[] = {
> + {
> + UUID_INIT(0x94cf71ad, 0x80e6, 0x40b5,
> + 0xa7, 0xc6, 0x3d, 0xc5, 0x01, 0xeb, 0x28, 0x03)
> + },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(tee, stm32_bsec_id_table);
> +
> +static struct tee_client_driver stm32_bsec_pta_driver = {
> + .id_table = stm32_bsec_id_table,
> + .driver = {
> + .name = "stm32-bsec-pta",
> + .bus = &tee_bus_type,
> + .probe = stm32_bsec_pta_probe,
> + .remove = stm32_bsec_pta_remove,
> + },
> +};
> +
> +static void stm32_bsec_put_device(void *data)
> +{
> + put_device(data);
> +}
> +
> +static struct device *stm32_bsec_pta_find(struct device *dev)
> +{
> + struct device *pta_dev;
> + int ret;
> +
> + pta_dev = driver_find_next_device(&stm32_bsec_pta_driver.driver, NULL);
This is clearly not representing the dependencies in a proper device model.
If the nvmem provider is a TEE client driver lets model it that way..
brining in a additional device and somehow trying to link it with TEE
driver is a hack.
> +
> + if (pta_dev) {
> + ret = devm_add_action_or_reset(dev, stm32_bsec_put_device, pta_dev);
> + if (ret)
> + dev_err(dev, "devm_add_action_or_reset() failed (%d)\n", ret);
> +
> + return ERR_PTR(ret);
> + }
> +
> + return pta_dev;
> +}
> +
> +#else
> +static int stm32_bsec_pta_read(void *context, unsigned int offset, void *buf,
> + size_t bytes)
> +{
> + pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
> +
> + return -ENXIO;
> +}
> +
> +static int stm32_bsec_pta_write(void *context, unsigned int offset, void *buf,
> + size_t bytes)
> +{
> + pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
> +
> + return -ENXIO;
> +}
> +
> +static struct device *stm32_bsec_pta_find(struct device *dev)
> +{
> + pr_debug("%s: TA BSEC request without OPTEE support\n", __func__);
> +
> + return NULL;
> +}
> +#endif
ifdefing inside the drvier is really ugly, please move this libary
functions to a seperate file and add dependecy properly in Kconfig.
> +
> static int stm32_romem_read(void *context, unsigned int offset, void *buf,
> size_t bytes)
> {
> @@ -173,15 +561,27 @@ static int stm32_romem_probe(struct platform_device *pdev)
> } else {
> priv->cfg.size = cfg->size;
> priv->lower = cfg->lower;
> - priv->cfg.reg_read = stm32_bsec_read;
> - priv->cfg.reg_write = stm32_bsec_write;
> + if (cfg->ta) {
> + priv->ta = stm32_bsec_pta_find(dev);
> + /* wait for OP-TEE client driver to be up and ready */
> + if (!priv->ta)
> + return -EPROBE_DEFER;
> + if (IS_ERR(priv->ta))
> + return PTR_ERR(priv->ta);
> +
> + priv->cfg.reg_read = stm32_bsec_pta_read;
> + priv->cfg.reg_write = stm32_bsec_pta_write;
> + } else {
> + priv->cfg.reg_read = stm32_bsec_read;
> + priv->cfg.reg_write = stm32_bsec_write;
> + }
> }
>
> return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &priv->cfg));
> }
>
> /*
> - * STM32MP15 BSEC OTP regions: 4096 OTP bits (with 3072 effective bits)
> + * STM32MP15/13 BSEC OTP regions: 4096 OTP bits (with 3072 effective bits)
> * => 96 x 32-bits data words
> * - Lower: 1K bits, 2:1 redundancy, incremental bit programming
> * => 32 (x 32-bits) lower shadow registers = words 0 to 31
> @@ -191,6 +591,13 @@ static int stm32_romem_probe(struct platform_device *pdev)
> static const struct stm32_romem_cfg stm32mp15_bsec_cfg = {
> .size = 384,
> .lower = 32,
> + .ta = false,
> +};
> +
> +static const struct stm32_romem_cfg stm32mp13_bsec_cfg = {
> + .size = 384,
> + .lower = 32,
> + .ta = true,
> };
>
> static const struct of_device_id stm32_romem_of_match[] = {
> @@ -198,7 +605,10 @@ static const struct of_device_id stm32_romem_of_match[] = {
> .compatible = "st,stm32mp15-bsec",
> .data = (void *)&stm32mp15_bsec_cfg,
> }, {
> + .compatible = "st,stm32mp13-bsec",
> + .data = (void *)&stm32mp13_bsec_cfg,
> },
> + { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, stm32_romem_of_match);
>
> @@ -209,7 +619,34 @@ static struct platform_driver stm32_romem_driver = {
> .of_match_table = of_match_ptr(stm32_romem_of_match),
> },
> };
> -module_platform_driver(stm32_romem_driver);
> +
> +static int __init stm32_romem_init(void)
> +{
> + int rc;
> +
> + rc = platform_driver_register(&stm32_romem_driver);
> + if (rc)
> + return rc;
> +
> +#if IS_ENABLED(CONFIG_OPTEE)
> + rc = driver_register(&stm32_bsec_pta_driver.driver);
> + if (rc)
> + platform_driver_unregister(&stm32_romem_driver);
> +#endif
> +
> + return rc;
> +}
> +
> +static void __exit stm32_romem_exit(void)
> +{
> + platform_driver_unregister(&stm32_romem_driver);
> +#if IS_ENABLED(CONFIG_OPTEE)
> + driver_unregister(&stm32_bsec_pta_driver.driver);
> +#endif
> +}
> +
> +module_init(stm32_romem_init);
> +module_exit(stm32_romem_exit);
>
> MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@...com>");
> MODULE_DESCRIPTION("STMicroelectronics STM32 RO-MEM");
Powered by blists - more mailing lists