[PATCH] Improved hfsplus in the kernel

From: Pranjal Prasad
Date: Tue Feb 25 2025 - 15:38:04 EST

Hi,

Please find attached the patch to improve the HFS+ filesystem support in the kernel. I have not done much work, but as I require HFS, HFS+, and APFS in Linux, I decided to maintain it. Please allow me to be the maintainer of HFS and HFS+ in the kernel.

Best regards,
Pranjal Prasad
From 201b5513650ec86b87a64fa584013da8080e9ee3 Mon Sep 17 00:00:00 2001
From: Pranjal Prasad <prasadpranjal213@xxxxxxxxx>
Date: Wed, 26 Feb 2025 01:51:09 +0530
Subject: [PATCH] Improved hfsplus in the kernel

---
fs/hfsplus/attributes.c | 113 +++++++++++++-----
fs/hfsplus/bfind.c | 153 ++++++++++++++++---------
fs/hfsplus/btree.c | 223 +++++++++++++++++++++---------------
fs/hfsplus/xattr_security.c | 50 +++++---
4 files changed, 353 insertions(+), 186 deletions(-)

diff --git a/fs/hfsplus/attributes.c b/fs/hfsplus/attributes.c
index eeebe80c6be4..81d64944b145 100644
--- a/fs/hfsplus/attributes.c
+++ b/fs/hfsplus/attributes.c
@@ -5,6 +5,9 @@
* Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
*
* Handling of records in attributes tree
+ *
+ * Pranjal Prasad <prasadpranjal213@xxxxxxxxx>
+ * Improved and added comments
*/

#include "hfsplus_fs.h"
@@ -79,91 +82,149 @@ int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,

return 0;
}
-
+/*
+ * Allocate memory for an attribute entry from the attribute tree cache.
+ * This function uses the kernel's memory cache system to allocate memory
+ * for an attribute entry.
+ */
hfsplus_attr_entry *hfsplus_alloc_attr_entry(void)
{
- return kmem_cache_alloc(hfsplus_attr_tree_cachep, GFP_KERNEL);
+ return kmem_cache_alloc(hfsplus_attr_tree_cachep, GFP_KERNEL); // Allocates memory from the cache.
}

+/*
+ * Free an attribute entry back to the attribute tree cache.
+ * This function releases memory allocated for an attribute entry back to
+ * the kernel's memory cache system.
+ */
void hfsplus_destroy_attr_entry(hfsplus_attr_entry *entry)
{
- if (entry)
- kmem_cache_free(hfsplus_attr_tree_cachep, entry);
+ if (entry) // Check if the entry is valid (non-NULL).
+ kmem_cache_free(hfsplus_attr_tree_cachep, entry); // Frees the entry back to the cache.
}

+/*
+ * Constant for invalid attribute record.
+ * Used when the record type is invalid or when an error occurs during record creation.
+ */
#define HFSPLUS_INVALID_ATTR_RECORD -1

+/*
+ * Builds an attribute record in the provided entry based on the record type.
+ * Depending on the record type, this function either initializes the entry for
+ * a specific attribute type or returns an error code if the type is unsupported.
+ *
+ * record_type: The type of the record (e.g., fork data, extents, inline data).
+ * cnid: The CNID of the entry (used for file identification).
+ * value: The value to store in the entry (if applicable).
+ * size: The size of the value.
+ *
+ * Returns: The size of the record, or HFSPLUS_INVALID_ATTR_RECORD on error.
+ */
static int hfsplus_attr_build_record(hfsplus_attr_entry *entry, int record_type,
- u32 cnid, const void *value, size_t size)
+ u32 cnid, const void *value, size_t size)
{
+ // Handle fork data attributes (Mac OS X supports only inline data for forks)
if (record_type == HFSPLUS_ATTR_FORK_DATA) {
/*
- * Mac OS X supports only inline data attributes.
- * Do nothing
+ * Mac OS X supports only inline data attributes for forks.
+ * No action needed here, just initialize the entry with zeros.
*/
memset(entry, 0, sizeof(*entry));
- return sizeof(struct hfsplus_attr_fork_data);
+ return sizeof(struct hfsplus_attr_fork_data); // Return the size of the fork data record.
+
+ // Handle extents attributes (again, only inline data is supported)
} else if (record_type == HFSPLUS_ATTR_EXTENTS) {
/*
- * Mac OS X supports only inline data attributes.
- * Do nothing.
+ * Mac OS X supports only inline data attributes for extents.
+ * No action needed here, just initialize the entry with zeros.
*/
memset(entry, 0, sizeof(*entry));
- return sizeof(struct hfsplus_attr_extents);
+ return sizeof(struct hfsplus_attr_extents); // Return the size of the extents record.
+
+ // Handle inline data attributes
} else if (record_type == HFSPLUS_ATTR_INLINE_DATA) {
u16 len;

+ // Initialize the entry for inline data attributes
memset(entry, 0, sizeof(struct hfsplus_attr_inline_data));
- entry->inline_data.record_type = cpu_to_be32(record_type);
+ entry->inline_data.record_type = cpu_to_be32(record_type); // Set the record type in CPU-to-BE format.
+
+ // If the size is within the allowed range for inline data, proceed to set the length.
if (size <= HFSPLUS_MAX_INLINE_DATA_SIZE)
len = size;
else
- return HFSPLUS_INVALID_ATTR_RECORD;
- entry->inline_data.length = cpu_to_be16(len);
+ return HFSPLUS_INVALID_ATTR_RECORD; // Return an error if the size exceeds the limit.
+
+ // Set the length of the inline data attribute (in CPU-to-BE format)
+ entry->inline_data.length = cpu_to_be16(len);
+
+ // Copy the raw bytes of the data into the entry (memory location for the data).
memcpy(entry->inline_data.raw_bytes, value, len);
- /*
- * Align len on two-byte boundary.
- * It needs to add pad byte if we have odd len.
- */
+
+ // Ensure the length is aligned to a two-byte boundary. If the length is odd, we add a padding byte.
len = round_up(len, 2);
- return offsetof(struct hfsplus_attr_inline_data, raw_bytes) +
- len;
- } else /* invalid input */
- memset(entry, 0, sizeof(*entry));

- return HFSPLUS_INVALID_ATTR_RECORD;
+ // Return the size of the record, considering the padding applied to the raw bytes.
+ return offsetof(struct hfsplus_attr_inline_data, raw_bytes) + len;
+ } else {
+ // If the record type is invalid, clear the entry and return the invalid record constant.
+ memset(entry, 0, sizeof(*entry));
+ return HFSPLUS_INVALID_ATTR_RECORD;
+ }
}

+/*
+ * Find an attribute by its CNID and name in the attribute tree.
+ * This function searches for an attribute based on the CNID and name provided.
+ * If the name is NULL, it searches for the first attribute matching the CNID.
+ *
+ * sb: The superblock containing the filesystem.
+ * cnid: The CNID of the entry to search for.
+ * name: The name of the attribute to search for (optional).
+ * fd: The structure to store the search results.
+ *
+ * Returns: 0 on success, or an error code on failure.
+ */
int hfsplus_find_attr(struct super_block *sb, u32 cnid,
- const char *name, struct hfs_find_data *fd)
+ const char *name, struct hfs_find_data *fd)
{
int err = 0;

+ // Debugging output: log the search for the attribute.
hfs_dbg(ATTR_MOD, "find_attr: %s,%d\n", name ? name : NULL, cnid);

+ // If the attribute tree does not exist, log an error and return failure.
if (!HFSPLUS_SB(sb)->attr_tree) {
pr_err("attributes file doesn't exist\n");
return -EINVAL;
}

+ // Search by name if it is provided.
if (name) {
+ // Build the search key based on the CNID and the name.
err = hfsplus_attr_build_key(sb, fd->search_key, cnid, name);
if (err)
goto failed_find_attr;
+
+ // Perform the search by key in the attribute record tree.
err = hfs_brec_find(fd, hfs_find_rec_by_key);
if (err)
goto failed_find_attr;
} else {
+ // If no name is provided, search by CNID alone.
err = hfsplus_attr_build_key(sb, fd->search_key, cnid, NULL);
if (err)
goto failed_find_attr;
+
+ // Perform the search for the first record matching the CNID.
err = hfs_brec_find(fd, hfs_find_1st_rec_by_cnid);
if (err)
goto failed_find_attr;
}

-failed_find_attr:
- return err;
+ failed_find_attr:
+ return err; // Return the result of the attribute search (0 for success, error code on failure).
}

int hfsplus_attr_exists(struct inode *inode, const char *name)
diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
index 901e83d65d20..4d55e4c142ad 100644
--- a/fs/hfsplus/bfind.c
+++ b/fs/hfsplus/bfind.c
@@ -6,49 +6,70 @@
* Brad Boyer (flar@xxxxxxxxxxxxx)
* (C) 2003 Ardis Technologies <roman@xxxxxxxxxxxxx>
*
- * Search routines for btrees
+ * Search routines for btrees in HFS Plus filesystem.
+ *
+ * Pranjal Prasad <prasadpranjal213@xxxxxxxxx>
+ * Improved and added comments
*/

#include <linux/slab.h>
#include "hfsplus_fs.h"

+// Initializes the find data structure, allocating memory for the search key.
int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd)
{
void *ptr;

+ // Initialize tree and bnode pointers
fd->tree = tree;
fd->bnode = NULL;
+
+ // Allocate memory for search key and associated structures.
ptr = kmalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);
if (!ptr)
- return -ENOMEM;
- fd->search_key = ptr;
+ return -ENOMEM; // Return memory allocation error if failed.
+
+ // Set up pointers for the search key and key structure
+ fd->search_key = ptr;
fd->key = ptr + tree->max_key_len + 2;
+
+ // Log the initialization of the find operation
hfs_dbg(BNODE_REFS, "find_init: %d (%p)\n",
tree->cnid, __builtin_return_address(0));
- mutex_lock_nested(&tree->tree_lock,
- hfsplus_btree_lock_class(tree));
+
+ // Lock the btree structure to prevent concurrent modifications
+ mutex_lock_nested(&tree->tree_lock, hfsplus_btree_lock_class(tree));
+
return 0;
}

+// Frees resources allocated during the search operation.
void hfs_find_exit(struct hfs_find_data *fd)
{
+ // Release the bnode and free the allocated memory for search key.
hfs_bnode_put(fd->bnode);
kfree(fd->search_key);
+
+ // Log the exit from the find operation
hfs_dbg(BNODE_REFS, "find_exit: %d (%p)\n",
fd->tree->cnid, __builtin_return_address(0));
+
+ // Unlock the btree structure and set tree pointer to NULL
mutex_unlock(&fd->tree->tree_lock);
fd->tree = NULL;
}

+// Helper function to find the first record in a bnode matching the given CNID.
int hfs_find_1st_rec_by_cnid(struct hfs_bnode *bnode,
- struct hfs_find_data *fd,
- int *begin,
- int *end,
- int *cur_rec)
+ struct hfs_find_data *fd,
+ int *begin,
+ int *end,
+ int *cur_rec)
{
__be32 cur_cnid;
__be32 search_cnid;

+ // Check the CNID based on the btree type (ext, cat, or attr)
if (bnode->tree->cnid == HFSPLUS_EXT_CNID) {
cur_cnid = fd->key->ext.cnid;
search_cnid = fd->search_key->ext.cnid;
@@ -59,97 +80,107 @@ int hfs_find_1st_rec_by_cnid(struct hfs_bnode *bnode,
cur_cnid = fd->key->attr.cnid;
search_cnid = fd->search_key->attr.cnid;
} else {
- cur_cnid = 0; /* used-uninitialized warning */
+ cur_cnid = 0; // Used-uninitialized warning
search_cnid = 0;
- BUG();
+ BUG(); // Trigger a bug if CNID type is invalid.
}

+ // If the current CNID matches the search CNID, set the range and return.
if (cur_cnid == search_cnid) {
(*end) = (*cur_rec);
if ((*begin) == (*end))
- return 1;
+ return 1; // Found exact match.
} else {
+ // Adjust the range based on the comparison of CNIDs.
if (be32_to_cpu(cur_cnid) < be32_to_cpu(search_cnid))
(*begin) = (*cur_rec) + 1;
else
(*end) = (*cur_rec) - 1;
}

- return 0;
+ return 0; // Continue searching.
}

+// Helper function to compare keys and find the record matching the search key.
int hfs_find_rec_by_key(struct hfs_bnode *bnode,
- struct hfs_find_data *fd,
- int *begin,
- int *end,
- int *cur_rec)
+ struct hfs_find_data *fd,
+ int *begin,
+ int *end,
+ int *cur_rec)
{
int cmpval;

+ // Compare the keys using the btree's key comparison function.
cmpval = bnode->tree->keycmp(fd->key, fd->search_key);
if (!cmpval) {
(*end) = (*cur_rec);
- return 1;
+ return 1; // Found exact match.
}
+
+ // Adjust the search range based on the comparison result.
if (cmpval < 0)
(*begin) = (*cur_rec) + 1;
else
*(end) = (*cur_rec) - 1;

- return 0;
+ return 0; // Continue searching.
}

-/* Find the record in bnode that best matches key (not greater than...)*/
+// Function to find the best matching record in a bnode using binary search.
int __hfs_brec_find(struct hfs_bnode *bnode, struct hfs_find_data *fd,
- search_strategy_t rec_found)
+ search_strategy_t rec_found)
{
u16 off, len, keylen;
int rec;
int b, e;
int res;

- BUG_ON(!rec_found);
+ BUG_ON(!rec_found); // Ensure the strategy function is provided.
b = 0;
e = bnode->num_recs - 1;
- res = -ENOENT;
+ res = -ENOENT; // Default to record not found.
+
do {
- rec = (e + b) / 2;
+ rec = (e + b) / 2; // Binary search: Find the midpoint record.
len = hfs_brec_lenoff(bnode, rec, &off);
keylen = hfs_brec_keylen(bnode, rec);
if (keylen == 0) {
- res = -EINVAL;
+ res = -EINVAL; // Invalid record length.
goto fail;
}
hfs_bnode_read(bnode, fd->key, off, keylen);
+
+ // Check if the current record matches the search criteria.
if (rec_found(bnode, fd, &b, &e, &rec)) {
- res = 0;
+ res = 0; // Record found.
goto done;
}
- } while (b <= e);
+ } while (b <= e); // Continue binary search while the range is valid.

+ // If no exact match found, read the last record.
if (rec != e && e >= 0) {
len = hfs_brec_lenoff(bnode, e, &off);
keylen = hfs_brec_keylen(bnode, e);
if (keylen == 0) {
- res = -EINVAL;
+ res = -EINVAL; // Invalid record length.
goto fail;
}
hfs_bnode_read(bnode, fd->key, off, keylen);
}

-done:
+ done:
+ // Populate the find data structure with the found record details.
fd->record = e;
fd->keyoffset = off;
fd->keylength = keylen;
fd->entryoffset = off + keylen;
fd->entrylength = len - keylen;

-fail:
- return res;
+ fail:
+ return res; // Return the result (0 if found, error code otherwise).
}

-/* Traverse a B*Tree from the root to a leaf finding best fit to key */
-/* Return allocated copy of node found, set recnum to best record */
+// Traverse the B*Tree from the root to a leaf, finding the best match for the key.
int hfs_brec_find(struct hfs_find_data *fd, search_strategy_t do_key_compare)
{
struct hfs_btree *tree;
@@ -160,63 +191,77 @@ int hfs_brec_find(struct hfs_find_data *fd, search_strategy_t do_key_compare)

tree = fd->tree;
if (fd->bnode)
- hfs_bnode_put(fd->bnode);
- fd->bnode = NULL;
- nidx = tree->root;
+ hfs_bnode_put(fd->bnode); // Release any previous bnode.
+ fd->bnode = NULL;
+ nidx = tree->root; // Start with the root node.
+
if (!nidx)
- return -ENOENT;
- height = tree->depth;
+ return -ENOENT; // Return error if root node is invalid.
+
+ height = tree->depth;
res = 0;
parent = 0;
+
for (;;) {
+ // Find the bnode corresponding to the current index.
bnode = hfs_bnode_find(tree, nidx);
if (IS_ERR(bnode)) {
- res = PTR_ERR(bnode);
+ res = PTR_ERR(bnode); // Propagate error if node is invalid.
bnode = NULL;
break;
}
+
+ // Validate node height and type (should match the current depth and type).
if (bnode->height != height)
goto invalid;
if (bnode->type != (--height ? HFS_NODE_INDEX : HFS_NODE_LEAF))
goto invalid;
- bnode->parent = parent;

+ // Set the parent node index and perform the search within the node.
+ bnode->parent = parent;
res = __hfs_brec_find(bnode, fd, do_key_compare);
- if (!height)
+ if (!height) // Exit if we've reached the leaf level.
break;
- if (fd->record < 0)
+ if (fd->record < 0) // Handle invalid record.
goto release;

+ // Prepare for the next level of traversal.
parent = nidx;
hfs_bnode_read(bnode, &data, fd->entryoffset, 4);
nidx = be32_to_cpu(data);
hfs_bnode_put(bnode);
}
fd->bnode = bnode;
- return res;
+ return res; // Return the result of the search.

-invalid:
+ invalid:
pr_err("inconsistency in B*Tree (%d,%d,%d,%u,%u)\n",
- height, bnode->height, bnode->type, nidx, parent);
- res = -EIO;
-release:
- hfs_bnode_put(bnode);
+ height, bnode->height, bnode->type, nidx, parent);
+ res = -EIO; // Return I/O error for tree inconsistency.
+
+ release:
+ hfs_bnode_put(bnode); // Release the bnode.
return res;
}

+// Reads a record from the bnode based on the search data.
int hfs_brec_read(struct hfs_find_data *fd, void *rec, int rec_len)
{
int res;

+ // Find the record using the key comparison strategy.
res = hfs_brec_find(fd, hfs_find_rec_by_key);
if (res)
return res;
if (fd->entrylength > rec_len)
- return -EINVAL;
- hfs_bnode_read(fd->bnode, rec, fd->entryoffset, fd->entrylength);
+ return -EINVAL; // Return error if record is larger than buffer.
+
+ // Read the record from the bnode into the provided buffer.
+ hfs_bnode_read(fd->bnode, rec, fd->entryoffset, fd->entrylength);
return 0;
}

+// Goto a specific record (cnt) in the bnode, adjusting the position as needed.
int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
{
struct hfs_btree *tree;
@@ -227,6 +272,7 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
bnode = fd->bnode;
tree = bnode->tree;

+ // Move backwards if cnt is negative.
if (cnt < 0) {
cnt = -cnt;
while (cnt > fd->record) {
@@ -247,6 +293,7 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
}
fd->record -= cnt;
} else {
+ // Move forwards if cnt is positive.
while (cnt >= bnode->num_recs - fd->record) {
cnt -= bnode->num_recs - fd->record;
fd->record = 0;
@@ -266,18 +313,22 @@ int hfs_brec_goto(struct hfs_find_data *fd, int cnt)
fd->record += cnt;
}

+ // Read the key and entry information for the target record.
len = hfs_brec_lenoff(bnode, fd->record, &off);
keylen = hfs_brec_keylen(bnode, fd->record);
if (keylen == 0) {
res = -EINVAL;
goto out;
}
+
+ // Update the find data with the record's information.
fd->keyoffset = off;
fd->keylength = keylen;
fd->entryoffset = off + keylen;
fd->entrylength = len - keylen;
hfs_bnode_read(bnode, fd->key, off, keylen);
-out:
+
+ out:
fd->bnode = bnode;
return res;
}
diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 9e1732a2b92a..9c0f4667606b 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -7,6 +7,8 @@
* (C) 2003 Ardis Technologies <roman@xxxxxxxxxxxxx>
*
* Handle opening/closing btree
+ * Pranjal Prasad <prasadpranjal213@xxxxxxxxx>
+ * Improved and added comments
*/

#include <linux/slab.h>
@@ -18,15 +20,15 @@

/*
* Initial source code of clump size calculation is gotten
- * from http://opensource.apple.com/tarballs/diskdev_cmds/
+ * from https://github.com/apple-oss-distributions/diskdev_cmds/tags
*/
#define CLUMP_ENTRIES 15

static short clumptbl[CLUMP_ENTRIES * 3] = {
-/*
- * Volume Attributes Catalog Extents
- * Size Clump (MB) Clump (MB) Clump (MB)
- */
+ /*
+ * Volume Attributes Catalog Extents
+ * Size Clump (MB) Clump (MB) Clump (MB)
+ */
/* 1GB */ 4, 4, 4,
/* 2GB */ 6, 6, 4,
/* 4GB */ 8, 8, 4,
@@ -73,7 +75,7 @@ static short clumptbl[CLUMP_ENTRIES * 3] = {
};

u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
- u64 sectors, int file_id)
+ u64 sectors, int file_id)
{
u32 mod = max(node_size, block_size);
u32 clump_size;
@@ -82,15 +84,15 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,

/* Figure out which column of the above table to use for this file. */
switch (file_id) {
- case HFSPLUS_ATTR_CNID:
- column = 0;
- break;
- case HFSPLUS_CAT_CNID:
- column = 1;
- break;
- default:
- column = 2;
- break;
+ case HFSPLUS_ATTR_CNID:
+ column = 0;
+ break;
+ case HFSPLUS_CAT_CNID:
+ column = 1;
+ break;
+ default:
+ column = 2;
+ break;
}

/*
@@ -105,7 +107,7 @@ u32 hfsplus_calc_btree_clump_size(u32 block_size, u32 node_size,
/* turn exponent into table index... */
for (i = 0, sectors = sectors >> 22;
sectors && (i < CLUMP_ENTRIES - 1);
- ++i, sectors = sectors >> 1) {
+ ++i, sectors = sectors >> 1) {
/* empty body */
}

@@ -164,7 +166,7 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)

/* Load the header */
head = (struct hfs_btree_header_rec *)(kmap_local_page(page) +
- sizeof(struct hfs_bnode_desc));
+ sizeof(struct hfs_bnode_desc));
tree->root = be32_to_cpu(head->root);
tree->leaf_count = be32_to_cpu(head->leaf_count);
tree->leaf_head = be32_to_cpu(head->leaf_head);
@@ -176,84 +178,119 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
tree->max_key_len = be16_to_cpu(head->max_key_len);
tree->depth = be16_to_cpu(head->depth);

- /* Verify the tree and set the correct compare function */
+ /*
+ * Verify the B-tree structure and set the correct comparison function based on tree type (extent, catalog, or attributes).
+ * This section validates the B-tree properties such as the key length and flags, ensuring that they are consistent
+ * with the expected values for the respective B-tree type. It then sets the correct comparison function for key comparison.
+ */
switch (id) {
- case HFSPLUS_EXT_CNID:
- if (tree->max_key_len != HFSPLUS_EXT_KEYLEN - sizeof(u16)) {
- pr_err("invalid extent max_key_len %d\n",
- tree->max_key_len);
- goto fail_page;
- }
- if (tree->attributes & HFS_TREE_VARIDXKEYS) {
- pr_err("invalid extent btree flag\n");
- goto fail_page;
- }
+ case HFSPLUS_EXT_CNID:
+ /* For Extents B-tree:
+ * Ensure that the max key length is correct. The key length must match the expected length minus the size of a 16-bit value.
+ * If the key length is incorrect, log an error and fail the operation.
+ */
+ if (tree->max_key_len != HFSPLUS_EXT_KEYLEN - sizeof(u16)) {
+ pr_err("invalid extent max_key_len %d\n", tree->max_key_len);
+ goto fail_page; // Jump to failure handling if the key length is invalid.
+ }

- tree->keycmp = hfsplus_ext_cmp_key;
- break;
- case HFSPLUS_CAT_CNID:
- if (tree->max_key_len != HFSPLUS_CAT_KEYLEN - sizeof(u16)) {
- pr_err("invalid catalog max_key_len %d\n",
- tree->max_key_len);
- goto fail_page;
- }
- if (!(tree->attributes & HFS_TREE_VARIDXKEYS)) {
- pr_err("invalid catalog btree flag\n");
- goto fail_page;
- }
+ /* Check that the btree does not have the 'variable index keys' attribute, as it is not valid for Extents B-tree. */
+ if (tree->attributes & HFS_TREE_VARIDXKEYS) {
+ pr_err("invalid extent btree flag\n");
+ goto fail_page; // Jump to failure handling if the flag is invalid.
+ }

- if (test_bit(HFSPLUS_SB_HFSX, &HFSPLUS_SB(sb)->flags) &&
- (head->key_type == HFSPLUS_KEY_BINARY))
- tree->keycmp = hfsplus_cat_bin_cmp_key;
- else {
- tree->keycmp = hfsplus_cat_case_cmp_key;
- set_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags);
- }
- break;
- case HFSPLUS_ATTR_CNID:
- if (tree->max_key_len != HFSPLUS_ATTR_KEYLEN - sizeof(u16)) {
- pr_err("invalid attributes max_key_len %d\n",
- tree->max_key_len);
- goto fail_page;
- }
- tree->keycmp = hfsplus_attr_bin_cmp_key;
- break;
- default:
- pr_err("unknown B*Tree requested\n");
- goto fail_page;
+ /* Set the appropriate comparison function for extent keys. */
+ tree->keycmp = hfsplus_ext_cmp_key;
+ break;
+
+ case HFSPLUS_CAT_CNID:
+ /* For Catalog B-tree:
+ * Ensure the max key length is correct. Catalog keys must match a fixed length (HFSPLUS_CAT_KEYLEN - sizeof(u16)).
+ */
+ if (tree->max_key_len != HFSPLUS_CAT_KEYLEN - sizeof(u16)) {
+ pr_err("invalid catalog max_key_len %d\n", tree->max_key_len);
+ goto fail_page; // Jump to failure handling if the key length is invalid.
+ }
+
+ /* Ensure that the Catalog B-tree has the 'variable index keys' flag set. This is required for Catalogs. */
+ if (!(tree->attributes & HFS_TREE_VARIDXKEYS)) {
+ pr_err("invalid catalog btree flag\n");
+ goto fail_page; // Jump to failure handling if the flag is missing.
+ }
+
+ /* If the HFSX flag is set and the key type is binary, use the binary key comparison function. Otherwise, use case-insensitive comparison. */
+ if (test_bit(HFSPLUS_SB_HFSX, &HFSPLUS_SB(sb)->flags) &&
+ (head->key_type == HFSPLUS_KEY_BINARY)) {
+ tree->keycmp = hfsplus_cat_bin_cmp_key; // Binary key comparison for HFSX volumes.
+ } else {
+ /* For non-HFSX or case-sensitive volumes, use case-insensitive comparison. */
+ tree->keycmp = hfsplus_cat_case_cmp_key;
+ set_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags); // Set flag for case-insensitive comparison.
+ }
+ break;
+
+ case HFSPLUS_ATTR_CNID:
+ /* For Attributes B-tree:
+ * Ensure that the key length matches the expected value (HFSPLUS_ATTR_KEYLEN - sizeof(u16)).
+ */
+ if (tree->max_key_len != HFSPLUS_ATTR_KEYLEN - sizeof(u16)) {
+ pr_err("invalid attributes max_key_len %d\n", tree->max_key_len);
+ goto fail_page; // Jump to failure handling if the key length is invalid.
+ }
+
+ /* Set the appropriate comparison function for attribute keys (binary comparison). */
+ tree->keycmp = hfsplus_attr_bin_cmp_key;
+ break;
+
+ default:
+ /* If the B-tree ID does not match any known types, log an error and fail. */
+ pr_err("unknown B*Tree requested\n");
+ goto fail_page; // Jump to failure handling for unknown B-tree types.
}

+ /*
+ * Perform final validation and setup for the B-tree:
+ * - Ensure the tree has the 'big keys' attribute set (required for proper B-tree functionality).
+ * - Check that the node size is a valid power of 2.
+ * - Ensure that the B-tree has a non-zero node count.
+ */
if (!(tree->attributes & HFS_TREE_BIGKEYS)) {
pr_err("invalid btree flag\n");
- goto fail_page;
+ goto fail_page; // Fail if the 'big keys' flag is missing.
}

+ /* Ensure the node size is a valid power of 2. If not, it's an invalid configuration. */
size = tree->node_size;
if (!is_power_of_2(size))
- goto fail_page;
- if (!tree->node_count)
- goto fail_page;
+ goto fail_page; // Jump to failure handling if node size is not a power of 2.

- tree->node_size_shift = ffs(size) - 1;
+ /* Ensure that the node count is greater than zero. */
+ if (!tree->node_count)
+ goto fail_page; // Jump to failure handling if node count is zero.

- tree->pages_per_bnode =
- (tree->node_size + PAGE_SIZE - 1) >>
- PAGE_SHIFT;
+ tree->node_size_shift = ffs(size) - 1; // Calculate the node size shift based on the node size.

- kunmap_local(head);
- put_page(page);
- return tree;
+ tree->pages_per_bnode = (tree->node_size + PAGE_SIZE - 1) >> PAGE_SHIFT; // Calculate pages per B-node.

- fail_page:
- kunmap_local(head);
- put_page(page);
- free_inode:
- tree->inode->i_mapping->a_ops = &hfsplus_aops;
- iput(tree->inode);
- free_tree:
- kfree(tree);
- return NULL;
-}
+ kunmap_local(head); // Unmap the page after processing.
+ put_page(page); // Release the page.
+
+ return tree; // Return the successfully opened B-tree.
+
+ /* Failure handling section: Clean up resources in case of an error. */
+ fail_page:
+ kunmap_local(head); // Ensure we unmap the page in case of failure.
+ put_page(page); // Release the page.
+
+ free_inode:
+ tree->inode->i_mapping->a_ops = &hfsplus_aops; // Restore the original address space operations.
+ iput(tree->inode); // Release the inode associated with this B-tree.
+
+ free_tree:
+ kfree(tree); // Free the B-tree structure.
+
+ return NULL; // Return NULL to indicate failure.

/* Release resources used by a btree */
void hfs_btree_close(struct hfs_btree *tree)
@@ -269,11 +306,11 @@ void hfs_btree_close(struct hfs_btree *tree)
tree->node_hash[i] = node->next_hash;
if (atomic_read(&node->refcnt))
pr_crit("node %d:%d "
- "still has %d user(s)!\n",
+ "still has %d user(s)!\n",
node->tree->cnid, node->this,
- atomic_read(&node->refcnt));
- hfs_bnode_free(node);
- tree->node_hash_cnt--;
+ atomic_read(&node->refcnt));
+ hfs_bnode_free(node);
+ tree->node_hash_cnt--;
}
}
iput(tree->inode);
@@ -293,7 +330,7 @@ int hfs_btree_write(struct hfs_btree *tree)
/* Load the header */
page = node->page[0];
head = (struct hfs_btree_header_rec *)(kmap_local_page(page) +
- sizeof(struct hfs_bnode_desc));
+ sizeof(struct hfs_bnode_desc));

head->root = cpu_to_be32(tree->root);
head->leaf_count = cpu_to_be32(tree->leaf_count);
@@ -359,10 +396,10 @@ int hfs_bmap_reserve(struct hfs_btree *tree, int rsvd_nodes)
if (res)
return res;
hip->phys_size = inode->i_size =
- (loff_t)hip->alloc_blocks <<
- HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
+ (loff_t)hip->alloc_blocks <<
+ HFSPLUS_SB(tree->sb)->alloc_blksz_shift;
hip->fs_blocks =
- hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
+ hip->alloc_blocks << HFSPLUS_SB(tree->sb)->fs_shift;
inode_set_bytes(inode, inode->i_size);
count = inode->i_size >> tree->node_size_shift;
tree->free_nodes += count - tree->node_count;
@@ -413,7 +450,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
mark_inode_dirty(tree->inode);
hfs_bnode_put(node);
return hfs_bnode_create(tree,
- idx);
+ idx);
}
}
}
@@ -470,8 +507,8 @@ void hfs_bmap_free(struct hfs_bnode *node)
if (!i) {
/* panic */;
pr_crit("unable to free bnode %u. "
- "bmap not found!\n",
- node->this);
+ "bmap not found!\n",
+ node->this);
hfs_bnode_put(node);
return;
}
@@ -482,7 +519,7 @@ void hfs_bmap_free(struct hfs_bnode *node)
if (node->type != HFS_NODE_MAP) {
/* panic */;
pr_crit("invalid bmap found! "
- "(%u,%d)\n",
+ "(%u,%d)\n",
node->this, node->type);
hfs_bnode_put(node);
return;
@@ -497,7 +534,7 @@ void hfs_bmap_free(struct hfs_bnode *node)
byte = data[off];
if (!(byte & m)) {
pr_crit("trying to free free bnode "
- "%u(%d)\n",
+ "%u(%d)\n",
node->this, node->type);
kunmap_local(data);
hfs_bnode_put(node);
diff --git a/fs/hfsplus/xattr_security.c b/fs/hfsplus/xattr_security.c
index 90f68ec119cd..db39a8632b2c 100644
--- a/fs/hfsplus/xattr_security.c
+++ b/fs/hfsplus/xattr_security.c
@@ -17,9 +17,13 @@ static int hfsplus_security_getxattr(const struct xattr_handler *handler,
struct dentry *unused, struct inode *inode,
const char *name, void *buffer, size_t size)
{
+ /* Ensure valid parameters */
+ if (!name || !buffer)
+ return -EINVAL;
+
return hfsplus_getxattr(inode, name, buffer, size,
XATTR_SECURITY_PREFIX,
- XATTR_SECURITY_PREFIX_LEN);
+ XATTR_SECURITY_PREFIX_LEN);
}

static int hfsplus_security_setxattr(const struct xattr_handler *handler,
@@ -28,48 +32,62 @@ static int hfsplus_security_setxattr(const struct xattr_handler *handler,
const char *name, const void *buffer,
size_t size, int flags)
{
+ /* Ensure valid parameters */
+ if (!name || !buffer || size == 0)
+ return -EINVAL;
+
return hfsplus_setxattr(inode, name, buffer, size, flags,
XATTR_SECURITY_PREFIX,
- XATTR_SECURITY_PREFIX_LEN);
+ XATTR_SECURITY_PREFIX_LEN);
}

static int hfsplus_initxattrs(struct inode *inode,
- const struct xattr *xattr_array,
- void *fs_info)
+ const struct xattr *xattr_array,
+ void *fs_info)
{
const struct xattr *xattr;
char *xattr_name;
int err = 0;

- xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1,
- GFP_KERNEL);
+ /* Allocate buffer for xattr name */
+ xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1, GFP_KERNEL);
if (!xattr_name)
return -ENOMEM;
- for (xattr = xattr_array; xattr->name != NULL; xattr++) {

- if (!strcmp(xattr->name, ""))
+ /* Iterate over the provided xattrs */
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ /* Skip if xattr name is empty or NULL */
+ if (!xattr->name || strcmp(xattr->name, "") == 0)
continue;

- strcpy(xattr_name, XATTR_SECURITY_PREFIX);
- strcpy(xattr_name +
- XATTR_SECURITY_PREFIX_LEN, xattr->name);
- memset(xattr_name +
- XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name), 0, 1);
+ /* Copy the security prefix and xattr name into xattr_name */
+ strncpy(xattr_name, XATTR_SECURITY_PREFIX, NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN);
+ strncpy(xattr_name + XATTR_SECURITY_PREFIX_LEN, xattr->name, NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN);

+ /* Ensure null-termination of the name */
+ xattr_name[XATTR_SECURITY_PREFIX_LEN + strlen(xattr->name)] = '\0';
+
+ /* Set the xattr on the inode */
err = __hfsplus_setxattr(inode, xattr_name,
- xattr->value, xattr->value_len, 0);
+ xattr->value, xattr->value_len, 0);
if (err)
break;
}
+
+ /* Free allocated memory */
kfree(xattr_name);
return err;
}

int hfsplus_init_security(struct inode *inode, struct inode *dir,
- const struct qstr *qstr)
+ const struct qstr *qstr)
{
+ /* Ensure valid parameters */
+ if (!inode || !dir || !qstr)
+ return -EINVAL;
+
return security_inode_init_security(inode, dir, qstr,
- &hfsplus_initxattrs, NULL);
+ &hfsplus_initxattrs, NULL);
}

const struct xattr_handler hfsplus_xattr_security_handler = {
--
2.48.1