linux-kernel - Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

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]

Date:	Tue, 29 Oct 2013 17:18:21 +0200
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Qi Wang 王起 "(qiwang)" 
	<qiwang@...ron.com>
Cc:	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1]  MTD: UBI:  try to avoid program data to NOR flash
 after erasure interrupted

On Mon, 2013-10-28 at 04:54 +0000, Qi Wang 王起 (qiwang) wrote:
> On Sa, 2013-10-26 at 05:19 +0000, Artem Bityutskiy wrote:
> >On Thu, 2013-10-10 at 08:28 +0000, Qi Wang 王起 (qiwang) wrote:
> >> But I want to say the potential risk is if low level driver program data to 
> >> this block, it will get “timeout error”. And the timeout period could be very 
> >> long(almost several minutes), during this period, any operation on NOR flash 
> >> cannot be accept. so program data to a erasure interrupted block isn’t a 
> >> sensible action. in order to avoid program a erasure interrupted block, 
> >> I suggest UBIFS can read this block before program data. the code changing as below:
> >
> >Yes, reading first sounds like a good idea. Would you please send a
> >patch implementing it?
> 
> From: Qi Wang <qiwang@...ron.com>
> 
> nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
> into a block to mark this block. But program data into a erasure interrupted block
> can cause program timtout(several minutes at most) error, could impact other 
> operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
> program operation. 
> 
> This patch try to put read operation at at head of write operation in 
> nor_erase_prepare(), read out the data. 
> If the data is already corrupt, then no need to program any data into this block, 
> just go to erase this block.
> 
> Signed-off-by: Qi Wang <qiwang@...ang@...ron.com>
> ---
> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
> index bf79def..be6ab56 100644
> --- a/drivers/mtd/ubi/io.c
> +++ b/drivers/mtd/ubi/io.c
> @@ -509,26 +509,10 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
>  	struct ubi_vid_hdr vid_hdr;
>  
>  	/*
> -	 * It is important to first invalidate the EC header, and then the VID
> -	 * header. Otherwise a power cut may lead to valid EC header and
> -	 * invalid VID header, in which case UBI will treat this PEB as
> -	 * corrupted and will try to preserve it, and print scary warnings.
> -	 */
> -	addr = (loff_t)pnum * ubi->peb_size;
> -	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
> -	if (!err) {
> -		addr += ubi->vid_hdr_aloffset;
> -		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
> -		if (!err)
> -			return 0;
> -	}

How about structuring the code this way:

if (EC header is good)
     invalidate EC header()
if (VID header is good)
     invalidate VID header()

Then you'll handle the case when only one of the headers is already
corrupted.

-- 
Best Regards,
Artem Bityutskiy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/