Skip to contentSkip to conversation

fsverity: explicitly check for buffer overflow in build_merkle_tree()

PATCH New Eric Biggers 1 participant 1 message Mar 28, 2023

Subsystems (2)
VFS fs-verity: read-only file-based authenticity protection
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

PATCH 1/1 fsverity: explicitly check for buffer overflow in build_merkle_tree()

From: Eric Biggers <[email protected]>

The new Merkle tree construction algorithm is a bit fragile in that it may overflow the 'root_hash' array if the tree actually generated does not match the calculated tree parameters.

This should never happen unless there is a filesystem bug that allows the file size to change despite deny_write_access(), or a bug in the Merkle tree logic itself. Regardless, it's fairly easy to check for buffer overflow here, so let's do so.

This is a robustness improvement only; this case is not currently known to be reachable. I've added a Fixes tag anyway, since I recommend that this be included in kernels that have the mentioned commit.

Fixes 56124d6c87fd ("fsverity: support enabling with tree block size < PAGE_SIZE")
Signed-off-by Eric Biggers <[email protected]>
---
 fs/verity/enable.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
fs/verity/enable.c +10 -0
index 7a0e3a84d370b..30012e734a77a 100644
@@ -13,6 +13,7 @@
 
 struct block_buffer {
 	u32 filled;
+	bool is_root_hash;
 	u8 *data;
 };
 

@@ -24,6 +25,14 @@ static int hash_one_block(struct inode *inode,
 	struct block_buffer *next = cur + 1;
 	int err;
 
+	/*
+	 * Safety check to prevent a buffer overflow in case of a filesystem bug
+	 * that allows the file size to change despite deny_write_access(), or a
+	 * bug in the Merkle tree logic itself
+	 */
+	if (WARN_ON_ONCE(next->is_root_hash && next->filled != 0))
+		return -EINVAL;
+
 	/* Zero-pad the block if it's shorter than the block size. */
 	memset(&cur->data[cur->filled], 0, params->block_size - cur->filled);
 

@@ -97,6 +106,7 @@ static int build_merkle_tree(struct file *filp,
 		}
 	}
 	buffers[num_levels].data = root_hash;
+	buffers[num_levels].is_root_hash = true;
 
 	BUILD_BUG_ON(sizeof(level_offset) != sizeof(params->level_start));
 	memcpy(level_offset, params->level_start, sizeof(level_offset));

base-commit: 2da81b8479434c62a9ae189ec24729445f74b6a9

Signature · 1 line
2.40.0