linux-kernel - Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9bfb10df-64d8-4a5e-b0ad-a8b5f4efc56f@ti.com>
Date:   Wed, 30 Sep 2020 15:06:46 +0530
From:   Vignesh Raghavendra <vigneshr@...com>
To:     Ivan Mikhaylov <i.mikhaylov@...ro.com>,
        Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>
CC:     <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without
 SPI_NOR_HAS_TB



On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> Some chips like macronix don't have TB(Top/Bottom protection)
> bit in the status register. Do not write tb_mask inside status
> register, unless SPI_NOR_HAS_TB is present for the chip.
> 

Not entirely accurate.. Macronix chips have TB bit in config register
and is OTP and hence should not be touched ideally...

You still need to "read" that bit to determine actual scheme (Top vs
Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
flag for macronix flashes.

> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@...ro.com>
> ---
>  drivers/mtd/spi-nor/core.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0369d98b2d12..f9853dd566dc 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1735,13 +1735,18 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>  			return -EINVAL;
>  	}
>  
> -	status_new = (status_old & ~mask & ~tb_mask) | val;
> +	if (nor->flags & SNOR_F_HAS_SR_TB)
> +		status_new = (status_old & ~mask & ~tb_mask) | val;
> +	else
> +		status_new = (status_old & ~mask) | val;
>  
>  	/* Disallow further writes if WP pin is asserted */
>  	status_new |= SR_SRWD;
>  

I guess macronix does not support SR_SRWD right? This needs special
treatment as well.

So either, macronix.c should implements its own locking ops or convert
this function in to more generic library so that its suitable to be
called from macronix.c file while hiding vendor specific stuff in that
driver,

Regards
Vignesh