public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Stefan Roese <sr@denx.de>
Cc: u-boot@lists.denx.de
Subject: [PATCH] ubifs: Fix lockup/crash when reading files
Date: Tue, 17 May 2022 22:45:28 +0200	[thread overview]
Message-ID: <20220517204528.7277-1-pali@kernel.org> (raw)

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 <pali@kernel.org>
---

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.
---
 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;
-- 
2.20.1


             reply	other threads:[~2022-05-17 20:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 20:45 Pali Rohár [this message]
2022-05-19  5:01 ` [PATCH] ubifs: Fix lockup/crash when reading files Stefan Roese
2022-05-19  8:04   ` Pali Rohár
2022-05-23  9:25     ` Pali Rohár
2022-05-30  6:11       ` Stefan Roese
2022-05-30 15:06         ` Tom Rini
2022-06-03 19:48 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220517204528.7277-1-pali@kernel.org \
    --to=pali@kernel.org \
    --cc=sr@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox