linux-kernel - Re: mtd:nor: unlock enhancement for cmdset0002

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]

Date:   Thu, 11 May 2017 23:22:43 +0200
From:   Honza Petrouš <jpetrous@...il.com>
To:     linux-kernel@...r.kernel.org,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Richard Weinberger <richard@....at>,
        Cyrille Pitchen <cyrille.pitchen@...ev4u.fr>,
        linux-mtd@...ts.infradead.org
Subject: Re: mtd:nor: unlock enhancement for cmdset0002

Sorry for self-answering, but I forgot to add mtd maintainers and
mtd-mailinglist.

/Honza

2017-05-11 12:11 GMT+02:00 Honza Petrouš <jpetrous@...il.com>:
> Hi,
>
> I'm not so much experienced in MTD area, so please correct me, if I'm wrong.
>
> NOR flashes supported by cmdset0002 (AMD & Fujitsu Standard Vendor Command Set)
> are not able to unlock one particular eraseblock, the unlocking is
> done by unlock
> the whole chip. Because of that the driver (cfi_cmdset_0002.c) do it
> in free steps:
>   1) remember all locked eraseblocks
>   2) do chip unlock
>   3) lock all remaining eraseblocks (minus, of course ones which have to be
> enabled by current ioctl call).
>
> Unfortunately, in step 2 (unlocking chip) there is used unlocking over
> all eraseblocks
> which belong to requested unlock area. It means that chip unlock operation
> is doing multiple times, without any reason of doing so. It is enough to do
> unlock only on one eraseblock (as it already means full chip unlock). This way
> we can save programming time, on bigger parts even more dramatically
> (one chip unlock per request vs. [number of eraseblocks] * chip unlock).
>
> So I would like to ask, if my finding is correct.
>
> Thanks
> /Honza
>
> BTW, in current project we have one NOR chip, so when I applied
> the following patch (I know it has to be generalized, even now I
> understand that it can not work correctly in case of multiple NOR chips
> or with different eraseblock sizes, ... etc) I lowered unlocking for 20x
> on 64MB filesystem.
>
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 9dca881..36e45d2 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -2694,8 +2694,8 @@ static int __maybe_unused cfi_ppb_unlock(struct
> mtd_info *mtd, loff_t ofs,
>         }
>
>         /* Now unlock the whole chip */
> -       ret = cfi_varsize_frob(mtd, do_ppb_xxlock, ofs, len,
> -                              DO_XXLOCK_ONEBLOCK_UNLOCK);
> +       ret = do_ppb_xxlock(map, &cfi->chips[0], 0, regions[0].erasesize,
> +                                DO_XXLOCK_ONEBLOCK_UNLOCK);
>         if (ret) {
>                 kfree(sect);
>                 return ret;