[PATCH v2 4/4] minix: fix symlink and truncate for iomap

[Jeremy Bingham]

The original minix iomap conversion patches had a couple of issues
exposed by syzbot:

1. NULL pointer dereferences in page_symlink. Since the iomap based
minix_aops no longer used write_begin and write_end, when page_link
attempted to call aops->write_begin there was a crash because there was
no longer a pointer assigned to aops->write_begin. This was fixed with a
custom function that writes the symlink target directly to a newly
allocated data block acquired with minix_new_block and sb_getblk, thus
bypassing the aops write path entirely. The symlink read path was in
turn replaced with a new minix_get_link that reads the target from the
first data block directly with sb_bread, similar to how ext4_get_link
does it.

2. Truncate crashed. There was a conflict in minix's truncate() function
between buffer_head and iomap where block_truncate_page was attaching
buffer_heads to folios, but this ran up against how iomap works. Fixed
this by using iomap_truncate_page for inodes using minix_aops and
block_truncate_page for inodes with minix_dir_aops.

Also, minix_symlink_inode_operations now has .setattr = minix_setattr so
symlinks can be truncated more properly through iomap rather than using
the default simple_setattr.

Signed-off-by: Jeremy Bingham <jbingham@xxxxxxxxx>
---
 fs/minix/inode.c        |   4 +-
 fs/minix/itree_common.c |  11 +++-
 fs/minix/minix.h        |   5 +-
 fs/minix/namei.c        | 137 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 152 insertions(+), 5 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 2ba6766fce51..b113c44764ff 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -573,8 +573,9 @@ static const struct address_space_operations minix_dir_aops = {
 };
 
 static const struct inode_operations minix_symlink_inode_operations = {
-	.get_link	= page_get_link,
+	.get_link	= minix_get_link,
 	.getattr	= minix_getattr,
+	.setattr	= minix_setattr,
 };
 
 void minix_set_inode(struct inode *inode, dev_t rdev)
@@ -838,4 +839,3 @@ module_init(init_minix_fs)
 module_exit(exit_minix_fs)
 MODULE_DESCRIPTION("Minix file system");
 MODULE_LICENSE("GPL");
-
diff --git a/fs/minix/itree_common.c b/fs/minix/itree_common.c
index c3cd2c75af9c..5a8b73a7beda 100644
--- a/fs/minix/itree_common.c
+++ b/fs/minix/itree_common.c
@@ -311,7 +311,16 @@ static inline void truncate (struct inode * inode)
 	long iblock;
 
 	iblock = (inode->i_size + sb->s_blocksize -1) >> sb->s_blocksize_bits;
-	block_truncate_page(inode->i_mapping, inode->i_size, get_block);
+
+	/* Depending on what address space operations are being used by the
+	 * inode being truncated, we need to either call iomap_truncate_page or
+	 * block_truncate_page.
+	 */
+	if (inode->i_mapping->a_ops == &minix_aops)
+		iomap_truncate_page(inode, inode->i_size, NULL,
+			minix_iomap_ops_ver(inode), NULL, NULL);
+	else
+		block_truncate_page(inode->i_mapping, inode->i_size, get_block);
 
 	n = block_to_path(inode, iblock, offsets);
 	if (!n)
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 76718f789369..d1a890e96abe 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -57,7 +57,7 @@ int minix_new_block(struct inode *inode);
 void minix_free_block(struct inode *inode, unsigned long block);
 unsigned long minix_count_free_blocks(struct super_block *sb);
 int minix_getattr(struct mnt_idmap *, const struct path *,
-		struct kstat *, u32, unsigned int);
+		struct kstat *, u32, unsigned);
 int minix_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	struct iattr *attr);
 int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len);
@@ -82,6 +82,9 @@ int minix_make_empty(struct inode*, struct inode*);
 int minix_empty_dir(struct inode*);
 int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
 		struct inode *inode);
+extern const char *minix_get_link(struct dentry *dentry, struct inode *inode,
+		struct delayed_call *callback);
+
 struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
 ino_t minix_inode_by_name(struct dentry*);
 
diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index 263e4ba8b1c8..e245f55a68ff 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -6,6 +6,8 @@
  */
 
 #include "minix.h"
+#include <linux/buffer_head.h>
+#include <linux/namei.h>
 
 static int add_nondir(struct dentry *dentry, struct inode *inode)
 {
@@ -69,6 +71,101 @@ static int minix_create(struct mnt_idmap *idmap, struct inode *dir,
 	return minix_mknod(&nop_mnt_idmap, dir, dentry, mode, 0);
 }
 
+static inline u16 *v1_i_data(struct inode *inode)
+{
+	return (u16 *)minix_i(inode)->u.i1_data;
+}
+
+static inline u32 *v2_i_data(struct inode *inode)
+{
+	return (u32 *)minix_i(inode)->u.i2_data;
+}
+
+static inline u16 cpu_to_v1_block(sector_t n)
+{
+	return n;
+}
+
+static inline u32 cpu_to_v2_block(sector_t n)
+{
+	return n;
+}
+
+static inline sector_t v1_block_to_cpu(u16 n)
+{
+	return n;
+}
+
+static inline sector_t v2_block_to_cpu(u32 n)
+{
+	return n;
+}
+
+/* Reimplement page_symlink's general logic while avoiding using buffer head
+ * based aops operations like aops->write_begin so things behave better with
+ * the new regime of iomap based aops operations. Cribbing from page_symlink in
+ * fs/namei.c and ext4's ext4_init_symlink_block.
+ */
+static int __page_symlink(struct inode *inode, const char *symname, int len)
+{
+	struct super_block *sb = inode->i_sb;
+	struct buffer_head *bh;
+	char *kaddr;
+	int err = 0;
+	u16 *p16; /* v1 16 bit block */
+	u32 *p32; /* v2/3 32 bit block */
+
+	sector_t phys;
+
+	phys = minix_new_block(inode);
+	if (!phys) {
+		err = -ENOSPC;
+		goto ps_out;
+	}
+
+	if (INODE_VERSION(inode) == MINIX_V1) {
+		p16 = v1_i_data(inode);
+		*p16 = cpu_to_v1_block(phys);
+	} else {
+		p32 = v2_i_data(inode);
+		*p32 = cpu_to_v2_block(phys);
+	}
+
+	bh = sb_getblk(sb, phys);
+	if (!bh) {
+		err = -ENOMEM;
+		goto ps_fail;
+	}
+
+	lock_buffer(bh);
+	kaddr = (char *)bh->b_data;
+	memset(kaddr, 0, sb->s_blocksize);
+	memcpy(kaddr, symname, len);
+	inode->i_size = len - 1;
+	set_buffer_uptodate(bh);
+	unlock_buffer(bh);
+
+	mmb_mark_buffer_dirty(bh, &minix_i(inode)->i_metadata_bhs);
+	if (inode_needs_sync(inode)) {
+		sync_dirty_buffer(bh);
+		if (buffer_req(bh) && !buffer_uptodate(bh)) {
+			pr_err("i/o error syncing itable block");
+			err = -EIO;
+		}
+
+	}
+
+	mark_inode_dirty(inode);
+	brelse(bh);
+
+ps_out:
+	return err;
+
+ps_fail:
+	minix_free_block(inode, phys);
+	goto ps_out;
+}
+
 static int minix_symlink(struct mnt_idmap *idmap, struct inode *dir,
 			 struct dentry *dentry, const char *symname)
 {
@@ -84,7 +181,7 @@ static int minix_symlink(struct mnt_idmap *idmap, struct inode *dir,
 		return PTR_ERR(inode);
 
 	minix_set_inode(inode, 0);
-	err = page_symlink(inode, symname, i);
+	err = __page_symlink(inode, symname, i);
 	if (unlikely(err)) {
 		inode_dec_link_count(inode);
 		iput(inode);
@@ -273,6 +370,44 @@ static int minix_rename(struct mnt_idmap *idmap,
 	return err;
 }
 
+/* straight up thievery here; stolen verbatim from ext4_get_link */
+static void minix_free_link(void *bh)
+{
+	brelse(bh);
+}
+
+/* Borrowing from ext4_get_link to a degree; since minix inodes and symlinks
+ * are significantly simpler, we don't need to do nearly as much as ext4
+ * requires for old-timey ext4 slow links.
+ */
+const char *minix_get_link(struct dentry *dentry, struct inode *inode,
+		struct delayed_call *callback)
+{
+	struct super_block *sb = inode->i_sb;
+	struct buffer_head *bh;
+	sector_t blk;
+
+	/* Get yon block, depending on what version of the minix fs this is. */
+	if (INODE_VERSION(inode) == MINIX_V1)
+		blk = v1_block_to_cpu(*(v1_i_data(inode)));
+	else
+		blk = v2_block_to_cpu(*(v2_i_data(inode)));
+
+	bh = sb_bread(sb, blk);
+	if (IS_ERR(bh))
+		return ERR_CAST(bh);
+	if (!bh) {
+		pr_err("bad symlink on inode %llu", inode->i_ino);
+		return ERR_PTR(-EFSCORRUPTED);
+	}
+
+	set_delayed_call(callback, minix_free_link, bh);
+	nd_terminate_link(bh->b_data, inode->i_size,
+			inode->i_sb->s_blocksize - 1);
+
+	return bh->b_data;
+}
+
 /*
  * directories can handle most operations...
  */
-- 
2.47.3