fs/ntfs3: fix slab-out-of-bounds write in ni_create_attr_list()
PATCH 1/1 fs/ntfs3: fix slab-out-of-bounds write in ni_create_attr_list()
From: HE WEI(ギカク) <[email protected]>
ni_create_attr_list() allocates a fixed buffer of al_aligned(record_size) (== record_size) bytes and then walks every attribute of the primary MFT record, writing one ATTR_LIST_ENTRY per attribute and advancing the cursor by le_size(name_len), with no check against the end of the buffer; the total size is only computed after the loop.
A minimum-size resident attribute occupies SIZEOF_RESIDENT (0x18 = 24) bytes on disk, but an unnamed attribute expands to le_size(0) (0x20 = 32) bytes in the list. Because the number of attributes in a record is not bounded (mi_enum_attr() accepts arbitrarily many equal-type, nameless minimum-size attributes), a crafted record packed with such attributes produces a list larger than record_size and overflows the heap buffer.
This is reachable from a crafted, loop-mounted NTFS image: opening the file and adding an attribute (e.g. via setxattr) drives ntfs_set_ea() -> ni_insert_resident() -> ni_insert_attr() -> ni_ins_attr_ext() -> ni_create_attr_list().
BUG: KASAN: slab-out-of-bounds in ni_create_attr_list+0xc48/0x1058 Write of size 4 at addr ffff000008984c00 by task setfattr/345 ni_create_attr_list+0xc48/0x1058 ni_ins_attr_ext+0x510/0x7c0 ni_insert_attr+0x3f8/0x70c ni_insert_resident+0xc8/0x3b0 ntfs_set_ea+0x66c/0xd28 ntfs_setxattr+0x4d8/0x5b0 __arm64_sys_setxattr+0xa4/0x124 Allocated by task 345: ni_create_attr_list+0x188/0x1058 The buggy address belongs to the cache kmalloc-1k of size 1024 (the write lands at object+1024).
Size the buffer from the actual attributes instead of assuming a single record_size is always enough.
--- fs/ntfs3/frecord.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
fs/ntfs3/frecord.c +16 -4
@@ -768,10 +768,23 @@ int ni_create_attr_list(struct ntfs_inode *ni) rs = sbi->record_size;/* -* Skip estimating exact memory requirement. -* Looks like one record_size is always enough. +* Compute the exact size of the attribute list. Each attribute in the +* record yields one ATTR_LIST_ENTRY of le_size(name_len) bytes. The +* minimum on-disk attribute is SIZEOF_RESIDENT (0x18) bytes, but an +* unnamed one expands to le_size(0) (0x20) here, so a record crafted +* with many such attributes needs more than a single record_size; the +* previous fixed kzalloc(record_size) could therefore be overflowed by +* an attacker-controlled record. */ - le =kzalloc (al_aligned (rs),GFP_NOFS ); + lsize =0 ; + attr =NULL ; +while ((attr =mi_enum_attr (ni, &ni->mi, attr))) + lsize +=le_size (attr->name_len); + +if (!lsize) +return -EINVAL ; + + le =kzalloc (al_aligned (lsize),GFP_NOFS );if (!le)return -ENOMEM ;
@@ -781,7 +794,6 @@ int ni_create_attr_list(struct ntfs_inode *ni) attr =NULL ; nb =0 ; free_b =0 ; - attr =NULL ;for (; (attr =mi_enum_attr (ni, &ni->mi, attr)); le =Add2Ptr (le, sz)) { sz =le_size (attr->name_len);
Signature · 1 line
2.43.0
Konstantin Komarov
maintainerQuoted 84 lines · hewei-gikaku
From: HE WEI(ギカク) <[email protected]>
ni_create_attr_list()allocates a fixed buffer ofal_aligned(record_size)(==record_size) bytes and then walks every attribute of the primary MFT record, writing oneATTR_LIST_ENTRYper attribute and advancing the cursor byle_size(name_len), with no check against the end of the buffer; the total size is only computed after the loop.A minimum-size resident attribute occupies
SIZEOF_RESIDENT(0x18= 24) bytes on disk, but an unnamed attribute expands tole_size(0)(0x20= 32) bytes in the list. Because the number of attributes in a record is not bounded (mi_enum_attr()accepts arbitrarily many equal-type, nameless minimum-size attributes), a crafted record packed with such attributes produces a list larger thanrecord_sizeand overflows the heap buffer.This is reachable from a crafted, loop-mounted NTFS image: opening the file and adding an attribute (e.g. via setxattr) drives
ntfs_set_ea()->ni_insert_resident()->ni_insert_attr()->ni_ins_attr_ext()->ni_create_attr_list().BUG: KASAN: slab-out-of-bounds in ni_create_attr_list+0xc48/0x1058 Write of size 4 at addr ffff000008984c00 by task setfattr/345 ni_create_attr_list+0xc48/0x1058 ni_ins_attr_ext+0x510/0x7c0 ni_insert_attr+0x3f8/0x70c ni_insert_resident+0xc8/0x3b0 ntfs_set_ea+0x66c/0xd28 ntfs_setxattr+0x4d8/0x5b0 __arm64_sys_setxattr+0xa4/0x124 Allocated by task 345: ni_create_attr_list+0x188/0x1058 The buggy address belongs to the cache kmalloc-1k of size 1024 (the write lands at object+1024).Size the buffer from the actual attributes instead of assuming a single
record_sizeis always enough.Fixes 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")Reported-by HE WEI(ギカク) <[email protected]>Signed-off-by HE WEI(ギカク) <[email protected]>--- fs/ntfs3/frecord.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)fs/ntfs3/frecord.c +16
-5@@ -768,10 +768,23 @@ int ni_create_attr_list(struct ntfs_inode *ni) rs = sbi->record_size;/* -* Skip estimating exact memory requirement. -* Looks like one record_size is always enough. +* Compute the exact size of the attribute list. Each attribute in the +* record yields one ATTR_LIST_ENTRY of le_size(name_len) bytes. The +* minimum on-disk attribute is SIZEOF_RESIDENT (0x18) bytes, but an +* unnamed one expands to le_size(0) (0x20) here, so a record crafted +* with many such attributes needs more than a single record_size; the +* previous fixed kzalloc(record_size) could therefore be overflowed by +* an attacker-controlled record. */ - le =kzalloc (al_aligned (rs),GFP_NOFS ); + lsize =0 ; + attr =NULL ; +while ((attr =mi_enum_attr (ni, &ni->mi, attr))) + lsize +=le_size (attr->name_len); + +if (!lsize) +return -EINVAL ; + + le =kzalloc (al_aligned (lsize),GFP_NOFS );if (!le)return -ENOMEM ;@@ -781,7 +794,6 @@ int ni_create_attr_list(struct ntfs_inode *ni) attr =NULL ; nb =0 ; free_b =0 ; - attr =NULL ;for (; (attr =mi_enum_attr (ni, &ni->mi, attr)); le =Add2Ptr (le, sz)) { sz =le_size (attr->name_len); --
Quoted 1 line
2.43.0
Hello,
Very sorry for the delay.
The patch was applied, thank you.
Regards,
Konstantin