On Thursday 19 May 2022 07:01:19 Stefan Roese wrote: > On 17.05.22 22:45, Pali Rohár wrote: > > Commit b1a14f8a1c2e ("UBIFS: Change ubifsload to not read beyond the > > requested size") added optimization to do not read more bytes than it is > > really needed. But this commit introduced incorrect handling of the hole at > > the end of file. This logic cause U-Boot to crash or lockup when trying to > > read from the ubifs filesystem. > > > > When read_block() call returns -ENOENT error (not an error, but the hole) > > then dn-> structure is not filled and contain garbage. So using of dn->size > > for memcpy() argument cause that U-Boot tries to copy unspecified amount of > > bytes from possible unmapped memory. Which randomly cause lockup of P2020 > > CPU. > > > > Fix this issue by copying UBIFS_BLOCK_SIZE bytes from read buffer when > > dn->size is not available. UBIFS_BLOCK_SIZE is the size of the buffer > > itself and read_block() fills buffer by zeros when it returns -ENOENT. > > > > This patch fixes ubifsload on P2020. > > > > Fixes: b1a14f8a1c2e ("UBIFS: Change ubifsload to not read beyond the > > requested size") > > Signed-off-by: Pali Rohár <p...@kernel.org> > > Reviewed-by: Stefan Roese <s...@denx.de> > > > --- > > > > Stefan, could you please look at this patch? Mentioned commit was > > introduced by you more than 11 years ago. I'm surprised that nobody hit > > this issue yet, but it looks like older U-Boot versions somehow filled > > small garbage number into dn->size and did not cause U-Boot > > lockup/crash. > > So long ago. I don't really remember the details. IIRC, I introduced the > UBIFS support for some MIPS based board at that time. My best assumption > is, that UBIFS is rarely used in U-Boot in general.
It is used on Turris 1.x (powerpc). > Thanks, > Stefan > > > --- > > fs/ubifs/ubifs.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c > > index d6be5c947d7e..d3026e310168 100644 > > --- a/fs/ubifs/ubifs.c > > +++ b/fs/ubifs/ubifs.c > > @@ -771,40 +771,42 @@ static int do_readpage(struct ubifs_info *c, struct > > inode *inode, > > buff = malloc_cache_aligned(UBIFS_BLOCK_SIZE); > > if (!buff) { > > printf("%s: Error, malloc fails!\n", > > __func__); > > err = -ENOMEM; > > break; > > } > > /* Read block-size into temp buffer */ > > ret = read_block(inode, buff, block, dn); > > if (ret) { > > err = ret; > > if (err != -ENOENT) { > > free(buff); > > break; > > } > > } > > if (last_block_size) > > dlen = last_block_size; > > + else if (ret) > > + dlen = UBIFS_BLOCK_SIZE; > > else > > dlen = le32_to_cpu(dn->size); > > /* Now copy required size back to dest */ > > memcpy(addr, buff, dlen); > > free(buff); > > } else { > > ret = read_block(inode, addr, block, dn); > > if (ret) { > > err = ret; > > if (err != -ENOENT) > > break; > > } > > } > > } > > if (++i >= UBIFS_BLOCKS_PER_PAGE) > > break; > > block += 1; > > addr += UBIFS_BLOCK_SIZE; > > Viele Grüße, > Stefan Roese > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de