usb: gadget: configfs: fix 1-byte OOB read in ext_prop_data_show()

In ext_prop_data_store(), for unicode property types, the data buffer is allocated via kmemdup() with size 'len', but data_len is inflated to len*2+2 to account for the UTF-16 encoding and a 2-byte null terminator. The null terminator is not actually stored in the data buffer.

When ext_prop_data_show() reads the data back, it computes the read length as data_len >> 1 = len+1, then does memcpy(page, data, len+1), reading 1 byte past the allocated buffer. This is a slab-out-of-bounds read that leaks 1 byte of adjacent heap data to userspace via configfs.

KASAN report (5.10.252):

  BUG: KASAN: slab-out-of-bounds in ext_prop_data_show+0x4a/0x60
  Read of size 9 at addr ffff888005546008 by task poc/62

  Allocated by task 62:
   kmemdup+0x17/0x40
   ext_prop_data_store+0x52/0x130
   configfs_write_file+0x168/0x200

  The buggy address belongs to the object at ffff888005546008
   which belongs to the cache kmalloc-8 of size 8

Fix by allocating len+1 bytes and null-terminating the buffer, so the extra byte read in show() returns a known-zero byte instead of adjacent slab data.

---
 drivers/usb/gadget/configfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 183a25f65ac8..a1b2c3d4e5f6 100644
@@ -1352,8 +1352,11 @@ static ssize_t ext_prop_data_store(struct config_item *item,
 
 	if (page[len - 1] == '\n' || page[len - 1] == '\0')
 		--len;
-	new_data = kmemdup(page, len, GFP_KERNEL);
+	new_data = kmalloc(len + 1, GFP_KERNEL);
 	if (!new_data)
 		return -ENOMEM;
+	memcpy(new_data, page, len);
+	new_data[len] = '\0';
 
 	if (desc->opts_mutex)

--
2.43.0

Isn't the problem the "+2" in the size calculation? The terminator is never stored nor used.

Kees Cook