Commit 4d998142 authored by Linus Torvalds's avatar Linus Torvalds
Browse files
Pull hfsplus updates from Viacheslav Dubeyko:
 "This contains several fixes of syzbot reported issues and HFS+ fixes
  of xfstests failures.

   - Fix a syzbot reported issue of a KMSAN uninit-value in
     hfsplus_strcasecmp().

     The root cause was that hfs_brec_read() doesn't validate that the
     on-disk record size matches the expected size for the record type
     being read. The fix introduced hfsplus_brec_read_cat() wrapper that
     validates the record size based on the type field and returns -EIO
     if size doesn't match (Deepanshu Kartikey)

   - Fix a syzbot reported issue of processing corrupted HFS+ images
     where the b-tree allocation bitmap indicates that the header node
     (Node 0) is free. Node 0 must always be allocated. Violating this
     invariant leads to allocator corruption, which cascades into kernel
     panics or undefined behavior.

     Prevent trusting a corrupted allocator state by adding a validation
     check during hfs_btree_open(). If corruption is detected, print a
     warning identifying the specific corrupted tree and force the
     filesystem to mount read-only (SB_RDONLY).

     This prevents kernel panics from corrupted images while enabling
     data recovery (Shardul Bankar)

   - Fix a potential deadlock in hfsplus_fill_super().

     hfsplus_fill_super() calls hfs_find_init() to initialize a search
     structure, which acquires tree->tree_lock. If the subsequent call
     to hfsplus_cat_build_key() fails, the function jumps to the
     out_put_root error label without releasing the lock.

     Fix this by adding the missing hfs_find_exit(&fd) call before
     jumping to the out_put_root error label. This ensures that
     tree->tree_lock is properly released on the error path (Zilin Guan)

   - Update a files ctime after rename in hfsplus_rename() (Yangtao Li)

  The rest of the patches introduce the HFS+ fixes for the case of
  generic/348, generic/728, generic/533, generic/523, and generic/642
  test-cases of xfstests suite"

* tag 'hfs-v7.1-tag1' of git://git.kernel.org/pub/scm/linux/kernel/git/vdubeyko/hfs:
  hfsplus: fix generic/642 failure
  hfsplus: rework logic of map nodes creation in xattr b-tree
  hfsplus: fix logic of alloc/free b-tree node
  hfsplus: fix error processing issue in hfs_bmap_free()
  hfsplus: fix potential race conditions in b-tree functionality
  hfsplus: extract hidden directory search into a helper function
  hfsplus: fix held lock freed on hfsplus_fill_super()
  hfsplus: fix generic/523 test-case failure
  hfsplus: validate b-tree node 0 bitmap at mount time
  hfsplus: refactor b-tree map page access and add node-type validation
  hfsplus: fix to update ctime after rename
  hfsplus: fix generic/533 test-case failure
  hfsplus: set ctime after setxattr and removexattr
  hfsplus: fix uninit-value by validating catalog record size
  hfsplus: fix potential Allocation File corruption after fsync
parents f3756afb c1307d18
Loading
Loading
Loading
Loading
+34 −10
Original line number Diff line number Diff line
@@ -57,7 +57,8 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
	if (name) {
		int res = hfsplus_asc2uni(sb,
				(struct hfsplus_unistr *)&key->attr.key_name,
				HFSPLUS_ATTR_MAX_STRLEN, name, strlen(name));
				HFSPLUS_ATTR_MAX_STRLEN, name, strlen(name),
				HFS_XATTR_NAME);
		if (res)
			return res;
		len = be16_to_cpu(key->attr.key_name.length);
@@ -153,14 +154,22 @@ int hfsplus_find_attr(struct super_block *sb, u32 cnid,
		if (err)
			goto failed_find_attr;
		err = hfs_brec_find(fd, hfs_find_rec_by_key);
		if (err)
		if (err == -ENOENT) {
			/* file exists but xattr is absent */
			err = -ENODATA;
			goto failed_find_attr;
		} else if (err)
			goto failed_find_attr;
	} else {
		err = hfsplus_attr_build_key(sb, fd->search_key, cnid, NULL);
		if (err)
			goto failed_find_attr;
		err = hfs_brec_find(fd, hfs_find_1st_rec_by_cnid);
		if (err)
		if (err == -ENOENT) {
			/* file exists but xattr is absent */
			err = -ENODATA;
			goto failed_find_attr;
		} else if (err)
			goto failed_find_attr;
	}

@@ -174,6 +183,9 @@ int hfsplus_attr_exists(struct inode *inode, const char *name)
	struct super_block *sb = inode->i_sb;
	struct hfs_find_data fd;

	hfs_dbg("name %s, ino %llu\n",
		name ? name : NULL, inode->i_ino);

	if (!HFSPLUS_SB(sb)->attr_tree)
		return 0;

@@ -241,6 +253,7 @@ int hfsplus_create_attr_nolock(struct inode *inode, const char *name,
		return err;
	}

	hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(sb), HFSPLUS_I_ATTR_DIRTY);
	hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY);

	return 0;
@@ -292,15 +305,16 @@ int hfsplus_create_attr(struct inode *inode,
static int __hfsplus_delete_attr(struct inode *inode, u32 cnid,
					struct hfs_find_data *fd)
{
	int err = 0;
	int err;
	__be32 found_cnid, record_type;

	found_cnid = U32_MAX;
	hfs_bnode_read(fd->bnode, &found_cnid,
			fd->keyoffset +
			offsetof(struct hfsplus_attr_key, cnid),
			sizeof(__be32));
	if (cnid != be32_to_cpu(found_cnid))
		return -ENOENT;
		return -ENODATA;

	hfs_bnode_read(fd->bnode, &record_type,
			fd->entryoffset, sizeof(record_type));
@@ -326,8 +340,10 @@ static int __hfsplus_delete_attr(struct inode *inode, u32 cnid,
	if (err)
		return err;

	hfsplus_mark_inode_dirty(HFSPLUS_ATTR_TREE_I(inode->i_sb),
				 HFSPLUS_I_ATTR_DIRTY);
	hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY);
	return err;
	return 0;
}

static
@@ -351,7 +367,10 @@ int hfsplus_delete_attr_nolock(struct inode *inode, const char *name,
	}

	err = hfs_brec_find(fd, hfs_find_rec_by_key);
	if (err)
	if (err == -ENOENT) {
		/* file exists but xattr is absent */
		return -ENODATA;
	} else if (err)
		return err;

	err = __hfsplus_delete_attr(inode, inode->i_ino, fd);
@@ -411,8 +430,13 @@ int hfsplus_delete_all_attrs(struct inode *dir, u32 cnid)

	for (;;) {
		err = hfsplus_find_attr(dir->i_sb, cnid, NULL, &fd);
		if (err) {
			if (err != -ENOENT)
		if (err == -ENOENT || err == -ENODATA) {
			/*
			 * xattr has not been found
			 */
			err = -ENODATA;
			goto end_delete_all;
		} else if (err) {
			pr_err("xattr search failed\n");
			goto end_delete_all;
		}
+51 −0
Original line number Diff line number Diff line
@@ -287,3 +287,54 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
	fd->bnode = bnode;
	return res;
}

/**
 * hfsplus_brec_read_cat - read and validate a catalog record
 * @fd: find data structure
 * @entry: pointer to catalog entry to read into
 *
 * Reads a catalog record and validates its size matches the expected
 * size based on the record type.
 *
 * Returns 0 on success, or negative error code on failure.
 */
int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
{
	int res;
	u32 expected_size;

	res = hfs_brec_read(fd, entry, sizeof(hfsplus_cat_entry));
	if (res)
		return res;

	/* Validate catalog record size based on type */
	switch (be16_to_cpu(entry->type)) {
	case HFSPLUS_FOLDER:
		expected_size = sizeof(struct hfsplus_cat_folder);
		break;
	case HFSPLUS_FILE:
		expected_size = sizeof(struct hfsplus_cat_file);
		break;
	case HFSPLUS_FOLDER_THREAD:
	case HFSPLUS_FILE_THREAD:
		/* Ensure we have at least the fixed fields before reading nodeName.length */
		if (fd->entrylength < HFSPLUS_MIN_THREAD_SZ) {
			pr_err("thread record too short (got %u)\n", fd->entrylength);
			return -EIO;
		}
		expected_size = hfsplus_cat_thread_size(&entry->thread);
		break;
	default:
		pr_err("unknown catalog record type %d\n",
		       be16_to_cpu(entry->type));
		return -EIO;
	}

	if (fd->entrylength != expected_size) {
		pr_err("catalog record size mismatch (type %d, got %u, expected %u)\n",
		       be16_to_cpu(entry->type), fd->entrylength, expected_size);
		return -EIO;
	}

	return 0;
}
+3 −0
Original line number Diff line number Diff line
@@ -420,7 +420,10 @@ void hfs_bnode_unlink(struct hfs_bnode *node)
		tree->root = 0;
		tree->depth = 0;
	}

	spin_lock(&tree->hash_lock);
	set_bit(HFS_BNODE_DELETED, &node->flags);
	spin_unlock(&tree->hash_lock);
}

static inline int hfs_bnode_hash(u32 num)
+20 −12
Original line number Diff line number Diff line
@@ -239,6 +239,9 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd)
	struct hfs_bnode_desc node_desc;
	int num_recs, new_rec_off, new_off, old_rec_off;
	int data_start, data_end, size;
	size_t rec_off_tbl_size;
	size_t node_desc_size = sizeof(struct hfs_bnode_desc);
	size_t rec_size = sizeof(__be16);

	tree = fd->tree;
	node = fd->bnode;
@@ -265,18 +268,22 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd)
		return next_node;
	}

	size = tree->node_size / 2 - node->num_recs * 2 - 14;
	old_rec_off = tree->node_size - 4;
	rec_off_tbl_size = node->num_recs * rec_size;
	size = tree->node_size / 2;
	size -= node_desc_size;
	size -= rec_off_tbl_size;
	old_rec_off = tree->node_size - (2 * rec_size);

	num_recs = 1;
	for (;;) {
		data_start = hfs_bnode_read_u16(node, old_rec_off);
		if (data_start > size)
			break;
		old_rec_off -= 2;
		old_rec_off -= rec_size;
		if (++num_recs < node->num_recs)
			continue;
		/* panic? */
		hfs_bnode_put(node);
		hfs_bnode_unlink(new_node);
		hfs_bnode_put(new_node);
		if (next_node)
			hfs_bnode_put(next_node);
@@ -287,7 +294,7 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd)
		/* new record is in the lower half,
		 * so leave some more space there
		 */
		old_rec_off += 2;
		old_rec_off += rec_size;
		num_recs--;
		data_start = hfs_bnode_read_u16(node, old_rec_off);
	} else {
@@ -295,27 +302,28 @@ static struct hfs_bnode *hfs_bnode_split(struct hfs_find_data *fd)
		hfs_bnode_get(new_node);
		fd->bnode = new_node;
		fd->record -= num_recs;
		fd->keyoffset -= data_start - 14;
		fd->entryoffset -= data_start - 14;
		fd->keyoffset -= data_start - node_desc_size;
		fd->entryoffset -= data_start - node_desc_size;
	}
	new_node->num_recs = node->num_recs - num_recs;
	node->num_recs = num_recs;

	new_rec_off = tree->node_size - 2;
	new_off = 14;
	new_rec_off = tree->node_size - rec_size;
	new_off = node_desc_size;
	size = data_start - new_off;
	num_recs = new_node->num_recs;
	data_end = data_start;
	while (num_recs) {
		hfs_bnode_write_u16(new_node, new_rec_off, new_off);
		old_rec_off -= 2;
		new_rec_off -= 2;
		old_rec_off -= rec_size;
		new_rec_off -= rec_size;
		data_end = hfs_bnode_read_u16(node, old_rec_off);
		new_off = data_end - size;
		num_recs--;
	}
	hfs_bnode_write_u16(new_node, new_rec_off, new_off);
	hfs_bnode_copy(new_node, 14, node, data_start, data_end - data_start);
	hfs_bnode_copy(new_node, node_desc_size,
			node, data_start, data_end - data_start);

	/* update new bnode header */
	node_desc.next = cpu_to_be32(new_node->next);
+194 −47
Original line number Diff line number Diff line
@@ -129,12 +129,148 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
	return clump_size;
}

/* Context for iterating b-tree map pages
 * @page_idx: The index of the page within the b-node's page array
 * @off: The byte offset within the mapped page
 * @len: The remaining length of the map record
 */
struct hfs_bmap_ctx {
	unsigned int page_idx;
	unsigned int off;
	u16 len;
};

/*
 * Finds the specific page containing the requested byte offset within the map
 * record. Automatically handles the difference between header and map nodes.
 * Returns the struct page pointer, or an ERR_PTR on failure.
 * Note: The caller is responsible for mapping/unmapping the returned page.
 */
static struct page *hfs_bmap_get_map_page(struct hfs_bnode *node,
					  struct hfs_bmap_ctx *ctx,
					  u32 byte_offset)
{
	u16 rec_idx, off16;
	unsigned int page_off;

	if (node->this == HFSPLUS_TREE_HEAD) {
		if (node->type != HFS_NODE_HEADER) {
			pr_err("hfsplus: invalid btree header node\n");
			return ERR_PTR(-EIO);
		}
		rec_idx = HFSPLUS_BTREE_HDR_MAP_REC_INDEX;
	} else {
		if (node->type != HFS_NODE_MAP) {
			pr_err("hfsplus: invalid btree map node\n");
			return ERR_PTR(-EIO);
		}
		rec_idx = HFSPLUS_BTREE_MAP_NODE_REC_INDEX;
	}

	ctx->len = hfs_brec_lenoff(node, rec_idx, &off16);
	if (!ctx->len)
		return ERR_PTR(-ENOENT);

	if (!is_bnode_offset_valid(node, off16))
		return ERR_PTR(-EIO);

	ctx->len = check_and_correct_requested_length(node, off16, ctx->len);

	if (byte_offset >= ctx->len)
		return ERR_PTR(-EINVAL);

	page_off = (u32)off16 + node->page_offset + byte_offset;
	ctx->page_idx = page_off >> PAGE_SHIFT;
	ctx->off = page_off & ~PAGE_MASK;

	return node->page[ctx->page_idx];
}

/**
 * hfs_bmap_test_bit - test a bit in the b-tree map
 * @node: the b-tree node containing the map record
 * @node_bit_idx: the relative bit index within the node's map record
 *
 * Returns true if set, false if clear or on failure.
 */
static bool hfs_bmap_test_bit(struct hfs_bnode *node, u32 node_bit_idx)
{
	struct hfs_bmap_ctx ctx;
	struct page *page;
	u8 *bmap, byte, mask;

	page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE);
	if (IS_ERR(page))
		return false;

	bmap = kmap_local_page(page);
	byte = bmap[ctx.off];
	kunmap_local(bmap);

	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));
	return (byte & mask) != 0;
}


/**
 * hfs_bmap_clear_bit - clear a bit in the b-tree map
 * @node: the b-tree node containing the map record
 * @node_bit_idx: the relative bit index within the node's map record
 *
 * Returns 0 on success, -EINVAL if already clear, or negative error code.
 */
static int hfs_bmap_clear_bit(struct hfs_bnode *node, u32 node_bit_idx)
{
	struct hfs_bmap_ctx ctx;
	struct page *page;
	u8 *bmap, mask;

	page = hfs_bmap_get_map_page(node, &ctx, node_bit_idx / BITS_PER_BYTE);
	if (IS_ERR(page))
		return PTR_ERR(page);

	bmap = kmap_local_page(page);

	mask = 1 << (7 - (node_bit_idx % BITS_PER_BYTE));

	if (!(bmap[ctx.off] & mask)) {
		kunmap_local(bmap);
		return -EINVAL;
	}

	bmap[ctx.off] &= ~mask;
	set_page_dirty(page);
	kunmap_local(bmap);

	return 0;
}

#define HFS_EXTENT_TREE_NAME  "Extents Overflow File"
#define HFS_CATALOG_TREE_NAME "Catalog File"
#define HFS_ATTR_TREE_NAME    "Attributes File"
#define HFS_UNKNOWN_TREE_NAME "Unknown B-tree"

static const char *hfs_btree_name(u32 cnid)
{
	switch (cnid) {
	case HFSPLUS_EXT_CNID:
		return HFS_EXTENT_TREE_NAME;
	case HFSPLUS_CAT_CNID:
		return HFS_CATALOG_TREE_NAME;
	case HFSPLUS_ATTR_CNID:
		return HFS_ATTR_TREE_NAME;
	default:
		return HFS_UNKNOWN_TREE_NAME;
	}
}

/* Get a reference to a B*Tree and do some initial checks */
struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
{
	struct hfs_btree *tree;
	struct hfs_btree_header_rec *head;
	struct address_space *mapping;
	struct hfs_bnode *node;
	struct inode *inode;
	struct page *page;
	unsigned int size;
@@ -242,6 +378,20 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)

	kunmap_local(head);
	put_page(page);

	node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD);
	if (IS_ERR(node))
		goto free_inode;

	if (!hfs_bmap_test_bit(node, 0)) {
		pr_warn("(%s): %s (cnid 0x%x) map record invalid or bitmap corruption detected, forcing read-only.\n",
				sb->s_id, hfs_btree_name(id), id);
		pr_warn("Run fsck.hfsplus to repair.\n");
		sb->s_flags |= SB_RDONLY;
	}

	hfs_bnode_put(node);

	return tree;

 fail_page:
@@ -351,6 +501,8 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
	u32 count;
	int res;

	lockdep_assert_held(&tree->tree_lock);

	if (rsvd_nodes <= 0)
		return 0;

@@ -374,14 +526,14 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
{
	struct hfs_bnode *node, *next_node;
	struct page **pagep;
	struct hfs_bmap_ctx ctx;
	struct page *page;
	u32 nidx, idx;
	unsigned off;
	u16 off16;
	u16 len;
	u8 *data, byte, m;
	int i, res;

	lockdep_assert_held(&tree->tree_lock);

	res = hfs_bmap_reserve(tree, 1);
	if (res)
		return ERR_PTR(res);
@@ -390,32 +542,29 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
	node = hfs_bnode_find(tree, nidx);
	if (IS_ERR(node))
		return node;
	len = hfs_brec_lenoff(node, 2, &off16);
	off = off16;

	if (!is_bnode_offset_valid(node, off)) {
	page = hfs_bmap_get_map_page(node, &ctx, 0);
	if (IS_ERR(page)) {
		res = PTR_ERR(page);
		hfs_bnode_put(node);
		return ERR_PTR(-EIO);
		return ERR_PTR(res);
	}
	len = check_and_correct_requested_length(node, off, len);

	off += node->page_offset;
	pagep = node->page + (off >> PAGE_SHIFT);
	data = kmap_local_page(*pagep);
	off &= ~PAGE_MASK;
	data = kmap_local_page(page);
	idx = 0;

	for (;;) {
		while (len) {
			byte = data[off];
		while (ctx.len) {
			byte = data[ctx.off];
			if (byte != 0xff) {
				for (m = 0x80, i = 0; i < 8; m >>= 1, i++) {
					if (!(byte & m)) {
						idx += i;
						data[off] |= m;
						set_page_dirty(*pagep);
						data[ctx.off] |= m;
						set_page_dirty(page);
						kunmap_local(data);
						tree->free_nodes--;
						hfs_btree_write(tree);
						mark_inode_dirty(tree->inode);
						hfs_bnode_put(node);
						return hfs_bnode_create(tree,
@@ -423,19 +572,21 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
					}
				}
			}
			if (++off >= PAGE_SIZE) {
			if (++ctx.off >= PAGE_SIZE) {
				kunmap_local(data);
				data = kmap_local_page(*++pagep);
				off = 0;
				page = node->page[++ctx.page_idx];
				data = kmap_local_page(page);
				ctx.off = 0;
			}
			idx += 8;
			len--;
			ctx.len--;
		}
		kunmap_local(data);
		nidx = node->next;
		if (!nidx) {
			hfs_dbg("create new bmap node\n");
			next_node = hfs_bmap_new_bmap(node, idx);
			hfs_btree_write(tree);
		} else
			next_node = hfs_bnode_find(tree, nidx);
		hfs_bnode_put(node);
@@ -443,26 +594,27 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
			return next_node;
		node = next_node;

		len = hfs_brec_lenoff(node, 0, &off16);
		off = off16;
		off += node->page_offset;
		pagep = node->page + (off >> PAGE_SHIFT);
		data = kmap_local_page(*pagep);
		off &= ~PAGE_MASK;
		page = hfs_bmap_get_map_page(node, &ctx, 0);
		if (IS_ERR(page)) {
			res = PTR_ERR(page);
			hfs_bnode_put(node);
			return ERR_PTR(res);
		}
		data = kmap_local_page(page);
	}
}

void hfs_bmap_free(struct hfs_bnode *node)
{
	struct hfs_btree *tree;
	struct page *page;
	u16 off, len;
	u32 nidx;
	u8 *data, byte, m;
	int res;

	hfs_dbg("node %u\n", node->this);
	BUG_ON(!node->this);
	tree = node->tree;
	lockdep_assert_held(&tree->tree_lock);
	nidx = node->this;
	node = hfs_bnode_find(tree, 0);
	if (IS_ERR(node))
@@ -495,24 +647,19 @@ void hfs_bmap_free(struct hfs_bnode *node)
		}
		len = hfs_brec_lenoff(node, 0, &off);
	}
	off += node->page_offset + nidx / 8;
	page = node->page[off >> PAGE_SHIFT];
	data = kmap_local_page(page);
	off &= ~PAGE_MASK;
	m = 1 << (~nidx & 7);
	byte = data[off];
	if (!(byte & m)) {
		pr_crit("trying to free free bnode "
				"%u(%d)\n",
			node->this, node->type);
		kunmap_local(data);
		hfs_bnode_put(node);
		return;
	}
	data[off] = byte & ~m;
	set_page_dirty(page);
	kunmap_local(data);
	hfs_bnode_put(node);

	res = hfs_bmap_clear_bit(node, nidx);
	if (res == -EINVAL) {
		pr_crit("trying to free the freed bnode %u(%d)\n",
			nidx, node->type);
	} else if (res) {
		pr_crit("fail to free bnode %u(%d)\n",
			nidx, node->type);
	} else {
		tree->free_nodes++;
		hfs_btree_write(tree);
		mark_inode_dirty(tree->inode);
	}

	hfs_bnode_put(node);
}
Loading