2021-05-17 14:12:51

by Pavel Skripkin

[permalink] [raw]
Subject: [PATCH v2] reiserfs: add check for invalid 1st journal block

syzbot reported divide error in reiserfs.
The problem was in incorrect journal 1st block.

Syzbot's reproducer manualy generated wrong superblock
with incorrect 1st block. In journal_init() wasn't
any checks about this particular case.

For example, if 1st journal block is before superblock
1st block, it can cause zeroing important superblock members
in do_journal_end().

Reported-by: [email protected]
Signed-off-by: Pavel Skripkin <[email protected]>
---

Changes in v2:
Added necessary !SB_ONDISK_JOURNAL_DEVICE(sb) in "if"

---
fs/reiserfs/journal.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 9edc8e2b154e..0834b101c316 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2758,6 +2758,20 @@ int journal_init(struct super_block *sb, const char *j_dev_name,
goto free_and_return;
}

+ /*
+ * Sanity check to see if journal first block is correct.
+ * If journal first block is invalid it can cause
+ * zeroing important superblock members.
+ */
+ if (!SB_ONDISK_JOURNAL_DEVICE(sb) &&
+ SB_ONDISK_JOURNAL_1st_BLOCK(sb) < SB_JOURNAL_1st_RESERVED_BLOCK(sb)) {
+ reiserfs_warning(sb, "journal-1393",
+ "journal 1st super block is invalid: 1st reserved block %d, but actual 1st block is %d",
+ SB_JOURNAL_1st_RESERVED_BLOCK(sb),
+ SB_ONDISK_JOURNAL_1st_BLOCK(sb));
+ goto free_and_return;
+ }
+
if (journal_init_dev(sb, journal, j_dev_name) != 0) {
reiserfs_warning(sb, "sh-462",
"unable to initialize journal device");
--
2.31.1



2021-05-17 19:29:14

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2] reiserfs: add check for invalid 1st journal block

On Mon 17-05-21 15:15:45, Pavel Skripkin wrote:
> syzbot reported divide error in reiserfs.
> The problem was in incorrect journal 1st block.
>
> Syzbot's reproducer manualy generated wrong superblock
> with incorrect 1st block. In journal_init() wasn't
> any checks about this particular case.
>
> For example, if 1st journal block is before superblock
> 1st block, it can cause zeroing important superblock members
> in do_journal_end().
>
> Reported-by: [email protected]
> Signed-off-by: Pavel Skripkin <[email protected]>

Thanks! I've added the patch to my tree.

Honza

> ---
>
> Changes in v2:
> Added necessary !SB_ONDISK_JOURNAL_DEVICE(sb) in "if"
>
> ---
> fs/reiserfs/journal.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
> index 9edc8e2b154e..0834b101c316 100644
> --- a/fs/reiserfs/journal.c
> +++ b/fs/reiserfs/journal.c
> @@ -2758,6 +2758,20 @@ int journal_init(struct super_block *sb, const char *j_dev_name,
> goto free_and_return;
> }
>
> + /*
> + * Sanity check to see if journal first block is correct.
> + * If journal first block is invalid it can cause
> + * zeroing important superblock members.
> + */
> + if (!SB_ONDISK_JOURNAL_DEVICE(sb) &&
> + SB_ONDISK_JOURNAL_1st_BLOCK(sb) < SB_JOURNAL_1st_RESERVED_BLOCK(sb)) {
> + reiserfs_warning(sb, "journal-1393",
> + "journal 1st super block is invalid: 1st reserved block %d, but actual 1st block is %d",
> + SB_JOURNAL_1st_RESERVED_BLOCK(sb),
> + SB_ONDISK_JOURNAL_1st_BLOCK(sb));
> + goto free_and_return;
> + }
> +
> if (journal_init_dev(sb, journal, j_dev_name) != 0) {
> reiserfs_warning(sb, "sh-462",
> "unable to initialize journal device");
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2021-06-08 12:00:13

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2] reiserfs: add check for invalid 1st journal block

On Tue 08-06-21 13:44:43, Pavel Skripkin wrote:
> On Mon, 17 May 2021 15:08:18 +0200
> Jan Kara <[email protected]> wrote:
>
> > On Mon 17-05-21 15:15:45, Pavel Skripkin wrote:
> > > syzbot reported divide error in reiserfs.
> > > The problem was in incorrect journal 1st block.
> > >
> > > Syzbot's reproducer manualy generated wrong superblock
> > > with incorrect 1st block. In journal_init() wasn't
> > > any checks about this particular case.
> > >
> > > For example, if 1st journal block is before superblock
> > > 1st block, it can cause zeroing important superblock members
> > > in do_journal_end().
> > >
> > > Reported-by: [email protected]
> > > Signed-off-by: Pavel Skripkin <[email protected]>
> >
> > Thanks! I've added the patch to my tree.
> >
> > Honza
> >
>
> Hi, Jan!
>
> Is this patched got lost somehow? I did't find it in your tree here
> https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git/.
>
> Sorry to bother you, maybe Im missing something :)

It's sitting in my for_next branch as commit a149127be52f. I'll push it to
Linus in the coming merge window.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2021-06-08 23:42:01

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [PATCH v2] reiserfs: add check for invalid 1st journal block

On Mon, 17 May 2021 15:08:18 +0200
Jan Kara <[email protected]> wrote:

> On Mon 17-05-21 15:15:45, Pavel Skripkin wrote:
> > syzbot reported divide error in reiserfs.
> > The problem was in incorrect journal 1st block.
> >
> > Syzbot's reproducer manualy generated wrong superblock
> > with incorrect 1st block. In journal_init() wasn't
> > any checks about this particular case.
> >
> > For example, if 1st journal block is before superblock
> > 1st block, it can cause zeroing important superblock members
> > in do_journal_end().
> >
> > Reported-by: [email protected]
> > Signed-off-by: Pavel Skripkin <[email protected]>
>
> Thanks! I've added the patch to my tree.
>
> Honza
>

Hi, Jan!

Is this patched got lost somehow? I did't find it in your tree here
https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git/.

Sorry to bother you, maybe Im missing something :)



With regards,
Pavel Skripkin