linux-kernel - Re: [PATCH v6 12/15] ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when fm->used

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: <e5f5093e-bddc-21d6-df96-d62654cf5b4f@huawei.com>
Date:   Mon, 17 Jan 2022 10:52:41 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     Richard Weinberger <richard@....at>
CC:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Vignesh Raghavendra <vigneshr@...com>,
        mcoquelin stm32 <mcoquelin.stm32@...il.com>,
        "kirill shutemov" <kirill.shutemov@...ux.intel.com>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        linux-mtd <linux-mtd@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 12/15] ubi: fastmap: Add all fastmap pebs into
 'ai->fastmap' when fm->used_blocks>=2

Hi Richard,
>> FYI, I think I understand now our disagreement.
>> You assume that old Fastmap PEBs are *guaranteed* to be part of 
>> Fastmap's erase list.
>> That's okay and this is what Linux as of today does.
>>
>> My point is that we need to be paranoid and check carefully for old 
>> Fastmap PEBs
>> which might be *not* on the erase list.
>> I saw such issues in the wild. These were causes by old and/or buggy 
>> Fastmap
>> implementations.
>> Keep in mind that Linux is not the only system that implements UBI 
>> (and fastmap).
> Uh, that is really a point, I met UBI implemented in Vxworks ever. Now, 
> you convinced me, we should process fastmap with considering bad 
> images(caused by other implementations). Let's keep this wonky assertion 
> until a better fix.
>>
>> So let me give the whole situation another thought on how to improve it.
>> I totally agree with you that currently there is a problem with 
>> fm->used_blocks > 1.
>> I'm just careful (maybe too careful) about changing Fastmap code.
>>I reconsider the WARNON, it can recognize the bad images and fall back 
full scanning mode early. If linux kernel encounters the WARNON, it 
means something wrong with your image(caused by bad UBI implementation). 
I begin to like this strict check.
After comparing WARNON with the assertion:
   WARN_ON(count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count 
- fm->used_blocks)
   ubi_assert(ubi->good_peb_count == found_pebs)
The 'found_pebs' consists of 'ai->erase', 'ai->free', 'ai->volumes' and 
'ai->fastmap'. The count_fastmap_pebs() includes 'ai->erase', 'ai->free' 
and 'ai->volumes'. This means the number of 'ai->fastmap' equals to 
'fm->used_blocks'. So, the 'ai->fastmap' adding position in my fix is right.
In a word, can you accept the point that 'WARNON' can help us recognize 
the bad fastmap images?