[PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[Jori Koolstra]

The fs/minix implementation of the minix filesystem does not currently
support any other value for s_log_zone_size than 0. This is also the
only value supported in util-linux; see mkfs.minix.c line 511. In
addition, this patch adds some sanity checking for the other minix
superblock fields, and moves the minix_blocks_needed() checks for the
zmap and imap also to minix_check_super_block().

This also closes a related syzbot bug report.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Reported-by: syzbot+5ad0824204c7bf9b67f2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5ad0824204c7bf9b67f2
---
Changes in v2:
- Moved the minix_blocks_needed() checks for the zmap and imap also
to minix_check_super_block().
---
 fs/minix/inode.c | 50 ++++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 51ea9bdc813f..c8c6b2135abe 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -170,10 +170,38 @@ static int minix_reconfigure(struct fs_context *fc)
 static bool minix_check_superblock(struct super_block *sb)
 {
 	struct minix_sb_info *sbi = minix_sb(sb);
+	unsigned long block;
 
-	if (sbi->s_imap_blocks == 0 || sbi->s_zmap_blocks == 0)
+	if (sbi->s_log_zone_size != 0) {
+		printk("minix-fs error: zone size must equal block size. "
+		       "s_log_zone_size > 0 is not supported.\n");
+		return false;
+	}
+
+	if (sbi->s_ninodes < 1 || sbi->s_firstdatazone <= 4 ||
+	    sbi->s_firstdatazone >= sbi->s_nzones)
 		return false;
 
+	/* Apparently minix can create filesystems that allocate more blocks for
+	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
+	 * create one with not enough blocks and bail out if so.
+	 */
+	block = minix_blocks_needed(sbi->s_ninodes, sb->s_blocksize);
+	if (sbi->s_imap_blocks < block) {
+		printk("MINIX-fs: file system does not have enough "
+		       "imap blocks allocated. Refusing to mount.\n");
+		return false;
+	}
+
+	block = minix_blocks_needed(
+			(sbi->s_nzones - sbi->s_firstdatazone + 1),
+			sb->s_blocksize);
+	if (sbi->s_zmap_blocks < block) {
+		printk("MINIX-fs: file system does not have enough "
+		       "zmap blocks allocated. Refusing to mount.\n");
+		return false;
+	}
+
 	/*
 	 * s_max_size must not exceed the block mapping limitation.  This check
 	 * is only needed for V1 filesystems, since V2/V3 support an extra level
@@ -293,26 +321,6 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 	minix_set_bit(0,sbi->s_imap[0]->b_data);
 	minix_set_bit(0,sbi->s_zmap[0]->b_data);
 
-	/* Apparently minix can create filesystems that allocate more blocks for
-	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
-	 * create one with not enough blocks and bail out if so.
-	 */
-	block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
-	if (sbi->s_imap_blocks < block) {
-		printk("MINIX-fs: file system does not have enough "
-				"imap blocks allocated.  Refusing to mount.\n");
-		goto out_no_bitmap;
-	}
-
-	block = minix_blocks_needed(
-			(sbi->s_nzones - sbi->s_firstdatazone + 1),
-			s->s_blocksize);
-	if (sbi->s_zmap_blocks < block) {
-		printk("MINIX-fs: file system does not have enough "
-				"zmap blocks allocated.  Refusing to mount.\n");
-		goto out_no_bitmap;
-	}
-
 	/* set up enough so that it can read an inode */
 	s->s_op = &minix_sops;
 	s->s_time_min = 0;
-- 
2.51.2

Re: [PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[Jan Kara]

On Mon 08-12-25 16:39:47, Jori Koolstra wrote:
> The fs/minix implementation of the minix filesystem does not currently
> support any other value for s_log_zone_size than 0. This is also the
> only value supported in util-linux; see mkfs.minix.c line 511. In
> addition, this patch adds some sanity checking for the other minix
> superblock fields, and moves the minix_blocks_needed() checks for the
> zmap and imap also to minix_check_super_block().
> 
> This also closes a related syzbot bug report.
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> Reported-by: syzbot+5ad0824204c7bf9b67f2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5ad0824204c7bf9b67f2

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes in v2:
> - Moved the minix_blocks_needed() checks for the zmap and imap also
> to minix_check_super_block().
> ---
>  fs/minix/inode.c | 50 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index 51ea9bdc813f..c8c6b2135abe 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -170,10 +170,38 @@ static int minix_reconfigure(struct fs_context *fc)
>  static bool minix_check_superblock(struct super_block *sb)
>  {
>  	struct minix_sb_info *sbi = minix_sb(sb);
> +	unsigned long block;
>  
> -	if (sbi->s_imap_blocks == 0 || sbi->s_zmap_blocks == 0)
> +	if (sbi->s_log_zone_size != 0) {
> +		printk("minix-fs error: zone size must equal block size. "
> +		       "s_log_zone_size > 0 is not supported.\n");
> +		return false;
> +	}
> +
> +	if (sbi->s_ninodes < 1 || sbi->s_firstdatazone <= 4 ||
> +	    sbi->s_firstdatazone >= sbi->s_nzones)
>  		return false;
>  
> +	/* Apparently minix can create filesystems that allocate more blocks for
> +	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> +	 * create one with not enough blocks and bail out if so.
> +	 */
> +	block = minix_blocks_needed(sbi->s_ninodes, sb->s_blocksize);
> +	if (sbi->s_imap_blocks < block) {
> +		printk("MINIX-fs: file system does not have enough "
> +		       "imap blocks allocated. Refusing to mount.\n");
> +		return false;
> +	}
> +
> +	block = minix_blocks_needed(
> +			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> +			sb->s_blocksize);
> +	if (sbi->s_zmap_blocks < block) {
> +		printk("MINIX-fs: file system does not have enough "
> +		       "zmap blocks allocated. Refusing to mount.\n");
> +		return false;
> +	}
> +
>  	/*
>  	 * s_max_size must not exceed the block mapping limitation.  This check
>  	 * is only needed for V1 filesystems, since V2/V3 support an extra level
> @@ -293,26 +321,6 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  	minix_set_bit(0,sbi->s_imap[0]->b_data);
>  	minix_set_bit(0,sbi->s_zmap[0]->b_data);
>  
> -	/* Apparently minix can create filesystems that allocate more blocks for
> -	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> -	 * create one with not enough blocks and bail out if so.
> -	 */
> -	block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
> -	if (sbi->s_imap_blocks < block) {
> -		printk("MINIX-fs: file system does not have enough "
> -				"imap blocks allocated.  Refusing to mount.\n");
> -		goto out_no_bitmap;
> -	}
> -
> -	block = minix_blocks_needed(
> -			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> -			s->s_blocksize);
> -	if (sbi->s_zmap_blocks < block) {
> -		printk("MINIX-fs: file system does not have enough "
> -				"zmap blocks allocated.  Refusing to mount.\n");
> -		goto out_no_bitmap;
> -	}
> -
>  	/* set up enough so that it can read an inode */
>  	s->s_op = &minix_sops;
>  	s->s_time_min = 0;
> -- 
> 2.51.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[Jori Koolstra]

Thanks for your time and the review Jan.

If someone has time to pull this and https://lkml.org/lkml/2025/12/1/726 in
that would be much appreciated, if there is no further critique :)

Thanks and happy holidays,
Jori.

> Op 09-12-2025 09:48 CET schreef Jan Kara <jack@suse.cz>:
> 
>  
> On Mon 08-12-25 16:39:47, Jori Koolstra wrote:
> > The fs/minix implementation of the minix filesystem does not currently
> > support any other value for s_log_zone_size than 0. This is also the
> > only value supported in util-linux; see mkfs.minix.c line 511. In
> > addition, this patch adds some sanity checking for the other minix
> > superblock fields, and moves the minix_blocks_needed() checks for the
> > zmap and imap also to minix_check_super_block().
> > 
> > This also closes a related syzbot bug report.
> > 
> > Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> > Reported-by: syzbot+5ad0824204c7bf9b67f2@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=5ad0824204c7bf9b67f2
> 
> Looks good. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> 								Honza
> 
> > ---
> > Changes in v2:
> > - Moved the minix_blocks_needed() checks for the zmap and imap also
> > to minix_check_super_block().
> > ---
> >  fs/minix/inode.c | 50 ++++++++++++++++++++++++++++--------------------
> >  1 file changed, 29 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> > index 51ea9bdc813f..c8c6b2135abe 100644
> > --- a/fs/minix/inode.c
> > +++ b/fs/minix/inode.c
> > @@ -170,10 +170,38 @@ static int minix_reconfigure(struct fs_context *fc)
> >  static bool minix_check_superblock(struct super_block *sb)
> >  {
> >  	struct minix_sb_info *sbi = minix_sb(sb);
> > +	unsigned long block;
> >  
> > -	if (sbi->s_imap_blocks == 0 || sbi->s_zmap_blocks == 0)
> > +	if (sbi->s_log_zone_size != 0) {
> > +		printk("minix-fs error: zone size must equal block size. "
> > +		       "s_log_zone_size > 0 is not supported.\n");
> > +		return false;
> > +	}
> > +
> > +	if (sbi->s_ninodes < 1 || sbi->s_firstdatazone <= 4 ||
> > +	    sbi->s_firstdatazone >= sbi->s_nzones)
> >  		return false;
> >  
> > +	/* Apparently minix can create filesystems that allocate more blocks for
> > +	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> > +	 * create one with not enough blocks and bail out if so.
> > +	 */
> > +	block = minix_blocks_needed(sbi->s_ninodes, sb->s_blocksize);
> > +	if (sbi->s_imap_blocks < block) {
> > +		printk("MINIX-fs: file system does not have enough "
> > +		       "imap blocks allocated. Refusing to mount.\n");
> > +		return false;
> > +	}
> > +
> > +	block = minix_blocks_needed(
> > +			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> > +			sb->s_blocksize);
> > +	if (sbi->s_zmap_blocks < block) {
> > +		printk("MINIX-fs: file system does not have enough "
> > +		       "zmap blocks allocated. Refusing to mount.\n");
> > +		return false;
> > +	}
> > +
> >  	/*
> >  	 * s_max_size must not exceed the block mapping limitation.  This check
> >  	 * is only needed for V1 filesystems, since V2/V3 support an extra level
> > @@ -293,26 +321,6 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
> >  	minix_set_bit(0,sbi->s_imap[0]->b_data);
> >  	minix_set_bit(0,sbi->s_zmap[0]->b_data);
> >  
> > -	/* Apparently minix can create filesystems that allocate more blocks for
> > -	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> > -	 * create one with not enough blocks and bail out if so.
> > -	 */
> > -	block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
> > -	if (sbi->s_imap_blocks < block) {
> > -		printk("MINIX-fs: file system does not have enough "
> > -				"imap blocks allocated.  Refusing to mount.\n");
> > -		goto out_no_bitmap;
> > -	}
> > -
> > -	block = minix_blocks_needed(
> > -			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> > -			s->s_blocksize);
> > -	if (sbi->s_zmap_blocks < block) {
> > -		printk("MINIX-fs: file system does not have enough "
> > -				"zmap blocks allocated.  Refusing to mount.\n");
> > -		goto out_no_bitmap;
> > -	}
> > -
> >  	/* set up enough so that it can read an inode */
> >  	s->s_op = &minix_sops;
> >  	s->s_time_min = 0;
> > -- 
> > 2.51.2
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

[PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[Jori Koolstra]

Hello Christian,

If you have time I would greatly appreciate if you can apply this minix patch
reviewed by Jan Kara (and my other minix patch that is also still outstanding).

I would normally not be in a hurry, but it has been outstanding for over
a month and I need to get two more patches upstream to graduate the LFX
Linux Kernel Mentorship program, and I am already overdue. So it would
really help me out if you could take a look.

Thanks,
Jori.

PS. Saw you will speak on VFS at FOSDEM this year. Hope to see you there.

> Op 09-12-2025 09:48 CET schreef Jan Kara <jack@suse.cz>:
> 
>  
> On Mon 08-12-25 16:39:47, Jori Koolstra wrote:
> > The fs/minix implementation of the minix filesystem does not currently
> > support any other value for s_log_zone_size than 0. This is also the
> > only value supported in util-linux; see mkfs.minix.c line 511. In
> > addition, this patch adds some sanity checking for the other minix
> > superblock fields, and moves the minix_blocks_needed() checks for the
> > zmap and imap also to minix_check_super_block().
> > 
> > This also closes a related syzbot bug report.
> > 
> > Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> > Reported-by: syzbot+5ad0824204c7bf9b67f2@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=5ad0824204c7bf9b67f2
> 
> Looks good. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> 								Honza
> 
> > ---
> > Changes in v2:
> > - Moved the minix_blocks_needed() checks for the zmap and imap also
> > to minix_check_super_block().
> > ---
> >  fs/minix/inode.c | 50 ++++++++++++++++++++++++++++--------------------
> >  1 file changed, 29 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> > index 51ea9bdc813f..c8c6b2135abe 100644
> > --- a/fs/minix/inode.c
> > +++ b/fs/minix/inode.c
> > @@ -170,10 +170,38 @@ static int minix_reconfigure(struct fs_context *fc)
> >  static bool minix_check_superblock(struct super_block *sb)
> >  {
> >  	struct minix_sb_info *sbi = minix_sb(sb);
> > +	unsigned long block;
> >  
> > -	if (sbi->s_imap_blocks == 0 || sbi->s_zmap_blocks == 0)
> > +	if (sbi->s_log_zone_size != 0) {
> > +		printk("minix-fs error: zone size must equal block size. "
> > +		       "s_log_zone_size > 0 is not supported.\n");
> > +		return false;
> > +	}
> > +
> > +	if (sbi->s_ninodes < 1 || sbi->s_firstdatazone <= 4 ||
> > +	    sbi->s_firstdatazone >= sbi->s_nzones)
> >  		return false;
> >  
> > +	/* Apparently minix can create filesystems that allocate more blocks for
> > +	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> > +	 * create one with not enough blocks and bail out if so.
> > +	 */
> > +	block = minix_blocks_needed(sbi->s_ninodes, sb->s_blocksize);
> > +	if (sbi->s_imap_blocks < block) {
> > +		printk("MINIX-fs: file system does not have enough "
> > +		       "imap blocks allocated. Refusing to mount.\n");
> > +		return false;
> > +	}
> > +
> > +	block = minix_blocks_needed(
> > +			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> > +			sb->s_blocksize);
> > +	if (sbi->s_zmap_blocks < block) {
> > +		printk("MINIX-fs: file system does not have enough "
> > +		       "zmap blocks allocated. Refusing to mount.\n");
> > +		return false;
> > +	}
> > +
> >  	/*
> >  	 * s_max_size must not exceed the block mapping limitation.  This check
> >  	 * is only needed for V1 filesystems, since V2/V3 support an extra level
> > @@ -293,26 +321,6 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
> >  	minix_set_bit(0,sbi->s_imap[0]->b_data);
> >  	minix_set_bit(0,sbi->s_zmap[0]->b_data);
> >  
> > -	/* Apparently minix can create filesystems that allocate more blocks for
> > -	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
> > -	 * create one with not enough blocks and bail out if so.
> > -	 */
> > -	block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
> > -	if (sbi->s_imap_blocks < block) {
> > -		printk("MINIX-fs: file system does not have enough "
> > -				"imap blocks allocated.  Refusing to mount.\n");
> > -		goto out_no_bitmap;
> > -	}
> > -
> > -	block = minix_blocks_needed(
> > -			(sbi->s_nzones - sbi->s_firstdatazone + 1),
> > -			s->s_blocksize);
> > -	if (sbi->s_zmap_blocks < block) {
> > -		printk("MINIX-fs: file system does not have enough "
> > -				"zmap blocks allocated.  Refusing to mount.\n");
> > -		goto out_no_bitmap;
> > -	}
> > -
> >  	/* set up enough so that it can read an inode */
> >  	s->s_op = &minix_sops;
> >  	s->s_time_min = 0;
> > -- 
> > 2.51.2
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH v2] minix: Add required sanity checking to minix_check_superblock()

[Christian Brauner]

On Mon, 08 Dec 2025 16:39:47 +0100, Jori Koolstra wrote:
> The fs/minix implementation of the minix filesystem does not currently
> support any other value for s_log_zone_size than 0. This is also the
> only value supported in util-linux; see mkfs.minix.c line 511. In
> addition, this patch adds some sanity checking for the other minix
> superblock fields, and moves the minix_blocks_needed() checks for the
> zmap and imap also to minix_check_super_block().
> 
> [...]

Applied to the vfs-7.0.minix branch of the vfs/vfs.git tree.
Patches in the vfs-7.0.minix branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.0.minix

[1/1] minix: Add required sanity checking to minix_check_superblock()
      https://git.kernel.org/vfs/vfs/c/8c97a6ddc956