linux-kernel - Re: [PATCH v3 4/5] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write

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:   Tue, 4 Jul 2023 17:43:12 -0700
From:   William Zhang <william.zhang@...adcom.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>,
        Linux MTD List <linux-mtd@...ts.infradead.org>,
        f.fainelli@...il.com, rafal@...ecki.pl, kursad.oney@...adcom.com,
        joel.peshkin@...adcom.com, computersforpeace@...il.com,
        anand.gore@...adcom.com, dregan@...l.com, kamal.dasu@...adcom.com,
        tomer.yacoby@...adcom.com, dan.beygelman@...adcom.com,
        Florian Fainelli <florian.fainelli@...adcom.com>,
        linux-kernel@...r.kernel.org,
        Vignesh Raghavendra <vigneshr@...com>,
        Richard Weinberger <richard@....at>,
        Kamal Dasu <kdasu.kdev@...il.com>
Subject: Re: [PATCH v3 4/5] mtd: rawnand: brcmnand: Fix potential
 out-of-bounds access in oob write

Hi Miquel,

On 07/04/2023 08:28 AM, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@...adcom.com wrote on Tue, 27 Jun 2023 12:37:37 -0700:
> 
>> When the oob buffer length is not in multiple of words, the oob write
>> function does out-of-bounds read on the oob source buffer at the last
>> iteration. Fix that by always checking length limit on the oob buffer
>> read and fill with 0xff when reaching the end of the buffer to the oob
>> registers.
>>
>> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
> 
> Wrong Fixes.
Same here.  The function write_oob_to_regs was added by this commit and 
no change since then.


> 
> Missing Cc stable.
Will add
> 
>> Signed-off-by: William Zhang <william.zhang@...adcom.com>
>> Reviewed-by: Florian Fainelli <florian.fainelli@...adcom.com>
>>
>> ---
>>
>> Changes in v3:
>> - Fix kernel test robot sparse warning:
>>     drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: expected unsigned int [usertype] data
>>     drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: got restricted __be32 [usertype]
>>
>> Changes in v2:
>> - Handle the remaining unaligned oob data after the oob data write loop
>>
>>   drivers/mtd/nand/raw/brcmnand/brcmnand.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> index ea03104692bf..407bf79cbaf4 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> @@ -1477,19 +1477,28 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
>>   			     const u8 *oob, int sas, int sector_1k)
>>   {
>>   	int tbytes = sas << sector_1k;
>> -	int j;
>> +	int j, k = 0;
>> +	u32 last = 0xffffffff;
>> +	u8 *plast = (u8 *)&last;
>>   
>>   	/* Adjust OOB values for 1K sector size */
>>   	if (sector_1k && (i & 0x01))
>>   		tbytes = max(0, tbytes - (int)ctrl->max_oob);
>>   	tbytes = min_t(int, tbytes, ctrl->max_oob);
>>   
>> -	for (j = 0; j < tbytes; j += 4)
>> +	for (j = 0; (j + 3) < tbytes; j += 4)
> 
> Maybe a comment here as well to mention that you stop at the last
> iteration? Otherwise, just reading the line does not make you choice
> obvious.
> 
Will add comment.

>>   		oob_reg_write(ctrl, j,
>>   				(oob[j + 0] << 24) |
>>   				(oob[j + 1] << 16) |
>>   				(oob[j + 2] <<  8) |
>>   				(oob[j + 3] <<  0));
>> +
>> +	while (j < tbytes)
>> +		plast[k++] = oob[j++];
>> +
>> +	if (tbytes & 0x3)
>> +		oob_reg_write(ctrl, (tbytes & ~0x3), (__force u32)cpu_to_be32(last));
>> +
>>   	return tbytes;
>>   }
>>   
> 
> 
> Thanks,
> Miquèl
> 

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4212 bytes)