[PATCH v2] firewire: core: validate descriptor bounds in fw_core_add_descriptor()

Sreeraj S Kurup posted 1 patch 11 hours ago
drivers/firewire/core-card.c | 57 +++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 23 deletions(-)
[PATCH v2] firewire: core: validate descriptor bounds in fw_core_add_descriptor()
Posted by Sreeraj S Kurup 11 hours ago
In fw_core_add_descriptor(), incoming descriptor data is processed without
prior bounds verification of individual sub-block headers. A malformed or
corrupted descriptor containing an oversized block length field can cause
the parsing loop to read past the end of the desc->data buffer, leading to
an out-of-bounds memory access.

Add validation logic at the start of fw_core_add_descriptor() to:
1. Reject empty descriptors (length == 0) or descriptors exceeding the
   maximum configuration ROM size (256 quadlets).
2. Validate each sub-block header's embedded length against the remaining
   descriptor length before advancing the loop pointer.
3. Ensure the inner loop explicitly increments by (block_len + 1) quadlets
   to prevent hangs or buffer overflows.

Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
---
v2:
 - Fixed tab indentation to 8-space tabs per kernel coding style.
 - Added explicit loop increment (i += block_len + 1) to prevent kernel hangs.
 - Isolated changes strictly to drivers/firewire/core-card.c.
 - Added descriptor bounds validation in fw_core_add_descriptor().

 drivers/firewire/core-card.c | 57 +++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 97439da6f480..cb8ce491fe9d 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -169,36 +169,47 @@ static size_t required_space(struct fw_descriptor *desc)
 
 int fw_core_add_descriptor(struct fw_descriptor *desc)
 {
-    size_t i;
+	size_t i;
 
-    /* Extra validation: reject empty or oversized descriptors */
-    if (desc->length == 0 || desc->length > 256)
-        return -EINVAL;
+	/* Reject empty or oversized descriptors early */
+	if (desc->length == 0 || desc->length > 256)
+		return -EINVAL;
+	i = 0;
+	/*
+	 * Validate internal block structures within the descriptor. Each sub-block
+	 * encodes its length in the top 16 bits of its header quadlet.
+	 */
+	while (i < desc->length) {
+		u16 block_len = desc->data[i] >> 16;
+
+		/*
+		 * Guard against corrupted descriptors where an individual block length
+		 * claims to extend past the allocated end of desc->data, avoiding
+		 * out-of-bounds reads.
+		 */
+		if (block_len >= desc->length - i)
+			return -EINVAL;
 
-    /*
-     * Check descriptor is valid, the length of all blocks in the
-     * descriptor has to add up to exactly the length of the block.
-     */
-    i = 0;
-    while (i < desc->length)
-        i += (desc->data[i] >> 16) + 1;
+		i += block_len + 1;
+	}
 
-    if (i != desc->length)
-        return -EINVAL;
+	/* The sum of sub-block lengths must match total descriptor length */
+	if (i != desc->length)
+		return -EINVAL;
 
-    guard(mutex)(&card_mutex);
+	guard(mutex)(&card_mutex);
 
-    if (config_rom_length + required_space(desc) > 256)
-        return -EBUSY;
+	if (config_rom_length + required_space(desc) > 256)
+		return -EBUSY;
 
-    list_add_tail(&desc->link, &descriptor_list);
-    config_rom_length += required_space(desc);
-    descriptor_count++;
-    if (desc->immediate > 0)
-        descriptor_count++;
-    update_config_roms();
+	list_add_tail(&desc->link, &descriptor_list);
+	config_rom_length += required_space(desc);
+	descriptor_count++;
+	if (desc->immediate > 0)
+		descriptor_count++;
+	update_config_roms();
 
-    return 0;
+	return 0;
 }
 EXPORT_SYMBOL(fw_core_add_descriptor);
 
-- 
2.54.0