| 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: <ecde4089-45c2-e9dd-4583-4c9441af3dcd@sancloud.com>
Date: Mon, 6 Dec 2021 11:03:21 +0000
From: Paul Barker <paul.barker@...cloud.com>
To: shiva.linuxworks@...il.com, tudor.ambarus@...rochip.com,
michael@...le.cc, p.yadav@...com, miquel.raynal@...tlin.com,
richard@....at, vigneshr@...com
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
Shivamurthy Shastri <sshivamurthy@...ron.com>
Subject: Re: [PATCH 2/4] mtd: spi-nor: add advanced protection and security
features support
On 27/10/2021 11:33, shiva.linuxworks@...il.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@...ron.com>
>
> Added functionalities to support advanced securtiy and protection
> features in new SPI NOR flashes.
>
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@...ron.com>
> ---
> drivers/mtd/spi-nor/Makefile | 2 +-
> drivers/mtd/spi-nor/advprotsec.c | 209 +++++++++++++++++++++++++++++++
> drivers/mtd/spi-nor/core.c | 2 +
> include/linux/mtd/mtd.h | 19 +++
> 4 files changed, 231 insertions(+), 1 deletion(-)
> create mode 100644 drivers/mtd/spi-nor/advprotsec.c
The changes to drivers/mtd/spi-nor/core.h in patch 1 of this series can
be merged into this patch, with the series re-ordered so this patch is
first.
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 6b904e439372..8e96e2c65c7a 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -spi-nor-objs := core.o sfdp.o swp.o otp.o sysfs.o
> +spi-nor-objs := core.o sfdp.o swp.o otp.o advprotsec.o sysfs.o
> spi-nor-objs += atmel.o
> spi-nor-objs += catalyst.o
> spi-nor-objs += eon.o
> diff --git a/drivers/mtd/spi-nor/advprotsec.c b/drivers/mtd/spi-nor/advprotsec.c
> new file mode 100644
> index 000000000000..4dc8e67b16ef
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/advprotsec.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SPI NOR Advanced Sector Protection and Security Features
> + *
> + * Copyright (C) 2021 Micron Technology, Inc.
> + */
> +
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static int spi_nor_secure_read(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->secure_read(nor, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_secure_write(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->secure_write(nor, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_read_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->read_vlock_bits(nor, addr, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_write_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->write_vlock_bits(nor, addr, len, buf);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_disable(nor);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_read_nvlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->read_nvlock_bits(nor, addr, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_write_nvlock_bits(struct mtd_info *mtd, u32 addr)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->write_nvlock_bits(nor, addr);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_disable(nor);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_erase_nvlock_bits(struct mtd_info *mtd)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->erase_nvlock_bits(nor);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_write_disable(nor);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_read_global_freeze_bits(struct mtd_info *mtd, size_t len,
> + u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->read_global_freeze_bits(nor, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_write_global_freeze_bits(struct mtd_info *mtd, size_t len,
> + u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->write_global_freeze_bits(nor, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +static int spi_nor_read_password(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + int ret;
> +
> + ret = spi_nor_lock_and_prep(nor);
> + if (ret)
> + return ret;
> +
> + ret = nor->params->sec_ops->read_password(nor, len, buf);
> +
> + spi_nor_unlock_and_unprep(nor);
> + return ret;
> +}
> +
> +void spi_nor_register_security_ops(struct spi_nor *nor)
> +{
> + struct mtd_info *mtd = &nor->mtd;
> +
> + if (!nor->params->sec_ops)
> + return;
> +
> + mtd->_secure_packet_read = spi_nor_secure_read;
> + mtd->_secure_packet_write = spi_nor_secure_write;
> + mtd->_read_vlock_bits = spi_nor_read_vlock_bits;
> + mtd->_write_vlock_bits = spi_nor_write_vlock_bits;
> + mtd->_read_nvlock_bits = spi_nor_read_nvlock_bits;
> + mtd->_write_nvlock_bits = spi_nor_write_nvlock_bits;
> + mtd->_erase_nvlock_bits = spi_nor_erase_nvlock_bits;
> + mtd->_read_global_freeze_bits = spi_nor_read_global_freeze_bits;
> + mtd->_write_global_freeze_bits = spi_nor_write_global_freeze_bits;
> + mtd->_read_password = spi_nor_read_password;
This approach requires all or none of the sec_ops functions to be
implemented. It doesn't consider other drivers which may be able to
implement a subset of the sec_ops functions.
I also think it would be better not to use extra function pointers here
and just let other code call the functions defined above directly. The
caller of these functions will need to check that the pointers aren't
NULL before calling them anyway, so I think we may as well call the
functions directly and have each of them check that the corresponding
sec_ops field is non-NULL before calling it.
> +}
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cc08bd707378..864f3c7783b3 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3199,6 +3199,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>
> spi_nor_register_locking_ops(nor);
>
> + spi_nor_register_security_ops(nor);
> +
> /* Send all the required SPI flash commands to initialize device */
> ret = spi_nor_init(nor);
> if (ret)
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 88227044fc86..bce358c9fb94 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -360,6 +360,25 @@ struct mtd_info {
> int (*_get_device) (struct mtd_info *mtd);
> void (*_put_device) (struct mtd_info *mtd);
>
> + /*
> + * Security Operations
> + */
> + int (*_secure_packet_read)(struct mtd_info *mtd, size_t len, u8 *buf);
> + int (*_secure_packet_write)(struct mtd_info *mtd, size_t len, u8 *buf);
> + int (*_read_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf);
> + int (*_write_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf);
> + int (*_read_nvlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> + u8 *buf);
> + int (*_write_nvlock_bits)(struct mtd_info *mtd, u32 addr);
> + int (*_erase_nvlock_bits)(struct mtd_info *mtd);
> + int (*_read_global_freeze_bits)(struct mtd_info *mtd, size_t len,
> + u8 *buf);
> + int (*_write_global_freeze_bits)(struct mtd_info *mtd, size_t len,
> + u8 *buf);
> + int (*_read_password)(struct mtd_info *mtd, size_t len, u8 *buf);
> +
> /*
> * flag indicates a panic write, low level drivers can take appropriate
> * action if required to ensure writes go through
>
Thanks,
--
Paul Barker
Principal Software Engineer
SanCloud Ltd
e: paul.barker@...cloud.com
w: https://sancloud.co.uk/
Download attachment "OpenPGP_0xA67255DFCCE62ECD.asc" of type "application/pgp-keys" (7526 bytes)
Download attachment "OpenPGP_signature" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists