linux-kernel - Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data

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:   Mon, 22 Mar 2021 16:48:31 +0100
From:   Michael Walle <michael@...le.cc>
To:     Pratyush Yadav <p.yadav@...com>
Cc:     linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
        Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data

Am 2021-03-22 16:32, schrieb Michael Walle:
>>> +
>>> +	sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords));
>> 
>> The SFDP spec says that Parameter Table Pointer should be DWORD 
>> aligned
>> and Parameter Table length is specified in number of DWORDs. So,
>> sfdp_size should always be a multiple of 4. Any SFDP table where this 
>> is
>> not true is an invalid one.
>> 
>> Also, the spec says "Device behavior when the Read SFDP command 
>> crosses
>> the SFDP structure boundary is not defined".
>> 
>> So I think this should be a check for alignment instead of a round-up.
> 
> Well, that woundn't help for debugging. I.e. you also want the SFDP 
> data
> in cases like this. IMHO we should try hard enough to actually get a
> reasonable dump.
> 
> OTOH we also rely on the header and the pointers in the header. Any
> other ideas, but just to chicken out?

Oh, forgot to mention, sfdp_size is used to read the data. I just want
to make sure, the allocated area is large enough. We shouldn't hit the
undefined behavior by reading past the SFDP.

Maybe that check should be part of the parsing code.

-michael