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(-)
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
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
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
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.
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
> > 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?
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
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>
> 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.
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
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>
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
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>
> > 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.
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>
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
> > 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
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
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>
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
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?
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
© 2016 - 2026 Red Hat, Inc.