[PATCH] Add error handling to minix filesystem similar to ext4

Jori Koolstra posted 1 patch 3 months, 1 week ago
fs/minix/inode.c        | 237 ++++++++++++++++++++++++++++++++++++----
fs/minix/itree_common.c |   2 +-
fs/minix/minix.h        |  43 ++++++++
fs/minix/namei.c        |  25 +++--
4 files changed, 275 insertions(+), 32 deletions(-)
[PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
This patch sets up generic handling of errors such as filesystem
corruption which are frequently raised by syzbot. Towards this aim it
adds the following mount options to the minix filesystem: errors=
continue/panic/remount-ro and (no)warn-on-error, with semantics
similar to ext4fs. When a minix_error() or minix_error_inode() is
raised, the error is reported and action is taken according to which of
these mount options is set (errors=continue,nowarn-on-error are the
default).

As an examle, this patch fixes a drop_nlink warning in rmdir exposed by
syzbot, originating from a corrupted nlink field of a directory.

The changes were tested using the syzbot reproducer with the various new
mount options. I also handcrafted a similar corrupted fs but with the
minix v3 format (the reproducer uses v1).

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Reported-by: syzbot+4e49728ec1cbaf3b91d2@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=4e49728ec1cbaf3b91d2
---
 fs/minix/inode.c        | 237 ++++++++++++++++++++++++++++++++++++----
 fs/minix/itree_common.c |   2 +-
 fs/minix/minix.h        |  43 ++++++++
 fs/minix/namei.c        |  25 +++--
 4 files changed, 275 insertions(+), 32 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 32db676127a9..62159b7526a2 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -21,10 +21,16 @@
 #include <linux/vfs.h>
 #include <linux/writeback.h>
 #include <linux/fs_context.h>
+#include <linux/fs_parser.h>
+#include <linux/fsnotify.h>
 
 static int minix_write_inode(struct inode *inode,
-		struct writeback_control *wbc);
+			     struct writeback_control *wbc);
 static int minix_statfs(struct dentry *dentry, struct kstatfs *buf);
+static int minix_init_fs_context(struct fs_context *fc);
+static void minix_handle_error(struct super_block *sb, int error,
+			       const char *func, unsigned int line);
+static bool system_going_down(void);
 
 static void minix_evict_inode(struct inode *inode)
 {
@@ -113,17 +119,121 @@ static const struct super_operations minix_sops = {
 	.statfs		= minix_statfs,
 };
 
+void __minix_error(struct super_block *sb, const char *function,
+		   unsigned int line, int error, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	printk(KERN_CRIT "minix-fs error (device %s): %s:%d: comm %s: %pV\n",
+	       sb->s_id, function, line, current->comm, &vaf);
+	va_end(args);
+
+	fsnotify_sb_error(sb, NULL, error ? error : EFSCORRUPTED);
+
+	minix_handle_error(sb, error, function, line);
+}
+
+void __minix_error_inode(struct inode *inode, const char *function,
+			 unsigned int line, int error, u32 block,
+			 const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	if (block)
+		printk(KERN_CRIT "minix-fs error (device %s): %s:%d: "
+		       "inode #%lu: block %du: comm %s: %pV\n",
+		       inode->i_sb->s_id, function, line, inode->i_ino,
+		       block, current->comm, &vaf);
+	else
+		printk(KERN_CRIT "minix-fs error (device %s): %s:%d: "
+		       "inode #%lu: comm %s: %pV\n",
+		       inode->i_sb->s_id, function, line, inode->i_ino,
+		       current->comm, &vaf);
+	va_end(args);
+
+	fsnotify_sb_error(inode->i_sb, NULL, error ? error : EFSCORRUPTED);
+
+	minix_handle_error(inode->i_sb, error, function, line);
+}
+
+static void minix_handle_error(struct super_block *sb, int error,
+			       const char *func, unsigned int line)
+{
+	struct minix_sb_info *sbi = minix_sb(sb);
+
+	if (sbi->s_version != MINIX_V3) {
+		sbi->s_mount_state |= MINIX_ERROR_FS;
+		mark_buffer_dirty(sbi->s_sbh);
+	}
+
+	if (test_opt(sb, WARN_ON_ERROR))
+		WARN_ON_ONCE(1);
+
+	/* Do not panic during 'reboot -f' */
+	if (test_opt(sb, ERRORS_PANIC) && !system_going_down()) {
+		panic("minix-fs (device %s): panic forced after error\n",
+		      sb->s_id);
+	}
+
+	if (test_opt(sb, ERRORS_CONT) || sb_rdonly(sb))
+		return;
+
+	minix_msg(sb, KERN_CRIT, "Remounting filesystem read-only");
+
+	sb->s_flags |= SB_RDONLY;
+}
+
+void __minix_msg(struct super_block *sb,
+		 const char *prefix, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	if (sb)
+		printk("%sminix-fs (%s): %pV\n", prefix, sb->s_id, &vaf);
+	else
+		printk("%sminix-fs: %pV\n", prefix, &vaf);
+	va_end(args);
+}
+
+static bool system_going_down(void)
+{
+	return system_state == SYSTEM_HALT || system_state == SYSTEM_POWER_OFF
+		|| system_state == SYSTEM_RESTART;
+}
+
+struct minix_fs_context {
+	unsigned int s_mount_opt;
+	unsigned int s_def_mount_opt;
+};
+
 static int minix_reconfigure(struct fs_context *fc)
 {
-	struct minix_super_block * ms;
+	struct minix_fs_context *ctx = fc->fs_private;
 	struct super_block *sb = fc->root->d_sb;
-	struct minix_sb_info * sbi = sb->s_fs_info;
+	unsigned int flags = fc->sb_flags;
+	struct minix_sb_info *sbi = minix_sb(sb);
+	struct minix_super_block *ms;
+
+	sbi->s_mount_opt = ctx->s_mount_opt;
 
 	sync_filesystem(sb);
 	ms = sbi->s_ms;
-	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
+
+	if ((bool)(flags & SB_RDONLY) == sb_rdonly(sb))
 		return 0;
-	if (fc->sb_flags & SB_RDONLY) {
+	if (flags & SB_RDONLY) {
 		if (ms->s_state & MINIX_VALID_FS ||
 		    !(sbi->s_mount_state & MINIX_VALID_FS))
 			return 0;
@@ -172,6 +282,7 @@ static bool minix_check_superblock(struct super_block *sb)
 
 static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 {
+	struct minix_fs_context *ctx = fc->fs_private;
 	struct buffer_head *bh;
 	struct buffer_head **map;
 	struct minix_super_block *ms;
@@ -198,6 +309,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 
 	ms = (struct minix_super_block *) bh->b_data;
 	sbi->s_ms = ms;
+	sbi->s_mount_opt = ctx->s_mount_opt;
+	sbi->s_def_mount_opt = ctx->s_def_mount_opt;
 	sbi->s_sbh = bh;
 	sbi->s_mount_state = ms->s_state;
 	sbi->s_ninodes = ms->s_ninodes;
@@ -226,7 +339,7 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 		s->s_max_links = MINIX2_LINK_MAX;
 	} else if (s->s_magic == MINIX2_SUPER_MAGIC2) {
 		sbi->s_version = MINIX_V2;
-		sbi->s_nzones = ms->s_zones;
+	sbi->s_nzones = ms->s_zones;
 		sbi->s_dirsize = 32;
 		sbi->s_namelen = 30;
 		s->s_max_links = MINIX2_LINK_MAX;
@@ -367,8 +480,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 out_bad_sb:
 	printk("MINIX-fs: unable to read superblock\n");
 out:
-	s->s_fs_info = NULL;
 	kfree(sbi);
+	fc->s_fs_info = NULL;
 	return ret;
 }
 
@@ -377,18 +490,6 @@ static int minix_get_tree(struct fs_context *fc)
 	 return get_tree_bdev(fc, minix_fill_super);
 }
 
-static const struct fs_context_operations minix_context_ops = {
-	.get_tree	= minix_get_tree,
-	.reconfigure	= minix_reconfigure,
-};
-
-static int minix_init_fs_context(struct fs_context *fc)
-{
-	fc->ops = &minix_context_ops;
-
-	return 0;
-}
-
 static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct super_block *sb = dentry->d_sb;
@@ -518,11 +619,15 @@ static struct inode *V1_minix_iget(struct inode *inode)
 		return ERR_PTR(-EIO);
 	}
 	if (raw_inode->i_nlinks == 0) {
-		printk("MINIX-fs: deleted inode referenced: %lu\n",
-		       inode->i_ino);
+		minix_error_inode(inode, "deleted inode referenced");
 		brelse(bh);
 		iget_failed(inode);
 		return ERR_PTR(-ESTALE);
+	} else if (S_ISDIR(raw_inode->i_mode) && raw_inode->i_nlinks == 1) {
+		minix_error_inode(inode, "directory inode has corrupted nlink");
+		brelse(bh);
+		iget_failed(inode);
+		return ERR_PTR(-EFSCORRUPTED);
 	}
 	inode->i_mode = raw_inode->i_mode;
 	i_uid_write(inode, raw_inode->i_uid);
@@ -556,11 +661,15 @@ static struct inode *V2_minix_iget(struct inode *inode)
 		return ERR_PTR(-EIO);
 	}
 	if (raw_inode->i_nlinks == 0) {
-		printk("MINIX-fs: deleted inode referenced: %lu\n",
-		       inode->i_ino);
+		minix_error_inode(inode, "deleted inode referenced");
 		brelse(bh);
 		iget_failed(inode);
 		return ERR_PTR(-ESTALE);
+	} else if (S_ISDIR(raw_inode->i_mode) && raw_inode->i_nlinks == 1) {
+		minix_error_inode(inode, "directory inode has corrupted nlink");
+		brelse(bh);
+		iget_failed(inode);
+		return ERR_PTR(-EFSCORRUPTED);
 	}
 	inode->i_mode = raw_inode->i_mode;
 	i_uid_write(inode, raw_inode->i_uid);
@@ -705,13 +814,95 @@ void minix_truncate(struct inode * inode)
 		V2_minix_truncate(inode);
 }
 
+enum {
+	Opt_errors, Opt_warn_on_error, Opt_nowarn_on_error
+};
+
+static const struct constant_table minix_param_errors[] = {
+	{"continue",	MINIX_MOUNT_ERRORS_CONT},
+	{"panic",	MINIX_MOUNT_ERRORS_PANIC},
+	{"remount-ro",	MINIX_MOUNT_ERRORS_RO},
+	{}
+};
+
+/*
+ * Mount option specification
+ */
+static const struct fs_parameter_spec minix_param_specs[] = {
+	fsparam_enum	("errors",		Opt_errors, minix_param_errors),
+	fsparam_flag	("warn-on-error",	Opt_warn_on_error),
+	fsparam_flag	("nowarn-on-error",	Opt_nowarn_on_error),
+	{}
+};
+
+static int minix_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct minix_fs_context *ctx = fc->fs_private;
+	struct fs_parse_result result;
+
+	int token;
+
+	token = fs_parse(fc, minix_param_specs, param, &result);
+	if (token < 0)
+		return token;
+
+	switch (token) {
+	case Opt_errors:
+		ctx->s_mount_opt &= ~MINIX_MOUNT_ERRORS_MASK;
+		ctx->s_mount_opt |= result.uint_32;
+		break;
+	case Opt_warn_on_error:
+		ctx->s_mount_opt |= MINIX_MOUNT_WARN_ON_ERROR;
+		break;
+	case Opt_nowarn_on_error:
+		ctx->s_mount_opt &= ~MINIX_MOUNT_WARN_ON_ERROR;
+		break;
+	}
+	return 0;
+}
+
+static void minix_fc_free(struct fs_context *fc)
+{
+	struct minix_fs_context *ctx = fc->fs_private;
+
+	if (!ctx)
+		return;
+	kfree(ctx);
+}
+
+static const struct fs_context_operations minix_context_ops = {
+	.get_tree	= minix_get_tree,
+	.reconfigure	= minix_reconfigure,
+	.parse_param	= minix_parse_param,
+	.free		= minix_fc_free,
+};
+
+int minix_init_fs_context(struct fs_context *fc)
+{
+	struct minix_fs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct minix_fs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	fc->fs_private = ctx;
+	fc->ops = &minix_context_ops;
+
+	ctx->s_def_mount_opt |= MINIX_MOUNT_ERRORS_DEF;
+	ctx->s_mount_opt = ctx->s_def_mount_opt;
+
+	return 0;
+}
+
 static struct file_system_type minix_fs_type = {
 	.owner			= THIS_MODULE,
 	.name			= "minix",
 	.kill_sb		= kill_block_super,
 	.fs_flags		= FS_REQUIRES_DEV,
 	.init_fs_context	= minix_init_fs_context,
+	.parameters		= minix_param_specs,
 };
+
 MODULE_ALIAS_FS("minix");
 
 static int __init init_minix_fs(void)
diff --git a/fs/minix/itree_common.c b/fs/minix/itree_common.c
index dad131e30c05..6832c671125e 100644
--- a/fs/minix/itree_common.c
+++ b/fs/minix/itree_common.c
@@ -188,7 +188,7 @@ static int get_block(struct inode * inode, sector_t block,
 	/*
 	 * Indirect block might be removed by truncate while we were
 	 * reading it. Handling of that case (forget what we've got and
-	 * reread) is taken out of the main path.
+	 * reread) is taken out of the msb_breadain path.
 	 */
 	if (err == -EAGAIN)
 		goto changed;
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index d54273c3c9ff..254220ffbf39 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -11,6 +11,22 @@
 #define MINIX_V2		0x0002		/* minix V2 fs */
 #define MINIX_V3		0x0003		/* minix V3 fs */
 
+#define MINIX_MOUNT_ERRORS_CONT		0x00001	/* Continue on errors */
+#define MINIX_MOUNT_ERRORS_RO		0x00002	/* Remount fs ro on errors */
+#define MINIX_MOUNT_ERRORS_PANIC	0x00004	/* Panic on errors */
+#define MINIX_MOUNT_WARN_ON_ERROR	0x00008 /* Trigger WARN_ON on error */
+
+#define MINIX_MOUNT_ERRORS_MASK		0x00007
+
+#define MINIX_MOUNT_ERRORS_DEF		MINIX_MOUNT_ERRORS_CONT
+
+#define clear_opt(sb, opt)		minix_sb(sb)->s_mount_opt &= \
+						~MINIX_MOUNT_##opt
+#define set_opt(sb, opt)		minix_sb(sb)->s_mount_opt |= \
+						MINIX_MOUNT_##opt
+#define test_opt(sb, opt)		(minix_sb(sb)->s_mount_opt & \
+						MINIX_MOUNT_##opt)
+
 /*
  * minix fs inode data in memory
  */
@@ -39,6 +55,8 @@ struct minix_sb_info {
 	struct buffer_head * s_sbh;
 	struct minix_super_block * s_ms;
 	unsigned short s_mount_state;
+	unsigned short s_mount_opt;
+	unsigned short s_def_mount_opt;
 	unsigned short s_version;
 };
 
@@ -55,6 +73,13 @@ int minix_getattr(struct mnt_idmap *, const struct path *,
 		struct kstat *, u32, unsigned int);
 int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len);
 
+extern __printf(3, 4)
+void __minix_msg(struct super_block *, const char *, const char *, ...);
+void __minix_error(struct super_block *, const char *, unsigned int, int,
+		   const char *, ...);
+void __minix_error_inode(struct inode *, const char *, unsigned int, int, u32,
+			 const char *, ...);
+
 extern void V1_minix_truncate(struct inode *);
 extern void V2_minix_truncate(struct inode *);
 extern void minix_truncate(struct inode *);
@@ -168,4 +193,22 @@ static inline int minix_test_bit(int nr, const void *vaddr)
 
 #endif
 
+#define minix_error(sb, fmt, ...)						\
+	__minix_error((sb), __func__, __LINE__, 0, (fmt), ##__VA_ARGS__)
+#define minix_error_err(sb, err, fmt, ...)					\
+	__minix_error((sb), __func__, __LINE__, (err), (fmt), ##__VA_ARGS__)
+#define minix_error_inode(inode, fmt, ...)					\
+	__minix_error_inode((inode), __func__, __LINE__, 0, 0,			\
+			    (fmt), ##__VA_ARGS__)
+#define minix_error_inode_err(inode, err, fmt, ...)				\
+	__minix_error_inode((inode), __func__, __LINE__, (err), 0,		\
+			    (fmt), ##__VA_ARGS__)
+#define minix_error_inode_block(inode, block, err, fmt, ...)			\
+	__minix_error_inode((inode), __func__, __LINE__, (err), (block),	\
+			    (fmt), ##__VA_ARGS__)
+#define minix_msg(sb, level, fmt, ...)				\
+	__minix_msg(sb, level, fmt, ##__VA_ARGS__)
+
+#define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
+
 #endif /* FS_MINIX_H */
diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index 8938536d8d3c..23f98f44e262 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -161,15 +161,24 @@ static int minix_unlink(struct inode * dir, struct dentry *dentry)
 static int minix_rmdir(struct inode * dir, struct dentry *dentry)
 {
 	struct inode * inode = d_inode(dentry);
-	int err = -ENOTEMPTY;
-
-	if (minix_empty_dir(inode)) {
-		err = minix_unlink(dir, dentry);
-		if (!err) {
-			inode_dec_link_count(dir);
-			inode_dec_link_count(inode);
-		}
+	int err = -EFSCORRUPTED;
+
+	if (dir->i_nlink <= 2) {
+		minix_error_inode(inode, "directory inode has corrupted nlink");
+		goto out;
 	}
+
+	err = -ENOTEMPTY;
+	if (!minix_empty_dir(inode))
+		goto out;
+
+	err = minix_unlink(dir, dentry);
+	if (!err) {
+		inode_dec_link_count(dir);
+		inode_dec_link_count(inode);
+ 	}
+
+out:
 	return err;
 }
 
-- 
2.51.1.dirty
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by kernel test robot 3 months, 1 week ago
Hi Jori,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on linus/master v6.18-rc3 next-20251031]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jori-Koolstra/Add-error-handling-to-minix-filesystem-similar-to-ext4/20251029-050224
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20251028205857.386719-1-jkoolstra%40xs4all.nl
patch subject: [PATCH] Add error handling to minix filesystem similar to ext4
config: x86_64-randconfig-161-20251101 (https://download.01.org/0day-ci/archive/20251102/202511020315.RrrqmaRR-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511020315.RrrqmaRR-lkp@intel.com/

smatch warnings:
fs/minix/inode.c:342 minix_fill_super() warn: inconsistent indenting

vim +342 fs/minix/inode.c

270ef41094e9fa Eric Biggers       2020-08-11  282  
7cd7bfe5932874 Bill O'Donnell     2024-03-07  283  static int minix_fill_super(struct super_block *s, struct fs_context *fc)
^1da177e4c3f41 Linus Torvalds     2005-04-16  284  {
e54e25638fc1ea Jori Koolstra      2025-10-28  285  	struct minix_fs_context *ctx = fc->fs_private;
^1da177e4c3f41 Linus Torvalds     2005-04-16  286  	struct buffer_head *bh;
^1da177e4c3f41 Linus Torvalds     2005-04-16  287  	struct buffer_head **map;
^1da177e4c3f41 Linus Torvalds     2005-04-16  288  	struct minix_super_block *ms;
939b00df0306bc Andries Brouwer    2007-02-12  289  	struct minix3_super_block *m3s = NULL;
939b00df0306bc Andries Brouwer    2007-02-12  290  	unsigned long i, block;
^1da177e4c3f41 Linus Torvalds     2005-04-16  291  	struct inode *root_inode;
^1da177e4c3f41 Linus Torvalds     2005-04-16  292  	struct minix_sb_info *sbi;
a90a088021f8f1 David Howells      2008-02-07  293  	int ret = -EINVAL;
7cd7bfe5932874 Bill O'Donnell     2024-03-07  294  	int silent = fc->sb_flags & SB_SILENT;
^1da177e4c3f41 Linus Torvalds     2005-04-16  295  
f8314dc60ccba7 Panagiotis Issaris 2006-09-27  296  	sbi = kzalloc(sizeof(struct minix_sb_info), GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds     2005-04-16  297  	if (!sbi)
^1da177e4c3f41 Linus Torvalds     2005-04-16  298  		return -ENOMEM;
^1da177e4c3f41 Linus Torvalds     2005-04-16  299  	s->s_fs_info = sbi;
^1da177e4c3f41 Linus Torvalds     2005-04-16  300  
2ecd05ae68a903 Alexey Dobriyan    2006-10-11  301  	BUILD_BUG_ON(32 != sizeof (struct minix_inode));
2ecd05ae68a903 Alexey Dobriyan    2006-10-11  302  	BUILD_BUG_ON(64 != sizeof(struct minix2_inode));
^1da177e4c3f41 Linus Torvalds     2005-04-16  303  
^1da177e4c3f41 Linus Torvalds     2005-04-16  304  	if (!sb_set_blocksize(s, BLOCK_SIZE))
^1da177e4c3f41 Linus Torvalds     2005-04-16  305  		goto out_bad_hblock;
^1da177e4c3f41 Linus Torvalds     2005-04-16  306  
^1da177e4c3f41 Linus Torvalds     2005-04-16  307  	if (!(bh = sb_bread(s, 1)))
^1da177e4c3f41 Linus Torvalds     2005-04-16  308  		goto out_bad_sb;
^1da177e4c3f41 Linus Torvalds     2005-04-16  309  
^1da177e4c3f41 Linus Torvalds     2005-04-16  310  	ms = (struct minix_super_block *) bh->b_data;
^1da177e4c3f41 Linus Torvalds     2005-04-16  311  	sbi->s_ms = ms;
e54e25638fc1ea Jori Koolstra      2025-10-28  312  	sbi->s_mount_opt = ctx->s_mount_opt;
e54e25638fc1ea Jori Koolstra      2025-10-28  313  	sbi->s_def_mount_opt = ctx->s_def_mount_opt;
^1da177e4c3f41 Linus Torvalds     2005-04-16  314  	sbi->s_sbh = bh;
^1da177e4c3f41 Linus Torvalds     2005-04-16  315  	sbi->s_mount_state = ms->s_state;
^1da177e4c3f41 Linus Torvalds     2005-04-16  316  	sbi->s_ninodes = ms->s_ninodes;
^1da177e4c3f41 Linus Torvalds     2005-04-16  317  	sbi->s_nzones = ms->s_nzones;
^1da177e4c3f41 Linus Torvalds     2005-04-16  318  	sbi->s_imap_blocks = ms->s_imap_blocks;
^1da177e4c3f41 Linus Torvalds     2005-04-16  319  	sbi->s_zmap_blocks = ms->s_zmap_blocks;
^1da177e4c3f41 Linus Torvalds     2005-04-16  320  	sbi->s_firstdatazone = ms->s_firstdatazone;
^1da177e4c3f41 Linus Torvalds     2005-04-16  321  	sbi->s_log_zone_size = ms->s_log_zone_size;
32ac86efff91a3 Eric Biggers       2020-08-11  322  	s->s_maxbytes = ms->s_max_size;
^1da177e4c3f41 Linus Torvalds     2005-04-16  323  	s->s_magic = ms->s_magic;
^1da177e4c3f41 Linus Torvalds     2005-04-16  324  	if (s->s_magic == MINIX_SUPER_MAGIC) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  325  		sbi->s_version = MINIX_V1;
^1da177e4c3f41 Linus Torvalds     2005-04-16  326  		sbi->s_dirsize = 16;
^1da177e4c3f41 Linus Torvalds     2005-04-16  327  		sbi->s_namelen = 14;
8de52778798fe3 Al Viro            2012-02-06  328  		s->s_max_links = MINIX_LINK_MAX;
^1da177e4c3f41 Linus Torvalds     2005-04-16  329  	} else if (s->s_magic == MINIX_SUPER_MAGIC2) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  330  		sbi->s_version = MINIX_V1;
^1da177e4c3f41 Linus Torvalds     2005-04-16  331  		sbi->s_dirsize = 32;
^1da177e4c3f41 Linus Torvalds     2005-04-16  332  		sbi->s_namelen = 30;
8de52778798fe3 Al Viro            2012-02-06  333  		s->s_max_links = MINIX_LINK_MAX;
^1da177e4c3f41 Linus Torvalds     2005-04-16  334  	} else if (s->s_magic == MINIX2_SUPER_MAGIC) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  335  		sbi->s_version = MINIX_V2;
^1da177e4c3f41 Linus Torvalds     2005-04-16  336  		sbi->s_nzones = ms->s_zones;
^1da177e4c3f41 Linus Torvalds     2005-04-16  337  		sbi->s_dirsize = 16;
^1da177e4c3f41 Linus Torvalds     2005-04-16  338  		sbi->s_namelen = 14;
8de52778798fe3 Al Viro            2012-02-06  339  		s->s_max_links = MINIX2_LINK_MAX;
^1da177e4c3f41 Linus Torvalds     2005-04-16  340  	} else if (s->s_magic == MINIX2_SUPER_MAGIC2) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  341  		sbi->s_version = MINIX_V2;
^1da177e4c3f41 Linus Torvalds     2005-04-16 @342  	sbi->s_nzones = ms->s_zones;
^1da177e4c3f41 Linus Torvalds     2005-04-16  343  		sbi->s_dirsize = 32;
^1da177e4c3f41 Linus Torvalds     2005-04-16  344  		sbi->s_namelen = 30;
8de52778798fe3 Al Viro            2012-02-06  345  		s->s_max_links = MINIX2_LINK_MAX;
939b00df0306bc Andries Brouwer    2007-02-12  346  	} else if ( *(__u16 *)(bh->b_data + 24) == MINIX3_SUPER_MAGIC) {
939b00df0306bc Andries Brouwer    2007-02-12  347  		m3s = (struct minix3_super_block *) bh->b_data;
939b00df0306bc Andries Brouwer    2007-02-12  348  		s->s_magic = m3s->s_magic;
939b00df0306bc Andries Brouwer    2007-02-12  349  		sbi->s_imap_blocks = m3s->s_imap_blocks;
939b00df0306bc Andries Brouwer    2007-02-12  350  		sbi->s_zmap_blocks = m3s->s_zmap_blocks;
939b00df0306bc Andries Brouwer    2007-02-12  351  		sbi->s_firstdatazone = m3s->s_firstdatazone;
939b00df0306bc Andries Brouwer    2007-02-12  352  		sbi->s_log_zone_size = m3s->s_log_zone_size;
32ac86efff91a3 Eric Biggers       2020-08-11  353  		s->s_maxbytes = m3s->s_max_size;
939b00df0306bc Andries Brouwer    2007-02-12  354  		sbi->s_ninodes = m3s->s_ninodes;
939b00df0306bc Andries Brouwer    2007-02-12  355  		sbi->s_nzones = m3s->s_zones;
939b00df0306bc Andries Brouwer    2007-02-12  356  		sbi->s_dirsize = 64;
939b00df0306bc Andries Brouwer    2007-02-12  357  		sbi->s_namelen = 60;
939b00df0306bc Andries Brouwer    2007-02-12  358  		sbi->s_version = MINIX_V3;
939b00df0306bc Andries Brouwer    2007-02-12  359  		sbi->s_mount_state = MINIX_VALID_FS;
939b00df0306bc Andries Brouwer    2007-02-12  360  		sb_set_blocksize(s, m3s->s_blocksize);
8de52778798fe3 Al Viro            2012-02-06  361  		s->s_max_links = MINIX2_LINK_MAX;
^1da177e4c3f41 Linus Torvalds     2005-04-16  362  	} else
^1da177e4c3f41 Linus Torvalds     2005-04-16  363  		goto out_no_fs;
^1da177e4c3f41 Linus Torvalds     2005-04-16  364  
32ac86efff91a3 Eric Biggers       2020-08-11  365  	if (!minix_check_superblock(s))
270ef41094e9fa Eric Biggers       2020-08-11  366  		goto out_illegal_sb;
270ef41094e9fa Eric Biggers       2020-08-11  367  
^1da177e4c3f41 Linus Torvalds     2005-04-16  368  	/*
^1da177e4c3f41 Linus Torvalds     2005-04-16  369  	 * Allocate the buffer map to keep the superblock small.
^1da177e4c3f41 Linus Torvalds     2005-04-16  370  	 */
^1da177e4c3f41 Linus Torvalds     2005-04-16  371  	i = (sbi->s_imap_blocks + sbi->s_zmap_blocks) * sizeof(bh);
f8314dc60ccba7 Panagiotis Issaris 2006-09-27  372  	map = kzalloc(i, GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds     2005-04-16  373  	if (!map)
^1da177e4c3f41 Linus Torvalds     2005-04-16  374  		goto out_no_map;
^1da177e4c3f41 Linus Torvalds     2005-04-16  375  	sbi->s_imap = &map[0];
^1da177e4c3f41 Linus Torvalds     2005-04-16  376  	sbi->s_zmap = &map[sbi->s_imap_blocks];
^1da177e4c3f41 Linus Torvalds     2005-04-16  377  
^1da177e4c3f41 Linus Torvalds     2005-04-16  378  	block=2;
^1da177e4c3f41 Linus Torvalds     2005-04-16  379  	for (i=0 ; i < sbi->s_imap_blocks ; i++) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  380  		if (!(sbi->s_imap[i]=sb_bread(s, block)))
^1da177e4c3f41 Linus Torvalds     2005-04-16  381  			goto out_no_bitmap;
^1da177e4c3f41 Linus Torvalds     2005-04-16  382  		block++;
^1da177e4c3f41 Linus Torvalds     2005-04-16  383  	}
^1da177e4c3f41 Linus Torvalds     2005-04-16  384  	for (i=0 ; i < sbi->s_zmap_blocks ; i++) {
^1da177e4c3f41 Linus Torvalds     2005-04-16  385  		if (!(sbi->s_zmap[i]=sb_bread(s, block)))
^1da177e4c3f41 Linus Torvalds     2005-04-16  386  			goto out_no_bitmap;
^1da177e4c3f41 Linus Torvalds     2005-04-16  387  		block++;
^1da177e4c3f41 Linus Torvalds     2005-04-16  388  	}
^1da177e4c3f41 Linus Torvalds     2005-04-16  389  
^1da177e4c3f41 Linus Torvalds     2005-04-16  390  	minix_set_bit(0,sbi->s_imap[0]->b_data);
^1da177e4c3f41 Linus Torvalds     2005-04-16  391  	minix_set_bit(0,sbi->s_zmap[0]->b_data);
^1da177e4c3f41 Linus Torvalds     2005-04-16  392  
016e8d44bc06dd Josh Boyer         2011-08-19  393  	/* Apparently minix can create filesystems that allocate more blocks for
016e8d44bc06dd Josh Boyer         2011-08-19  394  	 * the bitmaps than needed.  We simply ignore that, but verify it didn't
016e8d44bc06dd Josh Boyer         2011-08-19  395  	 * create one with not enough blocks and bail out if so.
016e8d44bc06dd Josh Boyer         2011-08-19  396  	 */
016e8d44bc06dd Josh Boyer         2011-08-19  397  	block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
016e8d44bc06dd Josh Boyer         2011-08-19  398  	if (sbi->s_imap_blocks < block) {
016e8d44bc06dd Josh Boyer         2011-08-19  399  		printk("MINIX-fs: file system does not have enough "
6d6747f8531468 Qi Yong            2014-08-08  400  				"imap blocks allocated.  Refusing to mount.\n");
ca85c07809ca19 Al Viro            2012-02-12  401  		goto out_no_bitmap;
016e8d44bc06dd Josh Boyer         2011-08-19  402  	}
016e8d44bc06dd Josh Boyer         2011-08-19  403  
016e8d44bc06dd Josh Boyer         2011-08-19  404  	block = minix_blocks_needed(
6d6747f8531468 Qi Yong            2014-08-08  405  			(sbi->s_nzones - sbi->s_firstdatazone + 1),
016e8d44bc06dd Josh Boyer         2011-08-19  406  			s->s_blocksize);
016e8d44bc06dd Josh Boyer         2011-08-19  407  	if (sbi->s_zmap_blocks < block) {
016e8d44bc06dd Josh Boyer         2011-08-19  408  		printk("MINIX-fs: file system does not have enough "
016e8d44bc06dd Josh Boyer         2011-08-19  409  				"zmap blocks allocated.  Refusing to mount.\n");
ca85c07809ca19 Al Viro            2012-02-12  410  		goto out_no_bitmap;
ca85c07809ca19 Al Viro            2012-02-12  411  	}
ca85c07809ca19 Al Viro            2012-02-12  412  
ca85c07809ca19 Al Viro            2012-02-12  413  	/* set up enough so that it can read an inode */
ca85c07809ca19 Al Viro            2012-02-12  414  	s->s_op = &minix_sops;
22b139691f9eb8 Deepa Dinamani     2019-07-30  415  	s->s_time_min = 0;
22b139691f9eb8 Deepa Dinamani     2019-07-30  416  	s->s_time_max = U32_MAX;
ca85c07809ca19 Al Viro            2012-02-12  417  	root_inode = minix_iget(s, MINIX_ROOT_INO);
ca85c07809ca19 Al Viro            2012-02-12  418  	if (IS_ERR(root_inode)) {
ca85c07809ca19 Al Viro            2012-02-12  419  		ret = PTR_ERR(root_inode);
ca85c07809ca19 Al Viro            2012-02-12  420  		goto out_no_root;
016e8d44bc06dd Josh Boyer         2011-08-19  421  	}
016e8d44bc06dd Josh Boyer         2011-08-19  422  
d6042eac44b54d Al Viro            2012-01-04  423  	ret = -ENOMEM;
ca85c07809ca19 Al Viro            2012-02-12  424  	s->s_root = d_make_root(root_inode);
d6042eac44b54d Al Viro            2012-01-04  425  	if (!s->s_root)
ca85c07809ca19 Al Viro            2012-02-12  426  		goto out_no_root;
d6042eac44b54d Al Viro            2012-01-04  427  
bc98a42c1f7d0f David Howells      2017-07-17  428  	if (!sb_rdonly(s)) {
d6042eac44b54d Al Viro            2012-01-04  429  		if (sbi->s_version != MINIX_V3) /* s_state is now out from V3 sb */
d6042eac44b54d Al Viro            2012-01-04  430  			ms->s_state &= ~MINIX_VALID_FS;
d6042eac44b54d Al Viro            2012-01-04  431  		mark_buffer_dirty(bh);
d6042eac44b54d Al Viro            2012-01-04  432  	}
d6042eac44b54d Al Viro            2012-01-04  433  	if (!(sbi->s_mount_state & MINIX_VALID_FS))
d6042eac44b54d Al Viro            2012-01-04  434  		printk("MINIX-fs: mounting unchecked file system, "
d6042eac44b54d Al Viro            2012-01-04  435  			"running fsck is recommended\n");
d6042eac44b54d Al Viro            2012-01-04  436  	else if (sbi->s_mount_state & MINIX_ERROR_FS)
d6042eac44b54d Al Viro            2012-01-04  437  		printk("MINIX-fs: mounting file system with errors, "
d6042eac44b54d Al Viro            2012-01-04  438  			"running fsck is recommended\n");
d6042eac44b54d Al Viro            2012-01-04  439  
^1da177e4c3f41 Linus Torvalds     2005-04-16  440  	return 0;
^1da177e4c3f41 Linus Torvalds     2005-04-16  441  
^1da177e4c3f41 Linus Torvalds     2005-04-16  442  out_no_root:
^1da177e4c3f41 Linus Torvalds     2005-04-16  443  	if (!silent)
^1da177e4c3f41 Linus Torvalds     2005-04-16  444  		printk("MINIX-fs: get root inode failed\n");
^1da177e4c3f41 Linus Torvalds     2005-04-16  445  	goto out_freemap;
^1da177e4c3f41 Linus Torvalds     2005-04-16  446  
^1da177e4c3f41 Linus Torvalds     2005-04-16  447  out_no_bitmap:
^1da177e4c3f41 Linus Torvalds     2005-04-16  448  	printk("MINIX-fs: bad superblock or unable to read bitmaps\n");
^1da177e4c3f41 Linus Torvalds     2005-04-16  449  out_freemap:
^1da177e4c3f41 Linus Torvalds     2005-04-16  450  	for (i = 0; i < sbi->s_imap_blocks; i++)
^1da177e4c3f41 Linus Torvalds     2005-04-16  451  		brelse(sbi->s_imap[i]);
^1da177e4c3f41 Linus Torvalds     2005-04-16  452  	for (i = 0; i < sbi->s_zmap_blocks; i++)
^1da177e4c3f41 Linus Torvalds     2005-04-16  453  		brelse(sbi->s_zmap[i]);
^1da177e4c3f41 Linus Torvalds     2005-04-16  454  	kfree(sbi->s_imap);
^1da177e4c3f41 Linus Torvalds     2005-04-16  455  	goto out_release;
^1da177e4c3f41 Linus Torvalds     2005-04-16  456  
^1da177e4c3f41 Linus Torvalds     2005-04-16  457  out_no_map:
a90a088021f8f1 David Howells      2008-02-07  458  	ret = -ENOMEM;
^1da177e4c3f41 Linus Torvalds     2005-04-16  459  	if (!silent)
^1da177e4c3f41 Linus Torvalds     2005-04-16  460  		printk("MINIX-fs: can't allocate map\n");
^1da177e4c3f41 Linus Torvalds     2005-04-16  461  	goto out_release;
^1da177e4c3f41 Linus Torvalds     2005-04-16  462  
f5fb09fa3392ad Andries Brouwer    2006-08-27  463  out_illegal_sb:
f5fb09fa3392ad Andries Brouwer    2006-08-27  464  	if (!silent)
f5fb09fa3392ad Andries Brouwer    2006-08-27  465  		printk("MINIX-fs: bad superblock\n");
f5fb09fa3392ad Andries Brouwer    2006-08-27  466  	goto out_release;
f5fb09fa3392ad Andries Brouwer    2006-08-27  467  
^1da177e4c3f41 Linus Torvalds     2005-04-16  468  out_no_fs:
^1da177e4c3f41 Linus Torvalds     2005-04-16  469  	if (!silent)
939b00df0306bc Andries Brouwer    2007-02-12  470  		printk("VFS: Can't find a Minix filesystem V1 | V2 | V3 "
939b00df0306bc Andries Brouwer    2007-02-12  471  		       "on device %s.\n", s->s_id);
^1da177e4c3f41 Linus Torvalds     2005-04-16  472  out_release:
^1da177e4c3f41 Linus Torvalds     2005-04-16  473  	brelse(bh);
^1da177e4c3f41 Linus Torvalds     2005-04-16  474  	goto out;
^1da177e4c3f41 Linus Torvalds     2005-04-16  475  
^1da177e4c3f41 Linus Torvalds     2005-04-16  476  out_bad_hblock:
11b8448751ba11 Denis Vlasenko     2006-03-25  477  	printk("MINIX-fs: blocksize too small for device\n");
^1da177e4c3f41 Linus Torvalds     2005-04-16  478  	goto out;
^1da177e4c3f41 Linus Torvalds     2005-04-16  479  
^1da177e4c3f41 Linus Torvalds     2005-04-16  480  out_bad_sb:
^1da177e4c3f41 Linus Torvalds     2005-04-16  481  	printk("MINIX-fs: unable to read superblock\n");
^1da177e4c3f41 Linus Torvalds     2005-04-16  482  out:
^1da177e4c3f41 Linus Torvalds     2005-04-16  483  	kfree(sbi);
e54e25638fc1ea Jori Koolstra      2025-10-28  484  	fc->s_fs_info = NULL;
a90a088021f8f1 David Howells      2008-02-07  485  	return ret;
^1da177e4c3f41 Linus Torvalds     2005-04-16  486  }
^1da177e4c3f41 Linus Torvalds     2005-04-16  487  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Tue 28-10-25 21:58:57, Jori Koolstra wrote:
> This patch sets up generic handling of errors such as filesystem
> corruption which are frequently raised by syzbot. Towards this aim it
> adds the following mount options to the minix filesystem: errors=
> continue/panic/remount-ro and (no)warn-on-error, with semantics
> similar to ext4fs. When a minix_error() or minix_error_inode() is
> raised, the error is reported and action is taken according to which of
> these mount options is set (errors=continue,nowarn-on-error are the
> default).
> 
> As an examle, this patch fixes a drop_nlink warning in rmdir exposed by
> syzbot, originating from a corrupted nlink field of a directory.
> 
> The changes were tested using the syzbot reproducer with the various new
> mount options. I also handcrafted a similar corrupted fs but with the
> minix v3 format (the reproducer uses v1).
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> Reported-by: syzbot+4e49728ec1cbaf3b91d2@syzkaller.appspotmail.com
> Closes: https://syzbot.org/bug?extid=4e49728ec1cbaf3b91d2

The patch looks ok to me but since minix filesystem driver is in the kernel
mostly to allow mounting ancient unix filesystems I don't quite understand
the motivation for adding the new mount options. Why not just fixup
minix_rmdir() to better handle corrupted filesystems?

								Honza

> ---
>  fs/minix/inode.c        | 237 ++++++++++++++++++++++++++++++++++++----
>  fs/minix/itree_common.c |   2 +-
>  fs/minix/minix.h        |  43 ++++++++
>  fs/minix/namei.c        |  25 +++--
>  4 files changed, 275 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index 32db676127a9..62159b7526a2 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -21,10 +21,16 @@
>  #include <linux/vfs.h>
>  #include <linux/writeback.h>
>  #include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
> +#include <linux/fsnotify.h>
>  
>  static int minix_write_inode(struct inode *inode,
> -		struct writeback_control *wbc);
> +			     struct writeback_control *wbc);
>  static int minix_statfs(struct dentry *dentry, struct kstatfs *buf);
> +static int minix_init_fs_context(struct fs_context *fc);
> +static void minix_handle_error(struct super_block *sb, int error,
> +			       const char *func, unsigned int line);
> +static bool system_going_down(void);
>  
>  static void minix_evict_inode(struct inode *inode)
>  {
> @@ -113,17 +119,121 @@ static const struct super_operations minix_sops = {
>  	.statfs		= minix_statfs,
>  };
>  
> +void __minix_error(struct super_block *sb, const char *function,
> +		   unsigned int line, int error, const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +	printk(KERN_CRIT "minix-fs error (device %s): %s:%d: comm %s: %pV\n",
> +	       sb->s_id, function, line, current->comm, &vaf);
> +	va_end(args);
> +
> +	fsnotify_sb_error(sb, NULL, error ? error : EFSCORRUPTED);
> +
> +	minix_handle_error(sb, error, function, line);
> +}
> +
> +void __minix_error_inode(struct inode *inode, const char *function,
> +			 unsigned int line, int error, u32 block,
> +			 const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +	if (block)
> +		printk(KERN_CRIT "minix-fs error (device %s): %s:%d: "
> +		       "inode #%lu: block %du: comm %s: %pV\n",
> +		       inode->i_sb->s_id, function, line, inode->i_ino,
> +		       block, current->comm, &vaf);
> +	else
> +		printk(KERN_CRIT "minix-fs error (device %s): %s:%d: "
> +		       "inode #%lu: comm %s: %pV\n",
> +		       inode->i_sb->s_id, function, line, inode->i_ino,
> +		       current->comm, &vaf);
> +	va_end(args);
> +
> +	fsnotify_sb_error(inode->i_sb, NULL, error ? error : EFSCORRUPTED);
> +
> +	minix_handle_error(inode->i_sb, error, function, line);
> +}
> +
> +static void minix_handle_error(struct super_block *sb, int error,
> +			       const char *func, unsigned int line)
> +{
> +	struct minix_sb_info *sbi = minix_sb(sb);
> +
> +	if (sbi->s_version != MINIX_V3) {
> +		sbi->s_mount_state |= MINIX_ERROR_FS;
> +		mark_buffer_dirty(sbi->s_sbh);
> +	}
> +
> +	if (test_opt(sb, WARN_ON_ERROR))
> +		WARN_ON_ONCE(1);
> +
> +	/* Do not panic during 'reboot -f' */
> +	if (test_opt(sb, ERRORS_PANIC) && !system_going_down()) {
> +		panic("minix-fs (device %s): panic forced after error\n",
> +		      sb->s_id);
> +	}
> +
> +	if (test_opt(sb, ERRORS_CONT) || sb_rdonly(sb))
> +		return;
> +
> +	minix_msg(sb, KERN_CRIT, "Remounting filesystem read-only");
> +
> +	sb->s_flags |= SB_RDONLY;
> +}
> +
> +void __minix_msg(struct super_block *sb,
> +		 const char *prefix, const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +	if (sb)
> +		printk("%sminix-fs (%s): %pV\n", prefix, sb->s_id, &vaf);
> +	else
> +		printk("%sminix-fs: %pV\n", prefix, &vaf);
> +	va_end(args);
> +}
> +
> +static bool system_going_down(void)
> +{
> +	return system_state == SYSTEM_HALT || system_state == SYSTEM_POWER_OFF
> +		|| system_state == SYSTEM_RESTART;
> +}
> +
> +struct minix_fs_context {
> +	unsigned int s_mount_opt;
> +	unsigned int s_def_mount_opt;
> +};
> +
>  static int minix_reconfigure(struct fs_context *fc)
>  {
> -	struct minix_super_block * ms;
> +	struct minix_fs_context *ctx = fc->fs_private;
>  	struct super_block *sb = fc->root->d_sb;
> -	struct minix_sb_info * sbi = sb->s_fs_info;
> +	unsigned int flags = fc->sb_flags;
> +	struct minix_sb_info *sbi = minix_sb(sb);
> +	struct minix_super_block *ms;
> +
> +	sbi->s_mount_opt = ctx->s_mount_opt;
>  
>  	sync_filesystem(sb);
>  	ms = sbi->s_ms;
> -	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
> +
> +	if ((bool)(flags & SB_RDONLY) == sb_rdonly(sb))
>  		return 0;
> -	if (fc->sb_flags & SB_RDONLY) {
> +	if (flags & SB_RDONLY) {
>  		if (ms->s_state & MINIX_VALID_FS ||
>  		    !(sbi->s_mount_state & MINIX_VALID_FS))
>  			return 0;
> @@ -172,6 +282,7 @@ static bool minix_check_superblock(struct super_block *sb)
>  
>  static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  {
> +	struct minix_fs_context *ctx = fc->fs_private;
>  	struct buffer_head *bh;
>  	struct buffer_head **map;
>  	struct minix_super_block *ms;
> @@ -198,6 +309,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  
>  	ms = (struct minix_super_block *) bh->b_data;
>  	sbi->s_ms = ms;
> +	sbi->s_mount_opt = ctx->s_mount_opt;
> +	sbi->s_def_mount_opt = ctx->s_def_mount_opt;
>  	sbi->s_sbh = bh;
>  	sbi->s_mount_state = ms->s_state;
>  	sbi->s_ninodes = ms->s_ninodes;
> @@ -226,7 +339,7 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  		s->s_max_links = MINIX2_LINK_MAX;
>  	} else if (s->s_magic == MINIX2_SUPER_MAGIC2) {
>  		sbi->s_version = MINIX_V2;
> -		sbi->s_nzones = ms->s_zones;
> +	sbi->s_nzones = ms->s_zones;
>  		sbi->s_dirsize = 32;
>  		sbi->s_namelen = 30;
>  		s->s_max_links = MINIX2_LINK_MAX;
> @@ -367,8 +480,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  out_bad_sb:
>  	printk("MINIX-fs: unable to read superblock\n");
>  out:
> -	s->s_fs_info = NULL;
>  	kfree(sbi);
> +	fc->s_fs_info = NULL;
>  	return ret;
>  }
>  
> @@ -377,18 +490,6 @@ static int minix_get_tree(struct fs_context *fc)
>  	 return get_tree_bdev(fc, minix_fill_super);
>  }
>  
> -static const struct fs_context_operations minix_context_ops = {
> -	.get_tree	= minix_get_tree,
> -	.reconfigure	= minix_reconfigure,
> -};
> -
> -static int minix_init_fs_context(struct fs_context *fc)
> -{
> -	fc->ops = &minix_context_ops;
> -
> -	return 0;
> -}
> -
>  static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
>  {
>  	struct super_block *sb = dentry->d_sb;
> @@ -518,11 +619,15 @@ static struct inode *V1_minix_iget(struct inode *inode)
>  		return ERR_PTR(-EIO);
>  	}
>  	if (raw_inode->i_nlinks == 0) {
> -		printk("MINIX-fs: deleted inode referenced: %lu\n",
> -		       inode->i_ino);
> +		minix_error_inode(inode, "deleted inode referenced");
>  		brelse(bh);
>  		iget_failed(inode);
>  		return ERR_PTR(-ESTALE);
> +	} else if (S_ISDIR(raw_inode->i_mode) && raw_inode->i_nlinks == 1) {
> +		minix_error_inode(inode, "directory inode has corrupted nlink");
> +		brelse(bh);
> +		iget_failed(inode);
> +		return ERR_PTR(-EFSCORRUPTED);
>  	}
>  	inode->i_mode = raw_inode->i_mode;
>  	i_uid_write(inode, raw_inode->i_uid);
> @@ -556,11 +661,15 @@ static struct inode *V2_minix_iget(struct inode *inode)
>  		return ERR_PTR(-EIO);
>  	}
>  	if (raw_inode->i_nlinks == 0) {
> -		printk("MINIX-fs: deleted inode referenced: %lu\n",
> -		       inode->i_ino);
> +		minix_error_inode(inode, "deleted inode referenced");
>  		brelse(bh);
>  		iget_failed(inode);
>  		return ERR_PTR(-ESTALE);
> +	} else if (S_ISDIR(raw_inode->i_mode) && raw_inode->i_nlinks == 1) {
> +		minix_error_inode(inode, "directory inode has corrupted nlink");
> +		brelse(bh);
> +		iget_failed(inode);
> +		return ERR_PTR(-EFSCORRUPTED);
>  	}
>  	inode->i_mode = raw_inode->i_mode;
>  	i_uid_write(inode, raw_inode->i_uid);
> @@ -705,13 +814,95 @@ void minix_truncate(struct inode * inode)
>  		V2_minix_truncate(inode);
>  }
>  
> +enum {
> +	Opt_errors, Opt_warn_on_error, Opt_nowarn_on_error
> +};
> +
> +static const struct constant_table minix_param_errors[] = {
> +	{"continue",	MINIX_MOUNT_ERRORS_CONT},
> +	{"panic",	MINIX_MOUNT_ERRORS_PANIC},
> +	{"remount-ro",	MINIX_MOUNT_ERRORS_RO},
> +	{}
> +};
> +
> +/*
> + * Mount option specification
> + */
> +static const struct fs_parameter_spec minix_param_specs[] = {
> +	fsparam_enum	("errors",		Opt_errors, minix_param_errors),
> +	fsparam_flag	("warn-on-error",	Opt_warn_on_error),
> +	fsparam_flag	("nowarn-on-error",	Opt_nowarn_on_error),
> +	{}
> +};
> +
> +static int minix_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +	struct minix_fs_context *ctx = fc->fs_private;
> +	struct fs_parse_result result;
> +
> +	int token;
> +
> +	token = fs_parse(fc, minix_param_specs, param, &result);
> +	if (token < 0)
> +		return token;
> +
> +	switch (token) {
> +	case Opt_errors:
> +		ctx->s_mount_opt &= ~MINIX_MOUNT_ERRORS_MASK;
> +		ctx->s_mount_opt |= result.uint_32;
> +		break;
> +	case Opt_warn_on_error:
> +		ctx->s_mount_opt |= MINIX_MOUNT_WARN_ON_ERROR;
> +		break;
> +	case Opt_nowarn_on_error:
> +		ctx->s_mount_opt &= ~MINIX_MOUNT_WARN_ON_ERROR;
> +		break;
> +	}
> +	return 0;
> +}
> +
> +static void minix_fc_free(struct fs_context *fc)
> +{
> +	struct minix_fs_context *ctx = fc->fs_private;
> +
> +	if (!ctx)
> +		return;
> +	kfree(ctx);
> +}
> +
> +static const struct fs_context_operations minix_context_ops = {
> +	.get_tree	= minix_get_tree,
> +	.reconfigure	= minix_reconfigure,
> +	.parse_param	= minix_parse_param,
> +	.free		= minix_fc_free,
> +};
> +
> +int minix_init_fs_context(struct fs_context *fc)
> +{
> +	struct minix_fs_context *ctx;
> +
> +	ctx = kzalloc(sizeof(struct minix_fs_context), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	fc->fs_private = ctx;
> +	fc->ops = &minix_context_ops;
> +
> +	ctx->s_def_mount_opt |= MINIX_MOUNT_ERRORS_DEF;
> +	ctx->s_mount_opt = ctx->s_def_mount_opt;
> +
> +	return 0;
> +}
> +
>  static struct file_system_type minix_fs_type = {
>  	.owner			= THIS_MODULE,
>  	.name			= "minix",
>  	.kill_sb		= kill_block_super,
>  	.fs_flags		= FS_REQUIRES_DEV,
>  	.init_fs_context	= minix_init_fs_context,
> +	.parameters		= minix_param_specs,
>  };
> +
>  MODULE_ALIAS_FS("minix");
>  
>  static int __init init_minix_fs(void)
> diff --git a/fs/minix/itree_common.c b/fs/minix/itree_common.c
> index dad131e30c05..6832c671125e 100644
> --- a/fs/minix/itree_common.c
> +++ b/fs/minix/itree_common.c
> @@ -188,7 +188,7 @@ static int get_block(struct inode * inode, sector_t block,
>  	/*
>  	 * Indirect block might be removed by truncate while we were
>  	 * reading it. Handling of that case (forget what we've got and
> -	 * reread) is taken out of the main path.
> +	 * reread) is taken out of the msb_breadain path.
>  	 */
>  	if (err == -EAGAIN)
>  		goto changed;
> diff --git a/fs/minix/minix.h b/fs/minix/minix.h
> index d54273c3c9ff..254220ffbf39 100644
> --- a/fs/minix/minix.h
> +++ b/fs/minix/minix.h
> @@ -11,6 +11,22 @@
>  #define MINIX_V2		0x0002		/* minix V2 fs */
>  #define MINIX_V3		0x0003		/* minix V3 fs */
>  
> +#define MINIX_MOUNT_ERRORS_CONT		0x00001	/* Continue on errors */
> +#define MINIX_MOUNT_ERRORS_RO		0x00002	/* Remount fs ro on errors */
> +#define MINIX_MOUNT_ERRORS_PANIC	0x00004	/* Panic on errors */
> +#define MINIX_MOUNT_WARN_ON_ERROR	0x00008 /* Trigger WARN_ON on error */
> +
> +#define MINIX_MOUNT_ERRORS_MASK		0x00007
> +
> +#define MINIX_MOUNT_ERRORS_DEF		MINIX_MOUNT_ERRORS_CONT
> +
> +#define clear_opt(sb, opt)		minix_sb(sb)->s_mount_opt &= \
> +						~MINIX_MOUNT_##opt
> +#define set_opt(sb, opt)		minix_sb(sb)->s_mount_opt |= \
> +						MINIX_MOUNT_##opt
> +#define test_opt(sb, opt)		(minix_sb(sb)->s_mount_opt & \
> +						MINIX_MOUNT_##opt)
> +
>  /*
>   * minix fs inode data in memory
>   */
> @@ -39,6 +55,8 @@ struct minix_sb_info {
>  	struct buffer_head * s_sbh;
>  	struct minix_super_block * s_ms;
>  	unsigned short s_mount_state;
> +	unsigned short s_mount_opt;
> +	unsigned short s_def_mount_opt;
>  	unsigned short s_version;
>  };
>  
> @@ -55,6 +73,13 @@ int minix_getattr(struct mnt_idmap *, const struct path *,
>  		struct kstat *, u32, unsigned int);
>  int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len);
>  
> +extern __printf(3, 4)
> +void __minix_msg(struct super_block *, const char *, const char *, ...);
> +void __minix_error(struct super_block *, const char *, unsigned int, int,
> +		   const char *, ...);
> +void __minix_error_inode(struct inode *, const char *, unsigned int, int, u32,
> +			 const char *, ...);
> +
>  extern void V1_minix_truncate(struct inode *);
>  extern void V2_minix_truncate(struct inode *);
>  extern void minix_truncate(struct inode *);
> @@ -168,4 +193,22 @@ static inline int minix_test_bit(int nr, const void *vaddr)
>  
>  #endif
>  
> +#define minix_error(sb, fmt, ...)						\
> +	__minix_error((sb), __func__, __LINE__, 0, (fmt), ##__VA_ARGS__)
> +#define minix_error_err(sb, err, fmt, ...)					\
> +	__minix_error((sb), __func__, __LINE__, (err), (fmt), ##__VA_ARGS__)
> +#define minix_error_inode(inode, fmt, ...)					\
> +	__minix_error_inode((inode), __func__, __LINE__, 0, 0,			\
> +			    (fmt), ##__VA_ARGS__)
> +#define minix_error_inode_err(inode, err, fmt, ...)				\
> +	__minix_error_inode((inode), __func__, __LINE__, (err), 0,		\
> +			    (fmt), ##__VA_ARGS__)
> +#define minix_error_inode_block(inode, block, err, fmt, ...)			\
> +	__minix_error_inode((inode), __func__, __LINE__, (err), (block),	\
> +			    (fmt), ##__VA_ARGS__)
> +#define minix_msg(sb, level, fmt, ...)				\
> +	__minix_msg(sb, level, fmt, ##__VA_ARGS__)
> +
> +#define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
> +
>  #endif /* FS_MINIX_H */
> diff --git a/fs/minix/namei.c b/fs/minix/namei.c
> index 8938536d8d3c..23f98f44e262 100644
> --- a/fs/minix/namei.c
> +++ b/fs/minix/namei.c
> @@ -161,15 +161,24 @@ static int minix_unlink(struct inode * dir, struct dentry *dentry)
>  static int minix_rmdir(struct inode * dir, struct dentry *dentry)
>  {
>  	struct inode * inode = d_inode(dentry);
> -	int err = -ENOTEMPTY;
> -
> -	if (minix_empty_dir(inode)) {
> -		err = minix_unlink(dir, dentry);
> -		if (!err) {
> -			inode_dec_link_count(dir);
> -			inode_dec_link_count(inode);
> -		}
> +	int err = -EFSCORRUPTED;
> +
> +	if (dir->i_nlink <= 2) {
> +		minix_error_inode(inode, "directory inode has corrupted nlink");
> +		goto out;
>  	}
> +
> +	err = -ENOTEMPTY;
> +	if (!minix_empty_dir(inode))
> +		goto out;
> +
> +	err = minix_unlink(dir, dentry);
> +	if (!err) {
> +		inode_dec_link_count(dir);
> +		inode_dec_link_count(inode);
> + 	}
> +
> +out:
>  	return err;
>  }
>  
> -- 
> 2.51.1.dirty
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
Hi Jan,

> 
> The patch looks ok to me but since minix filesystem driver is in the kernel
> mostly to allow mounting ancient unix filesystems I don't quite understand
> the motivation for adding the new mount options. Why not just fixup
> minix_rmdir() to better handle corrupted filesystems?
> 
> 								Honza

I am doing the Linux kernel mentorship program, and was looking to contribute
to fs. Since I saw a lot bugs on syzbot related to minix (and jfs too) not
handling corruptions well (yielding warnings in drop_nlink e.g.) I figured
it was a low stakes project, not completely trivial, yet within my scope, to
attempt to implement what ext4 does for these kind of problems (and jfs
implements the same opts too).

I would have asked about the exact status of minix, but was told not to
bother maintainers without a patch. I would be open with trying to improve
minix further, but of course if there are better options to get it out of
the kernel altogether that may be better. Sad for me, since that means still
zero patches, but that is not your problem :)

Anyway, I hope this clarifies why I submitted this patch.
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
Hi Jori!

On Thu 30-10-25 13:22:53, Jori Koolstra wrote:
> > The patch looks ok to me but since minix filesystem driver is in the kernel
> > mostly to allow mounting ancient unix filesystems I don't quite understand
> > the motivation for adding the new mount options. Why not just fixup
> > minix_rmdir() to better handle corrupted filesystems?
> > 
> > 								Honza
> 
> I am doing the Linux kernel mentorship program, and was looking to contribute
> to fs. Since I saw a lot bugs on syzbot related to minix (and jfs too) not
> handling corruptions well (yielding warnings in drop_nlink e.g.) I figured
> it was a low stakes project, not completely trivial, yet within my scope, to
> attempt to implement what ext4 does for these kind of problems (and jfs
> implements the same opts too).

Well, one thing is handling corruption well - that part of your patch was
fine and I think it is still useful - another thing are the mount options
that allow to configure what happens when we find a corruption - and that
is the part I don't think really makes a lot of sense for minix.

> I would have asked about the exact status of minix, but was told not to
> bother maintainers without a patch. I would be open with trying to improve
> minix further, but of course if there are better options to get it out of
> the kernel altogether that may be better. Sad for me, since that means still
> zero patches, but that is not your problem :)

Your fix for minix_rmdir() is needed. If minix is going out of the kernel
is uncertain and even if that happens it will certainly need some
deprecation period so functional fixes are still wanted in that period.

> Anyway, I hope this clarifies why I submitted this patch.

Fully, thanks for the clarification and your work on Linux :)

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
>  
> Hi Jori!
>

Hi Jan, thank you for your encouraging reply, I appreciate it.

> 
> Well, one thing is handling corruption well - that part of your patch was
> fine and I think it is still useful - another thing are the mount options
> that allow to configure what happens when we find a corruption - and that
> is the part I don't think really makes a lot of sense for minix.
>

I already had a patch for this specific syzbot bug, and tested with the
reproducer, but without the new mount options. What I could do is submit this
and see if the community will accept it. Is that reasonable?
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Fri 31-10-25 00:16:48, Jori Koolstra wrote:
> 
> >  
> > Hi Jori!
> >
> 
> Hi Jan, thank you for your encouraging reply, I appreciate it.
> 
> > 
> > Well, one thing is handling corruption well - that part of your patch was
> > fine and I think it is still useful - another thing are the mount options
> > that allow to configure what happens when we find a corruption - and that
> > is the part I don't think really makes a lot of sense for minix.
> >
> 
> I already had a patch for this specific syzbot bug, and tested with the
> reproducer, but without the new mount options. What I could do is submit
> this and see if the community will accept it. Is that reasonable?

Definitely.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jeff Layton 3 months, 1 week ago
On Thu, 2025-10-30 at 13:22 +0100, Jori Koolstra wrote:
> Hi Jan,
> 
> > 
> > The patch looks ok to me but since minix filesystem driver is in the kernel
> > mostly to allow mounting ancient unix filesystems I don't quite understand
> > the motivation for adding the new mount options. Why not just fixup
> > minix_rmdir() to better handle corrupted filesystems?
> > 
> > 								Honza
> 
> I am doing the Linux kernel mentorship program, and was looking to contribute
> to fs. Since I saw a lot bugs on syzbot related to minix (and jfs too) not
> handling corruptions well (yielding warnings in drop_nlink e.g.) I figured
> it was a low stakes project, not completely trivial, yet within my scope, to
> attempt to implement what ext4 does for these kind of problems (and jfs
> implements the same opts too).
> 
> I would have asked about the exact status of minix, but was told not to
> bother maintainers without a patch. I would be open with trying to improve
> minix further, but of course if there are better options to get it out of
> the kernel altogether that may be better. Sad for me, since that means still
> zero patches, but that is not your problem :)
> 

Not necessarily.

I'm not sure if your internship covers this, but you could start a
project to build a minixfs FUSE fs (if one doesn't already exist). If
you get it working and stable, you can then submit patches to deprecate
and remove it from the kernel.

> Anyway, I hope this clarifies why I submitted this patch.

For some background: this is a continuation of a discussion that we had
at LSF/MM summit this year. A lot of these smaller, less-used
filesystems represent a significant maintenance burden. Whenever we
have to make changes at the VFS layer, they represent another fs that
we have to touch.

Many of these are not performance-critical and are hard to test. They
would be _much_ easier to maintain in userland if we can make that
work.
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
> Not necessarily.
>
> I'm not sure if your internship covers this, but you could start a
> project to build a minixfs FUSE fs (if one doesn't already exist). If
> you get it working and stable, you can then submit patches to deprecate
> and remove it from the kernel.


I would have to ask Shuah but am open to it. But a quick search turns up
this: https://github.com/redcap97/fuse-mfs . I would have to see if it
actually works and it does not seem to support v1, v2 of minix fs either.
There might also be a licensing issue.

> For some background: this is a continuation of a discussion that we had
> at LSF/MM summit this year. A lot of these smaller, less-used
> filesystems represent a significant maintenance burden. Whenever we
> have to make changes at the VFS layer, they represent another fs that
> we have to touch.

> Many of these are not performance-critical and are hard to test. They
> would be _much_ easier to maintain in userland if we can make that
> work.

One question I would have about this is that if we move minix, for
instance, out of the kernel code, how can we be sure that it is
maintained. What if some Github repo suddenly disappears? Like I said,
I would be fine with helping maintain minix, otherwise what should be
the course of action from here? What demands do we place on a userland
replacement for minix before I submit a patch to deprecate and remove
the code?

Thanks,
Jori.

PS. Sorry for sending this twice. I am still having to remind myself to
"reply all". My apologies.
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Shuah Khan 3 months, 1 week ago
On 10/30/25 07:31, Jori Koolstra wrote:
>> Not necessarily.
>>
>> I'm not sure if your internship covers this, but you could start a
>> project to build a minixfs FUSE fs (if one doesn't already exist). If
>> you get it working and stable, you can then submit patches to deprecate
>> and remove it from the kernel.
> 
> 
> I would have to ask Shuah but am open to it. But a quick search turns up
> this: https://github.com/redcap97/fuse-mfs . I would have to see if it
> actually works and it does not seem to support v1, v2 of minix fs either.
> There might also be a licensing issue.

No objections from me. I was expecting we might end up here based on
initial reactions to syzbot fix. Since you are interested in fs, this
work you did for this patch is a good learning opportunity which will
stay with you.

thanks,
-- Shuah
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jeff Layton 3 months, 1 week ago
On Thu, 2025-10-30 at 14:31 +0100, Jori Koolstra wrote:
> > Not necessarily.
> > 
> > I'm not sure if your internship covers this, but you could start a
> > project to build a minixfs FUSE fs (if one doesn't already exist). If
> > you get it working and stable, you can then submit patches to deprecate
> > and remove it from the kernel.
> 
> 
> I would have to ask Shuah but am open to it. But a quick search turns up
> this: https://github.com/redcap97/fuse-mfs . I would have to see if it
> actually works and it does not seem to support v1, v2 of minix fs either.
> There might also be a licensing issue.
> 

I don't see a licensing issue. It's BSD licensed. Also, this is a
userland code, so we wouldn't need to worry about that too much.
 
> > For some background: this is a continuation of a discussion that we had
> > at LSF/MM summit this year. A lot of these smaller, less-used
> > filesystems represent a significant maintenance burden. Whenever we
> > have to make changes at the VFS layer, they represent another fs that
> > we have to touch.
> 
> > Many of these are not performance-critical and are hard to test. They
> > would be _much_ easier to maintain in userland if we can make that
> > work.
> 
> One question I would have about this is that if we move minix, for
> instance, out of the kernel code, how can we be sure that it is
> maintained. What if some Github repo suddenly disappears? Like I said,
> I would be fine with helping maintain minix, otherwise what should be
> the course of action from here? What demands do we place on a userland
> replacement for minix before I submit a patch to deprecate and remove
> the code?
> 

These are great questions that I don't think we have an answer for just
yet.

In practice, FUSE interfaces are quite stable, and the minixfs format
also doesn't change a lot. Much like minixfs in the kernel, I wouldn't
expect that it would require a lot of maintenance itself over the long
haul (but everything requires _some_). It might need some to keep up
with broader OS changes, but that's not usually too burdensome.

You're quite right though that userland replacements will need to meet
some criteria before we can rip out the in-kernel versions. This might
be a good discussion topic for next year's LSF/MM!
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Thu 30-10-25 09:43:26, Jeff Layton wrote:
> On Thu, 2025-10-30 at 14:31 +0100, Jori Koolstra wrote:
> > One question I would have about this is that if we move minix, for
> > instance, out of the kernel code, how can we be sure that it is
> > maintained. What if some Github repo suddenly disappears? Like I said,
> > I would be fine with helping maintain minix, otherwise what should be
> > the course of action from here? What demands do we place on a userland
> > replacement for minix before I submit a patch to deprecate and remove
> > the code?
> > 
> 
> These are great questions that I don't think we have an answer for just
> yet.
> 
> In practice, FUSE interfaces are quite stable, and the minixfs format
> also doesn't change a lot. Much like minixfs in the kernel, I wouldn't
> expect that it would require a lot of maintenance itself over the long
> haul (but everything requires _some_). It might need some to keep up
> with broader OS changes, but that's not usually too burdensome.
> 
> You're quite right though that userland replacements will need to meet
> some criteria before we can rip out the in-kernel versions. This might
> be a good discussion topic for next year's LSF/MM!

Usually the requirement is feature parity - meaning if the kernel can read
/ write filesystem with some set of features, then the other drivers should
support that as well. Also passing the same set of fstests is a good
indication the replacement is sound. Darrick did quite some work with
making fstests work better for FUSE filesystems recently if I remember
correctly.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jeff Layton 3 months, 1 week ago
On Thu, 2025-10-30 at 18:22 +0100, Jan Kara wrote:
> On Thu 30-10-25 09:43:26, Jeff Layton wrote:
> > On Thu, 2025-10-30 at 14:31 +0100, Jori Koolstra wrote:
> > > One question I would have about this is that if we move minix, for
> > > instance, out of the kernel code, how can we be sure that it is
> > > maintained. What if some Github repo suddenly disappears? Like I said,
> > > I would be fine with helping maintain minix, otherwise what should be
> > > the course of action from here? What demands do we place on a userland
> > > replacement for minix before I submit a patch to deprecate and remove
> > > the code?
> > > 
> > 
> > These are great questions that I don't think we have an answer for just
> > yet.
> > 
> > In practice, FUSE interfaces are quite stable, and the minixfs format
> > also doesn't change a lot. Much like minixfs in the kernel, I wouldn't
> > expect that it would require a lot of maintenance itself over the long
> > haul (but everything requires _some_). It might need some to keep up
> > with broader OS changes, but that's not usually too burdensome.
> > 
> > You're quite right though that userland replacements will need to meet
> > some criteria before we can rip out the in-kernel versions. This might
> > be a good discussion topic for next year's LSF/MM!
> 
> Usually the requirement is feature parity - meaning if the kernel can read
> / write filesystem with some set of features, then the other drivers should
> support that as well. Also passing the same set of fstests is a good
> indication the replacement is sound. Darrick did quite some work with
> making fstests work better for FUSE filesystems recently if I remember
> correctly.
> 

Right. We probably need to codify that into a checklist in
Documentation/ somewhere.

- feature parity (at least mostly, might can omit obscure things on
some crufty old filesystems)

- passes fstests (at least as well as the in-kernel driver)

- entry in a list of deprecated filesystems and where the tree is
hosted if they aren't hosted in the kernel tree

There are probably all sorts of other things I'm not thinking of too.
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
> 
> I don't see a licensing issue. It's BSD licensed. Also, this is a
> userland code, so we wouldn't need to worry about that too much.
>

Oh, my bad. I thought Minix (the OS) had some licensing incompatibilities
with Linux, and this repo takes code from Minix. But that may be long in
the past.

> 
> You're quite right though that userland replacements will need to meet
> some criteria before we can rip out the in-kernel versions. This might
> be a good discussion topic for next year's LSF/MM!

Would an in-tree but out of kernel implementation be an idea? Like how
kselftest is integrated in the code, even though most of that also takes
place in userland. That would guarantee a level of support, at least for
the time being. I could take the code, verify it, and write some tests
for in selftest.

And there is still the issue of what we do for the syzbot bugs until a
more permanent solution is achieved.

Anyway, this probably goes over my head as a clueless beginner. Just
trying to see where I can help. Thanks a lot Jeff for you answers, I
appreciate it.

Jori.
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jeff Layton 3 months, 1 week ago
On Thu, 2025-10-30 at 15:09 +0100, Jori Koolstra wrote:
> > 
> > I don't see a licensing issue. It's BSD licensed. Also, this is a
> > userland code, so we wouldn't need to worry about that too much.
> > 
> 
> Oh, my bad. I thought Minix (the OS) had some licensing incompatibilities
> with Linux, and this repo takes code from Minix. But that may be long in
> the past.
> 

Minix is BSD licensed too. That's not completely incompatible with the
GPL, but IANAL.

> > 
> > You're quite right though that userland replacements will need to meet
> > some criteria before we can rip out the in-kernel versions. This might
> > be a good discussion topic for next year's LSF/MM!
> 
> Would an in-tree but out of kernel implementation be an idea? Like how
> kselftest is integrated in the code, even though most of that also takes
> place in userland. That would guarantee a level of support, at least for
> the time being. I could take the code, verify it, and write some tests
> for in selftest.
> 

That's not a bad idea. We already have some userland code in the kernel
tree (the tools/ directory comes to mind). A directory with replacement
FUSE drivers for in-kernel filesystems could be a reasonable thing to
add. Anything we keep in-tree will need to be GPL-compatible though.

> And there is still the issue of what we do for the syzbot bugs until a
> more permanent solution is achieved.
> 

Yeah, that's a different issue.  Most likely we'll need to fix those in
the near term. Replacing minix.ko with a FUSE fs will take time
(years), even once we have a new driver in hand.

We'll need to mark the old driver deprecated and then wait a few
releases before we can rip it out.

> Anyway, this probably goes over my head as a clueless beginner. Just
> trying to see where I can help. Thanks a lot Jeff for you answers, I
> appreciate it.
> 

You're welcome. We all start out as beginners!
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Thu 30-10-25 11:12:44, Jeff Layton wrote:
> On Thu, 2025-10-30 at 15:09 +0100, Jori Koolstra wrote:
> > > You're quite right though that userland replacements will need to meet
> > > some criteria before we can rip out the in-kernel versions. This might
> > > be a good discussion topic for next year's LSF/MM!
> > 
> > Would an in-tree but out of kernel implementation be an idea? Like how
> > kselftest is integrated in the code, even though most of that also takes
> > place in userland. That would guarantee a level of support, at least for
> > the time being. I could take the code, verify it, and write some tests
> > for in selftest.
> 
> That's not a bad idea. We already have some userland code in the kernel
> tree (the tools/ directory comes to mind). A directory with replacement
> FUSE drivers for in-kernel filesystems could be a reasonable thing to
> add. Anything we keep in-tree will need to be GPL-compatible though.

Yes, I kind of like that idea too. I think we could maybe take the existing
in-kernel minix driver and morph it into a FUSE driver which would deal
with the licensing as well.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jori Koolstra 3 months, 1 week ago
> 
> Yes, I kind of like that idea too. I think we could maybe take the existing
> in-kernel minix driver and morph it into a FUSE driver which would deal
> with the licensing as well.
> 

By now I am quite comfortable with the in-kernel implementation as it exists.
The parts that I understand less are about how exactly the data from the
device ends up in the right place in memory and how address_space and folios
are built up for it. But these are precisely the parts that are not too
relevant for a userspace driver. So, I would be very happy to work on a GPL
FUSE driver for minix and get it to functional parity for all versions v1-v3.
After that, and when all the available tests run well, we can start the
deprecation process.

I think this would be a great opportunity for me to learn and contribute
something non-trivial, but that is still within my current level of ability.
Shuah has also encouraged me to proceed on this track. However, since this
is more than a couple of days work for me (for one I have to study the FUSE
API), it would be helpful if I can get a sense of whether there is a
reasonable chance for such work to be eventually included upstream. Of
course, only IF it functions according to the criteria set out. I am not
looking for guarantees, because I understand that is not how Linux
development works, but just a sense of how the community feels about this.
So far I have seen quite favorable responses, but I am too unfamiliar with
the development process to understand how this may play out.

Thanks again Jan and Jeff for your patient replies.

Jori
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Shuah Khan 3 months, 1 week ago
On 10/30/25 09:12, Jeff Layton wrote:
> On Thu, 2025-10-30 at 15:09 +0100, Jori Koolstra wrote:
>>>
>>> I don't see a licensing issue. It's BSD licensed. Also, this is a
>>> userland code, so we wouldn't need to worry about that too much.
>>>
>>
>> Oh, my bad. I thought Minix (the OS) had some licensing incompatibilities
>> with Linux, and this repo takes code from Minix. But that may be long in
>> the past.
>>
> 
> Minix is BSD licensed too. That's not completely incompatible with the
> GPL, but IANAL.
> 
>>>
>>> You're quite right though that userland replacements will need to meet
>>> some criteria before we can rip out the in-kernel versions. This might
>>> be a good discussion topic for next year's LSF/MM!
>>
>> Would an in-tree but out of kernel implementation be an idea? Like how
>> kselftest is integrated in the code, even though most of that also takes
>> place in userland. That would guarantee a level of support, at least for
>> the time being. I could take the code, verify it, and write some tests
>> for in selftest.
>>
> 
> That's not a bad idea. We already have some userland code in the kernel
> tree (the tools/ directory comes to mind). A directory with replacement
> FUSE drivers for in-kernel filesystems could be a reasonable thing to
> add. Anything we keep in-tree will need to be GPL-compatible though.

Jori, if you want to continue working on userland slotions and working
to initiate deprecating, working - please do.> 
>> And there is still the issue of what we do for the syzbot bugs until a
>> more permanent solution is achieved.
>>
> 
> Yeah, that's a different issue.  Most likely we'll need to fix those in
> the near term. Replacing minix.ko with a FUSE fs will take time
> (years), even once we have a new driver in hand.
Does this mean Jori can work on fixing these while replacing minix.ko
with fuse progresses?

> 
> We'll need to mark the old driver deprecated and then wait a few
> releases before we can rip it out.
Jori could work on patches for deprecating perhaps?

> 
>> Anyway, this probably goes over my head as a clueless beginner. Just
>> trying to see where I can help. Thanks a lot Jeff for you answers, I
>> appreciate it.
>>
> 
> You're welcome. We all start out as beginners!

+1

thanks,
-- Shuah
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jeff Layton 3 months, 1 week ago
On Thu, 2025-10-30 at 09:44 -0600, Shuah Khan wrote:
> On 10/30/25 09:12, Jeff Layton wrote:
> > On Thu, 2025-10-30 at 15:09 +0100, Jori Koolstra wrote:
> > > > 
> > > > I don't see a licensing issue. It's BSD licensed. Also, this is a
> > > > userland code, so we wouldn't need to worry about that too much.
> > > > 
> > > 
> > > Oh, my bad. I thought Minix (the OS) had some licensing incompatibilities
> > > with Linux, and this repo takes code from Minix. But that may be long in
> > > the past.
> > > 
> > 
> > Minix is BSD licensed too. That's not completely incompatible with the
> > GPL, but IANAL.
> > 
> > > > 
> > > > You're quite right though that userland replacements will need to meet
> > > > some criteria before we can rip out the in-kernel versions. This might
> > > > be a good discussion topic for next year's LSF/MM!
> > > 
> > > Would an in-tree but out of kernel implementation be an idea? Like how
> > > kselftest is integrated in the code, even though most of that also takes
> > > place in userland. That would guarantee a level of support, at least for
> > > the time being. I could take the code, verify it, and write some tests
> > > for in selftest.
> > > 
> > 
> > That's not a bad idea. We already have some userland code in the kernel
> > tree (the tools/ directory comes to mind). A directory with replacement
> > FUSE drivers for in-kernel filesystems could be a reasonable thing to
> > add. Anything we keep in-tree will need to be GPL-compatible though.
> 
> Jori, if you want to continue working on userland slotions and working
> to initiate deprecating, working - please do.> 
> > > And there is still the issue of what we do for the syzbot bugs until a
> > > more permanent solution is achieved.
> > > 
> > 
> > Yeah, that's a different issue.  Most likely we'll need to fix those in
> > the near term. Replacing minix.ko with a FUSE fs will take time
> > (years), even once we have a new driver in hand.
> Does this mean Jori can work on fixing these while replacing minix.ko
> with fuse progresses?
> 

Caveat: I have no real connection to minixfs. All of my contributions
are drive-bys where I was changing some other vfs layer interface.

I see no problem with fixing real bugs in the minixfs driver.
Personally, I'd focus on things that are actual security problems --
crashes that are triggerable by non-privileged users.

In this case, the problem involves a deliberately corrupted filesystem.
As a community, we have deprioritized fixing these sorts of bugs.
Someone with the access to mount a deliberately corrupt fs like this is
in a position to crash the kernel in any number of other ways. With a
legacy filesystem like this, fixing problems of this nature is usually
not worth developer/reviewer time.

> > 
> > We'll need to mark the old driver deprecated and then wait a few
> > releases before we can rip it out.
> Jori could work on patches for deprecating perhaps?
> 

That's probably premature until we have feature-complete replacement to
point users toward.

> > 
> > > Anyway, this probably goes over my head as a clueless beginner. Just
> > > trying to see where I can help. Thanks a lot Jeff for you answers, I
> > > appreciate it.
> > > 
> > 
> > You're welcome. We all start out as beginners!
> 
> +1
> 
> thanks,
> -- Shuah

-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Thu 30-10-25 09:44:40, Shuah Khan wrote:
> On 10/30/25 09:12, Jeff Layton wrote:
> > On Thu, 2025-10-30 at 15:09 +0100, Jori Koolstra wrote:
> > > And there is still the issue of what we do for the syzbot bugs until a
> > > more permanent solution is achieved.
> > > 
> > 
> > Yeah, that's a different issue.  Most likely we'll need to fix those in
> > the near term. Replacing minix.ko with a FUSE fs will take time
> > (years), even once we have a new driver in hand.
> Does this mean Jori can work on fixing these while replacing minix.ko
> with fuse progresses?

Yes, we generally accept fixes to syzbot bugs even for old filesystems.
It's just that for these old filesystems there's limited enthusiasm to
accept large-scale changes just to fix some odd syzbot issue. But luckily
most of the syzbot bugs are usually fixed just by proper input validation
which isn't usually very intrusive.

> > We'll need to mark the old driver deprecated and then wait a few
> > releases before we can rip it out.
> Jori could work on patches for deprecating perhaps?

He could but deprecation is pretty easy - just print a notice about
deprecation to the kernel log on mount, update MAINTAINERS file etc. But
more interesting is making sure the userspace driver has feature parity
with the kernel driver before we start the kernel driver deprecation.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Tetsuo Handa 3 months, 1 week ago
On 2025/10/30 19:59, Jan Kara wrote:
> The patch looks ok to me but since minix filesystem driver is in the kernel
> mostly to allow mounting ancient unix filesystems I don't quite understand
> the motivation for adding the new mount options. Why not just fixup
> minix_rmdir() to better handle corrupted filesystems?
I guess that a filesystem which is not needed from boot time can be
implemented in userspace. Can't we migrate legacy filesystems to
fuse-based userspace implementations?
Re: [PATCH] Add error handling to minix filesystem similar to ext4
Posted by Jan Kara 3 months, 1 week ago
On Thu 30-10-25 20:18:07, Tetsuo Handa wrote:
> On 2025/10/30 19:59, Jan Kara wrote:
> > The patch looks ok to me but since minix filesystem driver is in the kernel
> > mostly to allow mounting ancient unix filesystems I don't quite understand
> > the motivation for adding the new mount options. Why not just fixup
> > minix_rmdir() to better handle corrupted filesystems?
> I guess that a filesystem which is not needed from boot time can be
> implemented in userspace. Can't we migrate legacy filesystems to
> fuse-based userspace implementations?

I didn't research availability / usability of a FUSE driver for minix but
if there's working userspace alternative, deprecating the kernel driver is
certainly a welcome option :).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR