0.3.12

This release fixes a serious data corruption bug for containers bigger than ~1.32 TiB, which is immediately triggered by a writable mount. This goes to show why mounts are still read-only by default. Another much smaller data loss bug for much bigger containers (~7 TiB) is also addressed. Support is added for the 6.12 kernel release candidate. A bit early this time, so let's hope the build doesn't break again for the final release. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>
Silence gcc warnings due to switch to min()
2026-08-18 02:19:59 -07:00 · 2024-11-01 20:06:39 -03:00 · 2024-10-31 17:01:30 -03:00 · 2024-10-31 16:46:20 -03:00 · 2024-10-31 16:02:06 -03:00 · 2024-10-29 21:13:46 -03:00
9 changed files with 684 additions and 350 deletions
@@ -117,6 +117,11 @@ static inline bool apfs_node_has_fixed_kv_size(struct apfs_node *node)
 	return (node->flags & APFS_BTNODE_FIXED_KV_SIZE) != 0;
 }

+struct apfs_ip_bitmap_block_info {
+	bool	dirty;		/* Do we need to commit this block to disk? */
+	void	*block;		/* In-memory address of the bitmap block */
+};
+
 /*
 * Space manager data in memory.
 */
@@ -147,7 +152,7 @@ struct apfs_spaceman {
 	/* Number of ip bitmaps */
 	u32 sm_ip_bmaps_count;
 	/* List of ip bitmaps, in order */
-	struct buffer_head *sm_ip_bmaps[];
+	struct apfs_ip_bitmap_block_info sm_ip_bmaps[];
 };

 #define APFS_TRANS_MAIN_QUEUE_MAX	10000
@@ -217,6 +222,11 @@ struct apfs_ephemeral_object_info {
 /* Mount option flags for a container */
 #define APFS_CHECK_NODES	1
 #define APFS_READWRITE		2
+/*
+ * Mount options for a container are decided on its first mount. This flag lets
+ * future mounts know that has happened already.
+ */
+#define APFS_FLAGS_SET		4

 /*
 * Container superblock data in memory
@@ -1061,6 +1071,7 @@ extern int apfs_read_spaceman(struct super_block *sb);
 extern int apfs_free_queue_insert_nocache(struct super_block *sb, u64 bno, u64 count);
 extern int apfs_free_queue_insert(struct super_block *sb, u64 bno, u64 count);
 extern int apfs_spaceman_allocate_block(struct super_block *sb, u64 *bno, bool backwards);
+extern int apfs_write_ip_bitmaps(struct super_block *sb);

 /* super.c */
 extern int apfs_map_volume_super_bno(struct super_block *sb, u64 bno, bool check);
@@ -1,5 +1,5 @@
 PACKAGE_NAME="linux-apfs-rw"
-PACKAGE_VERSION="0.3.10"
+PACKAGE_VERSION="0.3.12"

 BUILT_MODULE_NAME[0]="apfs"
 DEST_MODULE_LOCATION[0]="/extra"
@@ -8,8 +8,6 @@
 #include <linux/blk_types.h>
 #include "apfs.h"

-#define MAX(X, Y)	((X) <= (Y) ? (Y) : (X))
-
 /**
 * apfs_ext_is_hole - Does this extent represent a hole in a sparse file?
 * @extent: the extent to check
@@ -1973,7 +1971,7 @@ restart:
 	prev_end = prev_ext.bno + prev_ext.blkcount;
 	if (prev_end < *paddr_end) {
 		/* The extent to put is part of a snapshot */
-		tmp.phys_block_num = MAX(prev_end, paddr_min);
+		tmp.phys_block_num = max(prev_end, paddr_min);
 		tmp.len = (*paddr_end - tmp.phys_block_num) << sb->s_blocksize_bits;
 		ret = apfs_create_update_pext(query, &tmp, -1);
 		*paddr_end = tmp.phys_block_num;
@@ -2113,7 +2111,7 @@ restart:
 	prev_end = prev_ext.bno + prev_ext.blkcount;
 	if (prev_end < *paddr_end) {
 		/* The extent to take is part of a snapshot */
-		tmp.phys_block_num = MAX(prev_end, paddr_min);
+		tmp.phys_block_num = max(prev_end, paddr_min);
 		tmp.len = (*paddr_end - tmp.phys_block_num) << sb->s_blocksize_bits;
 		ret = apfs_create_update_pext(query, &tmp, +1);
 		*paddr_end = tmp.phys_block_num;
@@ -565,7 +565,11 @@ int __apfs_write_begin(struct file *file, struct address_space *mapping, loff_t
 		}
 	}

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+	err = __block_write_begin(page_folio(page), pos, len, apfs_get_new_block);
+#else
 	err = __block_write_begin(page, pos, len, apfs_get_new_block);
+#endif
 	if (err) {
 		apfs_err(sb, "CoW failed in inode 0x%llx", apfs_ino(inode));
 		goto out_put_page;
@@ -580,7 +584,11 @@ out_put_page:
 	return err;
 }

-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 19, 0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+static int apfs_write_begin(struct file *file, struct address_space *mapping,
+			    loff_t pos, unsigned int len,
+			    struct folio **foliop, void **fsdata)
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(5, 19, 0)
 static int apfs_write_begin(struct file *file, struct address_space *mapping,
 			    loff_t pos, unsigned int len,
 			    struct page **pagep, void **fsdata)
@@ -592,6 +600,10 @@ static int apfs_write_begin(struct file *file, struct address_space *mapping,
 {
 	struct inode *inode = mapping->host;
 	struct super_block *sb = inode->i_sb;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+	struct page *page = NULL;
+	struct page **pagep = &page;
+#endif
 	int blkcount = (len + sb->s_blocksize - 1) >> inode->i_blkbits;
 	struct apfs_max_ops maxops;
 	int err;
@@ -615,6 +627,9 @@ static int apfs_write_begin(struct file *file, struct address_space *mapping,
 	err = __apfs_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
 	if (err)
 		goto fail;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+	*foliop = page_folio(page);
+#endif
 	return 0;

 fail:
@@ -628,7 +643,11 @@ int __apfs_write_end(struct file *file, struct address_space *mapping, loff_t po
 	struct apfs_dstream_info *dstream = &APFS_I(inode)->i_dstream;
 	int ret, err;

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+	ret = generic_write_end(file, mapping, pos, len, copied, page_folio(page), fsdata);
+#else
 	ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+#endif
 	dstream->ds_size = i_size_read(inode);
 	if (ret < len && pos + len > inode->i_size) {
 		truncate_pagecache(inode, inode->i_size);
@@ -641,13 +660,22 @@ int __apfs_write_end(struct file *file, struct address_space *mapping, loff_t po
 	return ret;
 }

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+static int apfs_write_end(struct file *file, struct address_space *mapping,
+			  loff_t pos, unsigned int len, unsigned int copied,
+			  struct folio *folio, void *fsdata)
+#else
 static int apfs_write_end(struct file *file, struct address_space *mapping,
 			  loff_t pos, unsigned int len, unsigned int copied,
 			  struct page *page, void *fsdata)
+#endif
 {
 	struct inode *inode = mapping->host;
 	struct super_block *sb = inode->i_sb;
 	struct apfs_nx_transaction *trans = &APFS_NXI(sb)->nx_transaction;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
+	struct page *page = &folio->page;
+#endif
 	int ret, err;

 	ret = __apfs_write_end(file, mapping, pos, len, copied, page, fsdata);
@@ -693,9 +721,9 @@ static const struct address_space_operations apfs_aops = {
 #endif

 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 8, 0)
-	.readahead      = apfs_readahead,
+	.readahead	= apfs_readahead,
 #else
-	.readpages      = apfs_readpages,
+	.readpages	= apfs_readpages,
 #endif

 	.write_begin	= apfs_write_begin,
@@ -12,8 +12,6 @@
 #include <linux/string.h>
 #include "libzbitmap.h"

-#define MIN(x, y)   ((x) > (y) ? (y) : (x))
-
 #define ZBM_MAGIC       "ZBM\x09"
 #define ZBM_MAGIC_SZ    4

@@ -101,13 +101,18 @@ static bool apfs_transaction_has_room(struct super_block *sb, struct apfs_max_op
 static int apfs_read_single_ephemeral_object(struct super_block *sb, struct apfs_checkpoint_mapping *map)
 {
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
+	struct apfs_nx_superblock *raw_sb = NULL;
 	struct apfs_ephemeral_object_info *list = NULL;
 	struct buffer_head *bh = NULL;
 	char *object = NULL;
 	int count;
-	u32 size;
-	u64 bno, oid;
-	int err, i;
+	u32 data_blks, size;
+	u64 data_base, bno, oid;
+	int err, i, data_idx;
+
+	raw_sb = nxi->nx_raw;
+	data_base = le64_to_cpu(raw_sb->nx_xp_data_base);
+	data_blks = le32_to_cpu(raw_sb->nx_xp_data_blocks);

 	list = nxi->nx_eph_list;
 	count = nxi->nx_eph_count;
@@ -135,8 +140,9 @@ static int apfs_read_single_ephemeral_object(struct super_block *sb, struct apfs
 	if (!object)
 		return -ENOMEM;

+	data_idx = bno - data_base;
 	for (i = 0; i < size >> sb->s_blocksize_bits; ++i) {
-		bh = apfs_sb_bread(sb, bno + i);
+		bh = apfs_sb_bread(sb, data_base + data_idx);
 		if (!bh) {
 			apfs_err(sb, "failed to read ephemeral block");
 			err = -EIO;
@@ -145,6 +151,9 @@ static int apfs_read_single_ephemeral_object(struct super_block *sb, struct apfs
 		memcpy(object + (i << sb->s_blocksize_bits), bh->b_data, sb->s_blocksize);
 		brelse(bh);
 		bh = NULL;
+		/* Somewhat surprisingly, objects can wrap around */
+		if (++data_idx == data_blks)
+			data_idx = 0;
 	}

 	/*
@@ -285,10 +294,8 @@ static void apfs_trans_commit_work(struct work_struct *work)
 	 * (TODO).
 	 */
 	down_write(&nxi->nx_big_sem);
-	mutex_lock(&nxs_mutex);
 	if (!sb || sb->s_flags & SB_RDONLY) {
 		/* The commit already took place, or there was an abort */
-		mutex_unlock(&nxs_mutex);
 		up_write(&nxi->nx_big_sem);
 		return;
 	}
@@ -330,11 +337,9 @@ int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops)
 	int err;

 	down_write(&nxi->nx_big_sem);
-	mutex_lock(&nxs_mutex); /* Don't mount during a transaction */

 	if (sb->s_flags & SB_RDONLY) {
 		/* A previous transaction has failed; this should be rare */
-		mutex_unlock(&nxs_mutex);
 		up_write(&nxi->nx_big_sem);
 		return -EROFS;
 	}
@@ -346,7 +351,6 @@ int apfs_transaction_start(struct super_block *sb, struct apfs_max_ops maxops)
 	if (!nxi->nx_eph_list) {
 		err = apfs_read_ephemeral_objects(sb);
 		if (err) {
-			mutex_unlock(&nxs_mutex);
 			up_write(&nxi->nx_big_sem);
 			apfs_err(sb, "failed to read the ephemeral objects");
 			return err;
@@ -440,14 +444,12 @@ int apfs_transaction_flush_all_inodes(struct super_block *sb)
 		list_del_init(&ai->i_list);

 		nx_trans->t_state |= APFS_NX_TRANS_COMMITTING;
-		mutex_unlock(&nxs_mutex);
 		up_write(&nxi->nx_big_sem);

 		/* Unlocked, so it may call evict() and wait for writeback */
 		iput(inode);

 		down_write(&nxi->nx_big_sem);
-		mutex_lock(&nxs_mutex);
 		nx_trans->t_state = 0;

 		/* Transaction aborted during writeback, error code is lost */
@@ -471,18 +473,24 @@ int apfs_transaction_flush_all_inodes(struct super_block *sb)
 static int apfs_write_single_ephemeral_object(struct super_block *sb, struct apfs_obj_phys *obj_raw, const struct apfs_checkpoint_mapping *map)
 {
 	struct apfs_nxsb_info *nxi = APFS_NXI(sb);
+	struct apfs_nx_superblock *raw_sb = NULL;
 	struct buffer_head *bh = NULL;
-	u64 bno;
-	u32 size;
-	int err, i;
+	u64 data_base, bno;
+	u32 data_blks, size;
+	int err, i, data_idx;
+
+	raw_sb = nxi->nx_raw;
+	data_base = le64_to_cpu(raw_sb->nx_xp_data_base);
+	data_blks = le32_to_cpu(raw_sb->nx_xp_data_blocks);

 	bno = le64_to_cpu(map->cpm_paddr);
 	size = le32_to_cpu(map->cpm_size);
 	obj_raw->o_xid = cpu_to_le64(nxi->nx_xid);
 	apfs_multiblock_set_csum((char *)obj_raw, size);

+	data_idx = bno - data_base;
 	for (i = 0; i < size >> sb->s_blocksize_bits; ++i) {
-		bh = apfs_getblk(sb, bno + i);
+		bh = apfs_getblk(sb, data_base + data_idx);
 		if (!bh) {
 			apfs_err(sb, "failed to map ephemeral block");
 			return -EIO;
@@ -496,6 +504,9 @@ static int apfs_write_single_ephemeral_object(struct super_block *sb, struct apf
 		memcpy(bh->b_data, (char *)obj_raw + (i << sb->s_blocksize_bits), sb->s_blocksize);
 		brelse(bh);
 		bh = NULL;
+		/* Somewhat surprisingly, objects can wrap around */
+		if (++data_idx == data_blks)
+			data_idx = 0;
 	}
 	return 0;
 }
@@ -571,15 +582,6 @@ static int apfs_write_ephemeral_objects(struct super_block *sb)
 		eph_info = &nxi->nx_eph_list[i];
 		data_next = (data_index + data_len) % data_blks;
 		obj_blkcnt = eph_info->size >> sb->s_blocksize_bits;
-		if (obj_blkcnt > data_blks - data_next) {
-			/*
-			 * This multiblock object does not fit in what's left
-			 * of the ring buffer, so move it to the beginning and
-			 * leave some empty blocks.
-			 */
-			data_len += data_blks - data_next;
-			data_next = 0;
-		}

 		err = apfs_create_cpoint_map(sb, cpm, eph_info->object, data_base + data_next, eph_info->size);
 		if (err) {
@@ -638,7 +640,6 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 	struct apfs_nx_transaction *nx_trans = &nxi->nx_transaction;
 	struct apfs_bh_info *bhi, *tmp;
 	int err = 0;
-	u32 bmap_idx;

 	ASSERT(!(sb->s_flags & SB_RDONLY));

@@ -662,6 +663,16 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 		}
 		sm->sm_free_cache_base = sm->sm_free_cache_blkcnt = 0;
 	}
+	/*
+	 * Writing the ip bitmaps modifies the spaceman, so it must happen
+	 * before we commit the ephemeral objects. It must also happen after we
+	 * flush the free queue, in case the last freed range was in the ip.
+	 */
+	err = apfs_write_ip_bitmaps(sb);
+	if (err) {
+		apfs_err(sb, "failed to commit the ip bitmaps");
+		return err;
+	}
 	err = apfs_write_ephemeral_objects(sb);
 	if (err)
 		return err;
@@ -682,7 +693,11 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 	}
 	list_for_each_entry_safe(bhi, tmp, &nx_trans->t_buffers, list) {
 		struct buffer_head *bh = bhi->bh;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 11, 0)
+		struct folio *folio = NULL;
+#else
 		struct page *page = NULL;
+#endif
 		bool is_metadata;

 		ASSERT(buffer_trans(bh));
@@ -702,27 +717,44 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 		kfree(bhi);
 		bhi = NULL;

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 11, 0)
+		folio = page_folio(bh->b_page);
+		folio_get(folio);
+#else
 		page = bh->b_page;
 		get_page(page);
+#endif

 		is_metadata = buffer_csum(bh);
 		clear_buffer_csum(bh);
 		put_bh(bh);
 		bh = NULL;

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 11, 0)
+		folio_lock(folio);
+		folio_mkclean(folio);
+#else
 		/* Future writes to mmapped areas should fault for CoW */
 		lock_page(page);
 		page_mkclean(page);
+#endif
 		/* XXX: otherwise, the page cache fills up and crashes the machine */
 		if (!is_metadata) {
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 19, 0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 11, 0)
+			try_to_free_buffers(folio);
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(5, 19, 0)
 			try_to_free_buffers(page_folio(page));
 #else
 			try_to_free_buffers(page);
 #endif
 		}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 11, 0)
+		folio_unlock(folio);
+		folio_put(folio);
+#else
 		unlock_page(page);
 		put_page(page);
+#endif
 	}
 	err = apfs_checkpoint_end(sb);
 	if (err) {
@@ -730,12 +762,6 @@ static int apfs_transaction_commit_nx(struct super_block *sb)
 		return err;
 	}

-	for (bmap_idx = 0; bmap_idx < APFS_SM(sb)->sm_ip_bmaps_count; ++bmap_idx) {
-		brelse(APFS_SM(sb)->sm_ip_bmaps[bmap_idx]);
-		APFS_SM(sb)->sm_ip_bmaps[bmap_idx] = NULL;
-	}
-	APFS_SM(sb)->sm_raw = NULL;
-
 	nx_trans->t_starts_count = 0;
 	nx_trans->t_buffers_count = 0;
 	return 0;
@@ -834,7 +860,6 @@ int apfs_transaction_commit(struct super_block *sb)
 		mod_delayed_work(system_wq, &trans->t_work, msecs_to_jiffies(100));
 	}

-	mutex_unlock(&nxs_mutex);
 	up_write(&nxi->nx_big_sem);
 	return 0;
 }
@@ -921,14 +946,11 @@ void apfs_transaction_abort(struct super_block *sb)
 	struct apfs_nx_transaction *nx_trans = &nxi->nx_transaction;
 	struct apfs_bh_info *bhi, *tmp;
 	struct apfs_inode_info *ai, *ai_tmp;
-	struct apfs_spaceman *sm = NULL;
-	u32 bmap_idx;

 	if (sb->s_flags & SB_RDONLY) {
 		/* Transaction already aborted, do nothing */
 		ASSERT(list_empty(&nx_trans->t_inodes));
 		ASSERT(list_empty(&nx_trans->t_buffers));
-		mutex_unlock(&nxs_mutex);
 		up_write(&nxi->nx_big_sem);
 		return;
 	}
@@ -951,22 +973,12 @@ void apfs_transaction_abort(struct super_block *sb)
 		kfree(bhi);
 	}

-	sm = APFS_SM(sb);
-	if (sm) {
-		for (bmap_idx = 0; bmap_idx < sm->sm_ip_bmaps_count; ++bmap_idx) {
-			brelse(sm->sm_ip_bmaps[bmap_idx]);
-			sm->sm_ip_bmaps[bmap_idx] = NULL;
-		}
-		APFS_SM(sb)->sm_raw = NULL;
-	}
-
 	/*
 	 * It's not possible to undo in-memory changes from old operations in
 	 * the aborted transaction. To avoid corruption, never write again.
 	 */
 	apfs_force_readonly(nxi);

-	mutex_unlock(&nxs_mutex);
 	up_write(&nxi->nx_big_sem);

 	list_for_each_entry_safe(ai, ai_tmp, &nx_trans->t_inodes, i_list) {
@@ -12,10 +12,9 @@
 #include <linux/types.h>
 #include <linux/nls.h>
 #include <linux/ctype.h>
+#include <linux/version.h>
 #include "unicode.h"

-#define MIN(X, Y)	((X) <= (Y) ? (X) : (Y))
-
 /* The arrays of unicode data are defined at the bottom of the file */
 /* TODO: would a single trie with all the data be more efficient? */
 static u16 apfs_nfd_trie[];
@@ -234,7 +233,7 @@ static int apfs_get_normalization_length(const char *utf8str, unsigned int total
 	while (1) {
 		if (!total_len || !*utf8str)
 			return norm_len;
-		utf8len = utf8_to_utf32(utf8str, MIN(total_len, 4), &utf32char);
+		utf8len = utf8_to_utf32(utf8str, min(total_len, 4U), &utf32char);
 		if (utf8len < 0) /* Invalid unicode; don't normalize anything */
 			return 0;

@@ -306,7 +305,7 @@ new_starter:
 		unicode_t utf32char;
 		int utf8len, pos;

-		utf8len = utf8_to_utf32(utf8str, MIN(total_len, 4), &utf32char);
+		utf8len = utf8_to_utf32(utf8str, min(total_len, 4U), &utf32char);
 		for (pos = 0;; pos++, str_pos++) {
 			unicode_t utf32norm;
 			u8 ccc;
Author	SHA1	Message	Date
Ernesto A. Fernández	5a2db9bd68	0.3.12 This release fixes a serious data corruption bug for containers bigger than ~1.32 TiB, which is immediately triggered by a writable mount. This goes to show why mounts are still read-only by default. Another much smaller data loss bug for much bigger containers (~7 TiB) is also addressed. Support is added for the 6.12 kernel release candidate. A bit early this time, so let's hope the build doesn't break again for the final release. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-11-01 20:06:39 -03:00
Ernesto A. Fernández	96e6d41bb7	Silence gcc warnings due to switch to min() Building an old kernel (5.3) with an old version of gcc (4.9.2), I get some warnings about "comparison of distinct pointer types". I'm pretty sure this doesn't matter, and I don't even know if the blame lies with the old compiler or the old kernel, but just silence this by making the integer literals of the warning unsigned like the other argument. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-10-31 17:01:30 -03:00
Woody Suwalski	6c2ce34a67	Fix whitespace damage in apfs_aops definition This has been around for a while so I guess checkpatch isn't picking up on it for some reason. [eafer: took this from a bigger patch, added commit message] Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-10-31 16:46:20 -03:00
Ernesto A. Fernández	a34220c93d	Switch ->write_begin() and ->write_end() to folios The build is broken for the 6.12 release candidate. The problem is that ->write_begin() and ->write_end() now take folios as parameters instead of pages. The same is true, of course, of the associated __block_write_begin() and generic_write_end(). Update the driver. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-10-31 16:02:06 -03:00
Ernesto A. Fernández	49507c65bc	Implement ip bitmap rotation correctly I never fully understood how the ip bitmap list was supposed to work, but now I'm noticing that the difference between what I do and what the official driver does are bad enough to quickly corrupt the list when a container moves back and forth. Of course this is only an issue for filesystems above 1.32 TiB or so, but it's still pretty serious. So, I thought about it harder and I'm pretty sure I finally figured it out: the array entries for each free bitmap are actually kept in a linked list, that starts in sm_ip_bm_free_head and ends in sm_ip_bm_free_tail. All entries outside the list (that is, the entries for the used blocks) are marked with the invalid index 0xFFFF. The tail itself also gets 0xFFFF even though it's free, because it has no next entry - so that accounts for the constant off-by-one weirdness in the number of free blocks. So, implement all this stuff correctly. Stop assuming that all ip bitmaps in use are consecutive, and stop rotating them all in lockstep as soon as the transaction begins. Instead, read all ip bitmaps into memory during the first transaction and keep them there, only tracking if they have been dirtied. At commit time, allocate new blocks for the dirty bitmaps and write them back to disk. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-10-29 21:13:46 -03:00
Ernesto A. Fernández	dfdafd2a07	Wrap ephemeral objects around if there is no room Currently, the driver doesn't allow multiblock ephemeral objects to wrap around. When an object doesn't fit in the checkpoint data area, we just leave some empty space and adjust the mappings accordingly. This seemed like a simple and reasonable solution to me, but the official driver actually demands that all ephemeral objects are kept consecutive (and in the same order as their mappings, which we already do). In practice this means that if you are working with a container with a multiblock spaceman (say 7T in size), and if the unmount transaction just so happens to be the one where the spaceman doesn't fit, then you will be left with a container that is seen as corrupt by the official driver. To make matters worse, checkpoint integrity is checked on mount, so macos will actually mount the container no problem, but silently discarding my final transaction. This is all, of course, extremely unlikely. Such large containers are rare, and checkpoint data areas for them are huge so a wrap-around takes a while to happen. And it needs to happen precisely during the unmount transaction, and that same transaction has to involve important data the user doesn't want to lose. All in all it's not a serious issue, but get it fixed. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-10-22 17:34:51 -03:00
Integral	2920180ed6	Remove unused macro MIN()/MAX() & replace them with min()/max() (#78 )	2024-10-04 16:04:05 -03:00
Ernesto A. Fernández	0667dbff17	0.3.11 This release prevents deadlock if a single volume gets mounted and unmounted at the same time. It also fixes two other (smaller) mount bugs found while testing this one. Support is added for the upcoming 6.11 kernel release, with a patch by Woody Suwalski and a later patch of my own because things broke a second time. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-09-12 20:20:49 -03:00
Ernesto A. Fernández	af2ee526c0	Avoid double parsing of mount options Originally all mount options were parsed inside apfs_fill_super(), because I was following the example of other filesystems (such as ext2) that have no subvolumes or snapshots. When I implemented those features, I realized that some mount options would be needed early on mount, so that sget() could use them to decide if the superblock is already mounted. My lazy solution at the time was to leave parse_options() as it was, but also add some redundant code early on mount to read the needed options. Just looking at the amount of TODOs in those functions makes it clear that this was always very silly. And now that the locking during mount has been simplified, this is causing a race, and I want to take that stuff seriously. The bug is that while parse_options() rereads the mount options needed by sget(), there is nothing preventing another concurrent mount from testing the superblock. Say the first mount is for volume 1: the volume number gets briefly reset to the default value of zero, and if the other mount actually wants volume 0, sget() will consider this a match. Anyway, split parse_options() into two equally clean functions, one for early mount and another to get called within apfs_fill_super(). I even considered simply parsing all the options early during mount, but that would require a few more changes because the nxi is not attached at that point. Maybe some other time. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-09-11 16:54:47 -03:00
Ernesto A. Fernández	44f4b4f631	Don't redefine MAX() and MIN() for the 6.11 kernel The build has become broken once again for the 6.11 kernel: https://github.com/linux-apfs/linux-apfs-rw/issues/75 The problem is that the second release candidate introduced generic MAX() and MIN() macros for everyone to use, and that crashes with our own definitions. So, only define them for older kernels. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-08-30 20:25:44 -03:00
Ernesto A. Fernández	b44d2a37bf	Don't forget to initialize the omap cache spinlock The omap cache has been in place for years, and it seems that its spinlock was never actually initialized before use. I have no idea what the consequences of this are, but for one it does cause lockdep to turn itself off after every mount. Fix it. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-08-15 16:01:07 -03:00
Ernesto A. Fernández	4881d7c707	Don't return 0 on mount if sget() fails For the previous patch I had to carefully test all the possible errors during mount and unmount. I noticed that injecting an error inside apfs_set_super() triggers npd. The cause is that the error code returned by sget() gets swallowed and the kernel is allowed to think that the mount succeeded. This is not a new issue, it's been around since apfs_mount() was first implemented. I never noticed because apfs_set_super() would only fail if we ran out of anonymous devices. So fix it in a new commit. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-08-15 16:01:07 -03:00
Ernesto A. Fernández	2982c8655f	Don't use the mount mutex so much Repeatedly running generic/604 of xfstests eventually hangs the kernel. This is a test that forces a mount to race with an unmount. The problem is a standard deadlock: ->put_super() gets called while the s_umount lock is held, and then starts a transaction, which takes nxs_mutex; meanwhile ->mount() first takes nxs_mutex and then calls sget(), which takes s_umount. Before this patch, I had never really put much thought as to where exactly nxs_mutex is needed. It's obviously the lock for the list of mounted containers, but it's also used as a generic way to avoid modifications to a container while another of its subvolumes is getting mounted. It's always been obvious that I'm holding it for too long and in too many places, so I want to finally put some effort into figuring this out now, and leave a record here so that I don't forget. The first obvious point is that the mutex can be dropped before calling sget(), and retaken later. There should be no problem if another mount called it at the same time, and it's enough to fix the bug. But putting some thought here exposes an existing issue with this code: sget() may actually test some superblock that is already half-unmounted and had its info freed, so apfs_test_super() is risking npd or use-after-free. Looking at other filesystems I realize that my mistake was freeing the info inside ->put_super(), because at that point the super is still listed in the fs_supers list for apfs. Instead ->kill_sb() should free the info itself, after generic_shutdown_super() is done (which calls ->put_super()), and after kill_super_notify() has taken care of the list. Since kill_super_notify() can't be called from inside a driver, a little hack is needed to call it through kill_anon_super(). The cleanups for apfs_mount() also need to be reconsidered, since ->put_super() is only called by deactivate_locked_super() after the root dentry is set, but the rest of ->kill_sb() runs either way. Going back to the mutex: even if a race for sget() itself is not an issue, dropping it there does have other consequences. The reference count for the container superblock is increased before sget(), and it's later checked on some initializations to see if this is the first mount. That won't do anymore, because a second mount could increase the refcnt before the mutex is retaken, and the first mount would never complete its work. Use other fields for this check: in apfs_read_main_super() use the blocksize (which is zero before the first mount sets it); for apfs_set_nx_flags(), define a new flag to tell if the other flags have been set. This already tells us two functions that definitely need to be protected by the mutex, so that they can complete their initializations before any other mount comes to check them. The situation is similar for apfs_first_read_omap(), which goes through the container's volume list to check for previous reads. After going line by line through the mount code, I'm fairly confident that the mutex only needs to be held for apfs_attach_nxi() and these three functions. For unmount it's the same: apfs_free_main_super() and apfs_put_omap() require the mutex. The second will now unset the omap pointer for the volume atomically, so that it can be checked by apfs_first_read_omap(); it is renamed accordingly. So far this is all about the races between different mounts and umounts; but we also don't want the mount to read inconsistent data from disk because some existing mount happened to write to it. Back when the driver could only mount one volume at a time, apfs_fill_super() didn't need any locking, so when multiple mounts got implemented I decided to just ban transactions during mounts entirely, by taking the nxs_mutex. This double lock looks very awkward and is a factor on the deadlock we are fixing, so let's clean that up now: use the read semaphore itself to protect apfs_fill_super(). Of course this means that two mounts for the same container can now run through this function at the same time, so be careful with changes to the shared nxi: we've already dealt with this issue for apfs_set_nx_flags(), but apfs_set_trans_buffer_limit() might also be unsafe. Just in case, move it to apfs_read_main_super(), it makes more sense that way anyway. By the way, apfs_read_main_super() also reads from disk, but it only does it on the first mount, so there is no risk of racing against a transaction. There is one function that writes without a transaction, namely apfs_make_super_copy(), but that function takes the nxs_mutex to ensure that no mount will begin during the superblock update. I think that covers everything? I'm really not sure if this writeup was clear at all, I have a hard time reasoning about locks. I also put a simpler list of the uses of nxs_mutex in a comment next to its declaration, which should be more practical as a reference. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-08-15 16:01:06 -03:00
Ernesto A. Fernández	707d91b486	Undo code reordering in previous patch I just picked up a patch to add support for the 6.11 kernel. I only did a shallow review, and now I notice that the code was reordered and a null buffer head is getting dereferenced. Revert that. Signed-off-by: Ernesto A. Fernández <ernesto@corellium.com>	2024-07-30 17:55:14 -03:00
Woody Suwalski	3e5c902a9a	Kernel 6.11: use folio for page ops (#74 )	2024-07-30 17:23:17 -03:00