linux-kernel - Re: [PATCH v2] UBI: add debugfs file for tracking PEB state

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

Message-ID: <20160921154723.GA16843@zach-desktop>
Date:   Wed, 21 Sep 2016 10:47:23 -0500
From:   Zach Brown <zach.brown@...com>
To:     Richard Weinberger <richard@....at>
CC:     <dwmw2@...radead.org>, <computersforpeace@...il.com>,
        <dedekind1@...il.com>, <linux-mtd@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] UBI: add debugfs file for tracking PEB state

On Wed, Sep 21, 2016 at 01:13:29PM +0200, Richard Weinberger wrote:
> Zach,
>
> On 20.09.2016 22:45, Zach Brown wrote:
> > From: Ben Shelton <ben.shelton@...com>
> >
> > Add a file under debugfs to allow easy access to the erase count for
> > each physical erase block on an UBI device.  This is useful when
> > debugging data integrity issues with UBIFS on NAND flash devices.
> >
> > Signed-off-by: Ben Shelton <ben.shelton@...com>
> > Signed-off-by: Zach Brown <zach.brown@...com>
> > ---
> > v2
> >  * Cast pointer in unsigned long instead of int to avoid build warning
> >  * Use ubi->lookuptbl[] to get erase counter instead of reading from flash
> >
> >
>
> [...]
>
> > +enum block_status {
> > +	BLOCK_STATUS_OK,
> > +	BLOCK_STATUS_BAD_BLOCK,
> > +	BLOCK_STATUS_ERASE_COUNT_BEYOND_MAX
> > +};
>
> Do you plan to add more states?
> In UBI a block can have much more states.
> I'd like to see all states, free, in protection, used, bad, corrupted, scrub, etc...
>

Adding more states sounds like a good idea, but I'm not sure how to get that
information. If I made the in_wl_tree function accessible I could use that to
get the information, but I'd have to check each RB Tree. Do you have a
suggestion?

> AFAIK BLOCK_STATUS_ERASE_COUNT_BEYOND_MAX is also unreachable since UBI aborts before that.
>
Do you mean that the block would've have already been marked as bad? So there's
no point checking?


> What about locking? :-)
> This is racy.
> You need at least wl_lock. Otherwise wl might disappear under you.
> And ->lookuptbl[] can return a NULL object too.
>
Do you know what ->lookuptbl[] returning NULL would signify about the state of
the block? Currently I'm thinking of treating as a bad read status and letting
the show function move on.

> Thanks,
> //richard