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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ