linux-kernel - Re: [PATCH v7] mtd: nand: increase ready wait timeout and report timeouts

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: <20151026200531.GE13239@google.com>
Date:	Mon, 26 Oct 2015 13:05:31 -0700
From:	Brian Norris <computersforpeace@...il.com>
To:	Harvey Hunt <harvey.hunt@...tec.com>
Cc:	IMG-MIPSLinuxKerneldevelopers@...tec.com,
	Alex Smith <alex.smith@...tec.com>,
	Alex Smith <alex@...x-smith.me.uk>,
	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Niklas Cassel <niklas.cassel@...s.com>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7] mtd: nand: increase ready wait timeout and report
 timeouts

On Tue, Oct 06, 2015 at 02:52:07PM +0100, Harvey Hunt wrote:
> From: Alex Smith <alex.smith@...tec.com>
> 
> If nand_wait_ready() times out, this is silently ignored, and its
> caller will then proceed to read from/write to the chip before it is
> ready. This can potentially result in corruption with no indication as
> to why.
> 
> While a 20ms timeout seems like it should be plenty enough, certain
> behaviour can cause it to timeout much earlier than expected. The
> situation which prompted this change was that CPU 0, which is
> responsible for updating jiffies, was holding interrupts disabled
> for a fairly long time while writing to the console during a printk,
> causing several jiffies updates to be delayed. If CPU 1 happens to
> enter the timeout loop in nand_wait_ready() just before CPU 0 re-
> enables interrupts and updates jiffies, CPU 1 will immediately time
> out when the delayed jiffies updates are made. The result of this is
> that nand_wait_ready() actually waits less time than the NAND chip
> would normally take to be ready, and then read_page() proceeds to
> read out bad data from the chip.
> 
> The situation described above may seem unlikely, but in fact it can be
> reproduced almost every boot on the MIPS Creator Ci20.
> 
> Therefore, this patch increases the timeout to 400ms. This should be
> enough to cover cases where jiffies updates get delayed. In nand_wait()
> the timeout was previously chosen based on whether erasing or
> programming. This is changed to be 400ms unconditionally as well to
> avoid similar problems there. nand_wait() is also slightly refactored
> to be consistent with nand_wait{,_status}_ready(). These changes should
> have no effect during normal operation.
> 
> Debugging this was made more difficult by the misleading comment above
> nand_wait_ready() stating "The timeout is caught later" - no timeout was
> ever reported, leading me away from the real source of the problem.
> Therefore, a pr_warn() is added when a timeout does occur so that it is
> easier to pinpoint similar problems in future.
> 
> Signed-off-by: Alex Smith <alex.smith@...tec.com>
> Signed-off-by: Harvey Hunt <harvey.hunt@...tec.com>
> Reviewed-by: Niklas Cassel <niklas.cassel@...s.com>
> Cc: Alex Smith <alex@...x-smith.me.uk>
> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>
> Cc: David Woodhouse <dwmw2@...radead.org>
> Cc: Brian Norris <computersforpeace@...il.com>
> Cc: Niklas Cassel <niklas.cassel@...s.com>
> Cc: linux-mtd@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> ---
> This patch was originally sent in a JZ4780 patch set, but sending
> it on its own was deemed more appropriate. Alex Smith sent the
> original patch - this is an unmodified version that he has asked me
> to send.

Looks good, thanks. Pushed to l2-mtd.git.
--
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/