From: Dmitry Vyukov <dvyukov@google.com>
To: syzbot <syzbot+345b75652b1d24227443@syzkaller.appspotmail.com>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>,
clang-built-linux <clang-built-linux@googlegroups.com>,
linux-ext4@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
Nathan Chancellor <natechancellor@gmail.com>,
Nick Desaulniers <ndesaulniers@google.com>,
syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
"Theodore Ts'o" <tytso@mit.edu>
Subject: Re: UBSAN: shift-out-of-bounds in ext4_fill_super
Date: Thu, 10 Dec 2020 14:56:58 +0100 [thread overview]
Message-ID: <CACT4Y+a+ZwwEup7xgfsJth-=T-o-tYNHpVc0m4ePx0fj9LBHZw@mail.gmail.com> (raw)
In-Reply-To: <CACT4Y+bkaVq1RzONGuPJxu-pSyCSRrEs7xV0sa2n0oLNkicHQQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3235 bytes --]
On Thu, Dec 10, 2020 at 9:09 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Thu, Dec 10, 2020 at 4:50 AM syzbot
> <syzbot+345b75652b1d24227443@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot tried to test the proposed patch but the build/boot failed:
> >
> > failed to checkout kernel repo git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git on commit e360ba58d067a30a4e3e7d55ebdd919885a058d6: failed to run ["git" "fetch" "--tags" "d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8"]: exit status 1
> > From git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
> > * [new branch] bisect-test-ext4-035 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/bisect-test-ext4-035
> > * [new branch] bisect-test-generic-307 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/bisect-test-generic-307
> > * [new branch] dev -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/dev
> > * [new branch] ext4-3.18 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-3.18
> > * [new branch] ext4-4.1 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-4.1
> > * [new branch] ext4-4.4 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-4.4
> > * [new branch] ext4-4.9 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-4.9
> > * [new branch] ext4-dax -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-dax
> > * [new branch] ext4-tools -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/ext4-tools
> > * [new branch] fix-bz-206443 -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/fix-bz-206443
> > * [new branch] for-stable -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/for-stable
> > * [new branch] fsverity -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/fsverity
> > * [new branch] lazy_journal -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/lazy_journal
> > * [new branch] master -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/master
> > * [new branch] origin -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/origin
> > * [new branch] pu -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/pu
> > * [new branch] test -> d06f7b29746c7f0a52f349ff7fbf2a3f22d27cf8/test
> > * [new tag] ext4-for-linus-5.8-rc1-2 -> ext4-for-linus-5.8-rc1-2
> > ! [rejected] ext4_for_linus -> ext4_for_linus (would clobber existing tag)
>
> Interesting. First time I see this. Should syzkaller use 'git fetch
> --tags --force"?...
> StackOverflow suggests it should help:
> https://stackoverflow.com/questions/58031165/how-to-get-rid-of-would-clobber-existing-tag
I've added --force to fetches:
https://github.com/google/syzkaller/commit/9a72bc3440b65a01187ba4277b49d6bd821079cd
and it should be deployed by now. Let's try again:
#syz test git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
e360ba58d067a30a4e3e7d55ebdd919885a058d6
[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 3292 bytes --]
From 3d3bc303a8a8f7123cf486f49fa9060116fa1465 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Wed, 9 Dec 2020 15:59:11 -0500
Subject: [PATCH] ext4: check for invalid block size early when mounting a file
system
Check for valid block size directly by validating s_log_block_size; we
were doing this in two places. First, by calculating blocksize via
BLOCK_SIZE << s_log_block_size, and then checking that the blocksize
was valid. And then secondly, by checking s_log_block_size directly.
The first check is not reliable, and can trigger an UBSAN warning if
s_log_block_size on a maliciously corrupted superblock is greater than
22. This is harmless, since the second test will correctly reject the
maliciously fuzzed file system, but to make syzbot shut up, and
because the two checks are duplicative in any case, delete the
blocksize check, and move the s_log_block_size earlier in
ext4_fill_super().
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reported-by: syzbot+345b75652b1d24227443@syzkaller.appspotmail.com
---
fs/ext4/super.c | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f86220a8df50..4a16bbf0432c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4202,18 +4202,25 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
*/
sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT;
- blocksize = BLOCK_SIZE << le32_to_cpu(es->s_log_block_size);
-
- if (blocksize == PAGE_SIZE)
- set_opt(sb, DIOREAD_NOLOCK);
-
- if (blocksize < EXT4_MIN_BLOCK_SIZE ||
- blocksize > EXT4_MAX_BLOCK_SIZE) {
+ if (le32_to_cpu(es->s_log_block_size) >
+ (EXT4_MAX_BLOCK_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) {
ext4_msg(sb, KERN_ERR,
- "Unsupported filesystem blocksize %d (%d log_block_size)",
- blocksize, le32_to_cpu(es->s_log_block_size));
+ "Invalid log block size: %u",
+ le32_to_cpu(es->s_log_block_size));
goto failed_mount;
}
+ if (le32_to_cpu(es->s_log_cluster_size) >
+ (EXT4_MAX_CLUSTER_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) {
+ ext4_msg(sb, KERN_ERR,
+ "Invalid log cluster size: %u",
+ le32_to_cpu(es->s_log_cluster_size));
+ goto failed_mount;
+ }
+
+ blocksize = EXT4_MIN_BLOCK_SIZE << le32_to_cpu(es->s_log_block_size);
+
+ if (blocksize == PAGE_SIZE)
+ set_opt(sb, DIOREAD_NOLOCK);
if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV) {
sbi->s_inode_size = EXT4_GOOD_OLD_INODE_SIZE;
@@ -4432,21 +4439,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
if (!ext4_feature_set_ok(sb, (sb_rdonly(sb))))
goto failed_mount;
- if (le32_to_cpu(es->s_log_block_size) >
- (EXT4_MAX_BLOCK_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) {
- ext4_msg(sb, KERN_ERR,
- "Invalid log block size: %u",
- le32_to_cpu(es->s_log_block_size));
- goto failed_mount;
- }
- if (le32_to_cpu(es->s_log_cluster_size) >
- (EXT4_MAX_CLUSTER_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) {
- ext4_msg(sb, KERN_ERR,
- "Invalid log cluster size: %u",
- le32_to_cpu(es->s_log_cluster_size));
- goto failed_mount;
- }
-
if (le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks) > (blocksize / 4)) {
ext4_msg(sb, KERN_ERR,
"Number of reserved GDT blocks insanely large: %d",
next prev parent reply other threads:[~2020-12-10 13:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-09 7:33 UBSAN: shift-out-of-bounds in ext4_fill_super syzbot
2020-12-10 2:36 ` Theodore Y. Ts'o
2020-12-10 3:50 ` syzbot
2020-12-10 8:09 ` Dmitry Vyukov
2020-12-10 13:56 ` Dmitry Vyukov [this message]
2020-12-10 19:24 ` syzbot
2020-12-10 18:28 ` Theodore Y. Ts'o
2020-12-14 14:37 ` Dmitry Vyukov
2020-12-14 20:36 ` Theodore Y. Ts'o
2020-12-15 10:09 ` Dmitry Vyukov
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='CACT4Y+a+ZwwEup7xgfsJth-=T-o-tYNHpVc0m4ePx0fj9LBHZw@mail.gmail.com' \
--to=dvyukov@google.com \
--cc=adilger.kernel@dilger.ca \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
--cc=syzbot+345b75652b1d24227443@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tytso@mit.edu \
/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;
as well as URLs for NNTP newsgroup(s).