usb: gadget: configfs: fix OOB read in ext_prop_data_show()
Subsystems (2)
PATCH v2 1/1 usb: gadget: configfs: fix 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 set to len*2+2 to account for the UTF-16 encoding and a 2-byte null terminator, as required by the Microsoft OS Extended Properties Descriptor specification (dwPropertyDataLength must include the terminator).
However, the null terminator is never 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+2 bytes and explicitly zero-terminating with a full 2-byte UTF-16 null terminator. This ensures the buffer fully matches the dwPropertyDataLength semantics (len*2+2) while eliminating the OOB read.
--- drivers/usb/gadget/configfs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/usb/gadget/configfs.c +4 -1
@@ -1352,8 +1352,12 @@ 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 +2 ,GFP_KERNEL );if (!new_data)return -ENOMEM ; +memcpy (new_data, page, len); + new_data[len] ='\0' ; + new_data[len +1 ] ='\0' ;if (desc->opts_mutex)
--
2.43.0
Greg Kroah-Hartman
maintainerQuoted 57 lines · Bingquan Chen
In
ext_prop_data_store(), for unicode property types, the data buffer is allocated viakmemdup()with size 'len', butdata_lenis set to len*2+2 to account for the UTF-16 encoding and a 2-byte null terminator, as required by the Microsoft OS Extended Properties Descriptor specification (dwPropertyDataLength must include the terminator).However, the null terminator is never actually stored in the data buffer. When
ext_prop_data_show()reads the data back, it computes the read length asdata_len>> 1 = len+1, then doesmemcpy(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 8Fix by allocating len+2 bytes and explicitly zero-terminating with a full 2-byte UTF-16 null terminator. This ensures the buffer fully matches the dwPropertyDataLength semantics (len*2+2) while eliminating the OOB read.
Fixes 7419485f197c ("usb: gadget: configfs: OS Extended Properties descriptors support")Signed-off-by Bingquan Chen <[email protected]>--- drivers/usb/gadget/configfs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/usb/gadget/configfs.c +4
-2@@ -1352,8 +1352,12 @@ 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 +2 ,GFP_KERNEL );if (!new_data)return -ENOMEM ; +memcpy (new_data, page, len); + new_data[len] ='\0' ; + new_data[len +1 ] ='\0' ;if (desc->opts_mutex) --
Quoted 1 line
2.43.0
This patch is corrupted and can not be applied :(