Skip to contentSkip to conversation

ext4: fix use-after-free in ext4_search_dir

PATCH Superseded Ye Bin 2 participants 4 messages Mar 23, 2022 ext4 file system

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Ye Bin

author
#0 lore raw

PATCH 1/1 ext4: fix use-after-free in ext4_search_dir

We got issue as follows:
EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
==================================================================

BUG: KASAN: use-after-free in ext4_search_dir fs/ext4/namei.c:1394 [inline]
BUG: KASAN: use-after-free in search_dirblock fs/ext4/namei.c:1199 [inline]
BUG: KASAN: use-after-free in __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
Read of size 1 at addr ffff8881317c3005 by task syz-executor117/2331

CPU: 1 PID: 2331 Comm: syz-executor117 Not tainted 5.10.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:83 [inline]
 dump_stack+0x144/0x187 lib/dump_stack.c:124
 print_address_description+0x7d/0x630 mm/kasan/report.c:387
 __kasan_report+0x132/0x190 mm/kasan/report.c:547
 kasan_report+0x47/0x60 mm/kasan/report.c:564
 ext4_search_dir fs/ext4/namei.c:1394 [inline]
 search_dirblock fs/ext4/namei.c:1199 [inline]
 __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
 ext4_lookup_entry fs/ext4/namei.c:1622 [inline]
 ext4_lookup+0xb8/0x3a0 fs/ext4/namei.c:1690
 __lookup_hash+0xc5/0x190 fs/namei.c:1451
 do_rmdir+0x19e/0x310 fs/namei.c:3760
 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x445e59
Code: 4d c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 1b c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff2277fac8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 0000000000400280 RCX: 0000000000445e59
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200000c0
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000002
R10: 00007fff2277f990 R11: 0000000000000246 R12: 0000000000000000
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the page:
page:0000000048cd3304 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x1317c3
flags: 0x200000000000000()
raw: 0200000000000000 ffffea0004526588 ffffea0004528088 0000000000000000
raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8881317c2f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff8881317c2f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8881317c3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                   ^
 ffff8881317c3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff8881317c3100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

ext4_search_dir:
...
de = (struct ext4_dir_entry_2 *)search_buf;
dlimit = search_buf + buf_size;
while ((char *) de < dlimit) {
...

    if ((char *) de + de->name_len <= dlimit &&
	 ext4_match(dir, fname, de)) {
	    ...
    }
  ...
    de_len = ext4_rec_len_from_disk(de->rec_len, dir->i_sb->s_blocksize);
    if (de_len <= 0)
      return -1;
    offset += de_len;
    de = (struct ext4_dir_entry_2 *) ((char *) de + de_len);

}

Assume:
de=0xffff8881317c2fff
dlimit=0x0xffff8881317c3000

If read 'de->name_len' which address is 0xffff8881317c3005, obviously is out of range, then will trigger use-after-free.
To solve this issue, 'dlimit' must reserve 8 bytes, as we will read 'de->name_len' to judge if '(char *) de + de->name_len' out of range.

---
 fs/ext4/ext4.h  | 4 ++++
 fs/ext4/namei.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)
fs/ext4/ext4.h +4 -0
index 3f87cca49f0c..276683f7ab77 100644
@@ -2273,6 +2273,10 @@ static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
  * Structure of a directory entry
  */
 #define EXT4_NAME_LEN 255
+/*
+ * Base length of ext4_dir_entry_2 and ext4_dir_entry exclude name
+ */
+#define EXT4_BASE_DIR_LEN 8
 
 struct ext4_dir_entry {
 	__le32	inode;			/* Inode number */
fs/ext4/namei.c +1 -1
index e37da8d5cd0c..4739a5aa13aa 100644
@@ -1465,7 +1465,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
 	int de_len;
 
 	de = (struct ext4_dir_entry_2 *)search_buf;
-	dlimit = search_buf + buf_size;
+	dlimit = search_buf + buf_size - EXT4_BASE_DIR_LEN;
 	while ((char *) de < dlimit) {
 		/* this code is executed quadratically often */
 		/* do minimal checking `by hand' */
Signature · 1 line
2.31.1
reviewer
#1 lore raw
Quoted 72 lines · Ye Bin

We got issue as follows:
EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
==================================================================

BUG: KASAN: use-after-free in ext4_search_dir fs/ext4/namei.c:1394 [inline]
BUG: KASAN: use-after-free in search_dirblock fs/ext4/namei.c:1199 [inline]
BUG: KASAN: use-after-free in __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
Read of size 1 at addr ffff8881317c3005 by task syz-executor117/2331

CPU: 1 PID: 2331 Comm: syz-executor117 Not tainted 5.10.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:83 [inline]
 dump_stack+0x144/0x187 lib/dump_stack.c:124
 print_address_description+0x7d/0x630 mm/kasan/report.c:387
 __kasan_report+0x132/0x190 mm/kasan/report.c:547
 kasan_report+0x47/0x60 mm/kasan/report.c:564
 ext4_search_dir fs/ext4/namei.c:1394 [inline]
 search_dirblock fs/ext4/namei.c:1199 [inline]
 __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
 ext4_lookup_entry fs/ext4/namei.c:1622 [inline]
 ext4_lookup+0xb8/0x3a0 fs/ext4/namei.c:1690
 __lookup_hash+0xc5/0x190 fs/namei.c:1451
 do_rmdir+0x19e/0x310 fs/namei.c:3760
 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x445e59
Code: 4d c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 1b c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff2277fac8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 0000000000400280 RCX: 0000000000445e59
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200000c0
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000002
R10: 00007fff2277f990 R11: 0000000000000246 R12: 0000000000000000
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the page:
page:0000000048cd3304 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x1317c3
flags: 0x200000000000000()
raw: 0200000000000000 ffffea0004526588 ffffea0004528088 0000000000000000
raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8881317c2f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff8881317c2f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8881317c3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                   ^
 ffff8881317c3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff8881317c3100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

ext4_search_dir:
...
de = (struct ext4_dir_entry_2 *)search_buf;
dlimit = search_buf + buf_size;
while ((char *) de < dlimit) {
...

    if ((char *) de + de->name_len <= dlimit &&
	 ext4_match(dir, fname, de)) {
	    ...
    }
  ...
    de_len = ext4_rec_len_from_disk(de->rec_len, dir->i_sb->s_blocksize);
    if (de_len <= 0)
      return -1;
    offset += de_len;
    de = (struct ext4_dir_entry_2 *) ((char *) de + de_len);

}

Assume:
de=0xffff8881317c2fff
dlimit=0x0xffff8881317c3000

Quoted 6 lines

If read 'de->name_len' which address is 0xffff8881317c3005, obviously is out of range, then will trigger use-after-free.
To solve this issue, 'dlimit' must reserve 8 bytes, as we will read 'de->name_len' to judge if '(char *) de + de->name_len' out of range.

Oh, good catch.

fs/ext4/ext4.h static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi) · line 2279

Quoted 12 lines
fs/ext4/ext4.h +4 -0
index 3f87cca49f0c..276683f7ab77 100644
@@ -2273,6 +2273,10 @@ static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
  * Structure of a directory entry
  */
 #define EXT4_NAME_LEN 255
+/*
+ * Base length of ext4_dir_entry_2 and ext4_dir_entry exclude name
+ */
+#define EXT4_BASE_DIR_LEN 8

I'd rather use (sizeof(struct ext4_dir_entry_2) - EXT4_NAME_LEN) here...

Quoted 7 lines
fs/ext4/ext4.h +0 -0
 struct ext4_dir_entry {
 	__le32	inode;			/* Inode number */
fs/ext4/namei.c +0 -0
index e37da8d5cd0c..4739a5aa13aa 100644

fs/ext4/namei.c int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size, · line 1471

Quoted 8 lines
fs/ext4/namei.c +1 -1
@@ -1465,7 +1465,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
 	int de_len;
 
 	de = (struct ext4_dir_entry_2 *)search_buf;
-	dlimit = search_buf + buf_size;
+	dlimit = search_buf + buf_size - EXT4_BASE_DIR_LEN;
 	while ((char *) de < dlimit) {
 		/* this code is executed quadratically often */
 		/* do minimal checking `by hand' */

This looks wrong because a bit later we use dlimit to verify de+de->name_len and that can certainly go upto bufsize. You need to modify only the condition in the while loop like:

  	while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
author
#2 lore raw
Quoted 114 lines · Jan Kara
Quoted 72 lines · Ye Bin

We got issue as follows:
EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
==================================================================

BUG: KASAN: use-after-free in ext4_search_dir fs/ext4/namei.c:1394 [inline]
BUG: KASAN: use-after-free in search_dirblock fs/ext4/namei.c:1199 [inline]
BUG: KASAN: use-after-free in __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
Read of size 1 at addr ffff8881317c3005 by task syz-executor117/2331

CPU: 1 PID: 2331 Comm: syz-executor117 Not tainted 5.10.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Call Trace:
  __dump_stack lib/dump_stack.c:83 [inline]
  dump_stack+0x144/0x187 lib/dump_stack.c:124
  print_address_description+0x7d/0x630 mm/kasan/report.c:387
  __kasan_report+0x132/0x190 mm/kasan/report.c:547
  kasan_report+0x47/0x60 mm/kasan/report.c:564
  ext4_search_dir fs/ext4/namei.c:1394 [inline]
  search_dirblock fs/ext4/namei.c:1199 [inline]
  __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
  ext4_lookup_entry fs/ext4/namei.c:1622 [inline]
  ext4_lookup+0xb8/0x3a0 fs/ext4/namei.c:1690
  __lookup_hash+0xc5/0x190 fs/namei.c:1451
  do_rmdir+0x19e/0x310 fs/namei.c:3760
  do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x445e59
Code: 4d c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 1b c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff2277fac8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 0000000000400280 RCX: 0000000000445e59
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200000c0
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000002
R10: 00007fff2277f990 R11: 0000000000000246 R12: 0000000000000000
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the page:
page:0000000048cd3304 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x1317c3
flags: 0x200000000000000()
raw: 0200000000000000 ffffea0004526588 ffffea0004528088 0000000000000000
raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881317c2f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8881317c2f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Quoted 1 line

ffff8881317c3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

                    ^

ffff8881317c3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8881317c3100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

ext4_search_dir:

   ...
   de = (struct ext4_dir_entry_2 *)search_buf;
   dlimit = search_buf + buf_size;
   while ((char *) de < dlimit) {
   ...
     if ((char *) de + de->name_len <= dlimit &&
	 ext4_match(dir, fname, de)) {
	    ...
     }
   ...
     de_len = ext4_rec_len_from_disk(de->rec_len, dir->i_sb->s_blocksize);
     if (de_len <= 0)
       return -1;
     offset += de_len;
     de = (struct ext4_dir_entry_2 *) ((char *) de + de_len);
   }

Assume:
de=0xffff8881317c2fff
dlimit=0x0xffff8881317c3000

Quoted 6 lines

If read 'de->name_len' which address is 0xffff8881317c3005, obviously is out of range, then will trigger use-after-free.
To solve this issue, 'dlimit' must reserve 8 bytes, as we will read 'de->name_len' to judge if '(char *) de + de->name_len' out of range.

Oh, good catch.

fs/ext4/ext4.h static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi) · line 2279

Quoted 12 lines
fs/ext4/ext4.h +4 -0
index 3f87cca49f0c..276683f7ab77 100644
@@ -2273,6 +2273,10 @@ static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
   * Structure of a directory entry
   */
  #define EXT4_NAME_LEN 255
+/*
+ * Base length of ext4_dir_entry_2 and ext4_dir_entry exclude name
+ */
+#define EXT4_BASE_DIR_LEN 8

I'd rather use (sizeof(struct ext4_dir_entry_2) - EXT4_NAME_LEN) here...

Quoted 7 lines
fs/ext4/ext4.h +0 -0
  struct ext4_dir_entry {
  	__le32	inode;			/* Inode number */
fs/ext4/namei.c +0 -0
index e37da8d5cd0c..4739a5aa13aa 100644

fs/ext4/namei.c int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size, · line 1471

Quoted 8 lines
fs/ext4/namei.c +1 -1
@@ -1465,7 +1465,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
  	int de_len;
 
  	de = (struct ext4_dir_entry_2 *)search_buf;
-	dlimit = search_buf + buf_size;
+	dlimit = search_buf + buf_size - EXT4_BASE_DIR_LEN;
  	while ((char *) de < dlimit) {
  		/* this code is executed quadratically often */
  		/* do minimal checking `by hand' */

This looks wrong because a bit later we use dlimit to verify de+de->name_len and that can certainly go upto bufsize. You need to modify only the condition in the while loop like:

Quoted 3 lines
   	while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {

Honza

I think 'dlimit' also need to minus EXT4_BASE_DIR_LEN when verify 'de+de->name_len' . Assume:
de = 0xffff8881317c2ff7
dlimit = 0x0xffff8881317c3000
de->name_len = 8

=>
de + de->name_len = 0xffff8881317c2fff ( <= dlimit=0x0xffff8881317c3000) de->name = 'de' address + EXT4_BASE_DIR_LEN = 0xffff8881317c2ff7 + 8 = 0xffff8881317c2fff If we read 8 bytes form 0xffff8881317c2fff will read out of range.

reviewer
#3 lore raw
Quoted 124 lines · yebin
Quoted 113 lines · Jan Kara
Quoted 72 lines · Ye Bin

We got issue as follows:
EXT4-fs (loop0): mounted filesystem without journal. Opts: ,errors=continue
==================================================================

BUG: KASAN: use-after-free in ext4_search_dir fs/ext4/namei.c:1394 [inline]
BUG: KASAN: use-after-free in search_dirblock fs/ext4/namei.c:1199 [inline]
BUG: KASAN: use-after-free in __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
Read of size 1 at addr ffff8881317c3005 by task syz-executor117/2331

CPU: 1 PID: 2331 Comm: syz-executor117 Not tainted 5.10.0+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
Call Trace:
  __dump_stack lib/dump_stack.c:83 [inline]
  dump_stack+0x144/0x187 lib/dump_stack.c:124
  print_address_description+0x7d/0x630 mm/kasan/report.c:387
  __kasan_report+0x132/0x190 mm/kasan/report.c:547
  kasan_report+0x47/0x60 mm/kasan/report.c:564
  ext4_search_dir fs/ext4/namei.c:1394 [inline]
  search_dirblock fs/ext4/namei.c:1199 [inline]
  __ext4_find_entry+0xdca/0x1210 fs/ext4/namei.c:1553
  ext4_lookup_entry fs/ext4/namei.c:1622 [inline]
  ext4_lookup+0xb8/0x3a0 fs/ext4/namei.c:1690
  __lookup_hash+0xc5/0x190 fs/namei.c:1451
  do_rmdir+0x19e/0x310 fs/namei.c:3760
  do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x445e59
Code: 4d c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 1b c7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff2277fac8 EFLAGS: 00000246 ORIG_RAX: 0000000000000054
RAX: ffffffffffffffda RBX: 0000000000400280 RCX: 0000000000445e59
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200000c0
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000002
R10: 00007fff2277f990 R11: 0000000000000246 R12: 0000000000000000
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the page:
page:0000000048cd3304 refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x1317c3
flags: 0x200000000000000()
raw: 0200000000000000 ffffea0004526588 ffffea0004528088 0000000000000000
raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881317c2f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8881317c2f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Quoted 1 line

ffff8881317c3000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

                    ^

ffff8881317c3080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8881317c3100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

ext4_search_dir:

   ...
   de = (struct ext4_dir_entry_2 *)search_buf;
   dlimit = search_buf + buf_size;
   while ((char *) de < dlimit) {
   ...
     if ((char *) de + de->name_len <= dlimit &&
	 ext4_match(dir, fname, de)) {
	    ...
     }
   ...
     de_len = ext4_rec_len_from_disk(de->rec_len, dir->i_sb->s_blocksize);
     if (de_len <= 0)
       return -1;
     offset += de_len;
     de = (struct ext4_dir_entry_2 *) ((char *) de + de_len);
   }

Assume:
de=0xffff8881317c2fff
dlimit=0x0xffff8881317c3000

Quoted 6 lines

If read 'de->name_len' which address is 0xffff8881317c3005, obviously is out of range, then will trigger use-after-free.
To solve this issue, 'dlimit' must reserve 8 bytes, as we will read 'de->name_len' to judge if '(char *) de + de->name_len' out of range.

Oh, good catch.

fs/ext4/ext4.h static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi) · line 2279

Quoted 12 lines
fs/ext4/ext4.h +4 -0
index 3f87cca49f0c..276683f7ab77 100644
@@ -2273,6 +2273,10 @@ static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
   * Structure of a directory entry
   */
  #define EXT4_NAME_LEN 255
+/*
+ * Base length of ext4_dir_entry_2 and ext4_dir_entry exclude name
+ */
+#define EXT4_BASE_DIR_LEN 8

I'd rather use (sizeof(struct ext4_dir_entry_2) - EXT4_NAME_LEN) here...

Quoted 7 lines
fs/ext4/ext4.h +0 -0
  struct ext4_dir_entry {
  	__le32	inode;			/* Inode number */
fs/ext4/namei.c +0 -0
index e37da8d5cd0c..4739a5aa13aa 100644

fs/ext4/namei.c int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size, · line 1470

Quoted 7 lines
fs/ext4/namei.c +1 -1
@@ -1465,7 +1465,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
  	int de_len;
  	de = (struct ext4_dir_entry_2 *)search_buf;
-	dlimit = search_buf + buf_size;
+	dlimit = search_buf + buf_size - EXT4_BASE_DIR_LEN;
  	while ((char *) de < dlimit) {
  		/* this code is executed quadratically often */
  		/* do minimal checking `by hand' */

This looks wrong because a bit later we use dlimit to verify de+de->name_len and that can certainly go upto bufsize. You need to modify only the condition in the while loop like:

Quoted 3 lines
   	while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {

Honza

I think 'dlimit' also need to minus EXT4_BASE_DIR_LEN when verify 'de+de->name_len' .
Assume:
de = 0xffff8881317c2ff7
dlimit = 0x0xffff8881317c3000
de->name_len = 8

Quoted 5 lines

=>
de + de->name_len = 0xffff8881317c2fff ( <= dlimit=0x0xffff8881317c3000) de->name = 'de' address + EXT4_BASE_DIR_LEN = 0xffff8881317c2ff7 + 8 = 0xffff8881317c2fff
If we read 8 bytes form 0xffff8881317c2fff will read out of range.

Ah, I see. Well, I'd rather modify the condition to look like:

                if (de->name + de->name_len <= dlimit &&
                    ext4_match(dir, fname, de)) {

Because that expresses better what we are checking...

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR