Bug 199185 - Invalid pointer dereference in get_acl (fs/posix_acl.c) when mounting and operating crafted ext4 image
Summary: Invalid pointer dereference in get_acl (fs/posix_acl.c) when mounting and ope...
Status: RESOLVED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: ext4 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: fs_ext4@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-22 21:06 UTC by Wen Xu
Modified: 2018-03-29 21:51 UTC (History)
4 users (show)

See Also:
Kernel Version: 4.15.x
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The crafted image which causes kernel panic (2.00 MB, application/octet-stream)
2018-03-22 21:06 UTC, Wen Xu
Details
poc.c (3.18 KB, text/plain)
2018-03-22 21:06 UTC, Wen Xu
Details
Revised patch: ext4: limit xattr size to INT_MAX (2.23 KB, patch)
2018-03-29 21:51 UTC, Theodore Tso
Details | Diff

Description Wen Xu 2018-03-22 21:06:17 UTC
Created attachment 274887 [details]
The crafted image which causes kernel panic

- Overview
Kernel references invalid pointer when mounting and later operating on a crafted ext4 image

- Reproduce
# mkdir mnt
# mount -t ext4 136.img mnt
# gcc -o poc poc.c
# ./poc ./mnt

- Reason
https://elixir.bootlin.com/linux/v4.15/source/fs/posix_acl.c#L154

acl = inode->i_op->get_acl(inode, type);

if (IS_ERR(acl)) {
      /*
       * Remove our sentinel so that we don't block future attempts
       * to cache the ACL.
       */
      cmpxchg(p, sentinel, ACL_NOT_CACHED);
      return acl;
}

Interestingly, it seems ext4's get_acl can return 0xffffffffc0c0c0c0, which also bypasses the check `if (IS_ERR(acl))`.

- Kernel dump
[  740.268286] EXT4-fs (loop0): warning: mounting unchecked fs, running e2fsck is recommended
[  740.268788] EXT4-fs (loop0): mounted filesystem without journal. Opts: (null)
[  750.149045] EXT4-fs error (device loop0): ext4_readdir:238: inode #2: block 44: comm poc: path /home/wen/data/ext4-kernel/20180311/get_acl/mnt: bad entry in directory: inode out of bounds - offset=12(12), inode=4294901762, rec_len=12, name_len=2
[  750.166268] EXT4-fs error (device loop0): mb_free_blocks:1468: group 0, inode 16: block 1985:freeing already freed block (bit 1984); block bitmap corrupt.
[  750.166342] EXT4-fs error (device loop0): ext4_mb_generate_buddy:756: group 0, block bitmap and bg descriptor inconsistent: 960 vs 961 free clusters
[  750.166525] BUG: unable to handle kernel paging request at ffffffffc0c0c0c0
[  750.166641] IP: get_acl+0x90/0x100
[  750.166687] PGD 1aa0e067 P4D 1aa0e067 PUD 1aa10067 PMD 0
[  750.166717] Oops: 0002 [#1] SMP PTI
[  750.166793] Modules linked in: vmw_balloon coretemp intel_rapl_perf input_leds joydev serio_raw snd_ens1371 btusb snd_ac97_codec uvcvideo btrtl videobuf2_vmalloc btbcm btintel gameport snd_rawmidi videobuf2_memops bluetooth videobuf2_v4l2 videobuf2_core snd_seq_device ac97_bus snd_pcm videodev media ecdh_generic snd_timer snd soundcore shpchp mac_hid vmw_vsock_vmci_transport vsock vmw_vmci sch_fq_codel ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid hid crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd glue_helper cryptd vmwgfx
[  750.167360]  psmouse ttm drm_kms_helper mptspi mptscsih ahci libahci e1000 mptbase scsi_transport_spi syscopyarea sysfillrect sysimgblt fb_sys_fops drm i2c_piix4 pata_acpi
[  750.167462] CPU: 0 PID: 1322 Comm: poc Not tainted 4.15.0-12-generic #13-Ubuntu
[  750.167538] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[  750.167636] RIP: 0010:get_acl+0x90/0x100
[  750.167683] RSP: 0018:ffffb90740d27d18 EFLAGS: 00010286
[  750.167708] RAX: ffffffffc0c0c0c0 RBX: ffffffffc0c0c0c0 RCX: 0000000000000000
[  750.167793] RDX: 0000000000000001 RSI: ffff8b4bb91e8c20 RDI: 0000000000000000
[  750.167847] RBP: ffffb90740d27d40 R08: 000000000002be00 R09: ffffffffa64ddca2
[  750.167900] R10: ffff8b4bb8df8ea0 R11: 007a0000007a6162 R12: ffff8b4bb8e277e8
[  750.167927] R13: ffff8b4bb6401701 R14: 0000000000008000 R15: ffff8b4bb8e277f8
[  750.167955] FS:  00007fa818f7c540(0000) GS:ffff8b4bbc600000(0000) knlGS:0000000000000000
[  750.168011] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  750.168868] CR2: ffffffffc0c0c0c0 CR3: 00000000391ee005 CR4: 00000000001606f0
[  750.169437] Call Trace:
[  750.169944]  posix_acl_chmod+0x51/0xf0
[  750.170435]  ext4_setattr+0x1f2/0x9d0
[  750.170925]  ? filename_lookup+0xf2/0x190
[  750.171379]  notify_change+0x2eb/0x440
[  750.171826]  chmod_common+0x16a/0x180
[  750.172516]  SyS_chmod+0x59/0xb0
[  750.173215]  do_syscall_64+0x73/0x130
[  750.173902]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[  750.174531] RIP: 0033:0x7fa818a7dac7
[  750.175097] RSP: 002b:00007ffd57657478 EFLAGS: 00000207 ORIG_RAX: 000000000000005a
[  750.175644] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa818a7dac7
[  750.176178] RDX: 00007ffd576574d0 RSI: 0000000000000000 RDI: 0000558df83702f0
[  750.176713] RBP: 00007ffd576575e0 R08: 0000000000000003 R09: 0000000000000000
[  750.177219] R10: ffffffffffffdc2b R11: 0000000000000207 R12: 0000558df6212d30
[  750.177719] R13: 00007ffd576576e0 R14: 0000000000000000 R15: 0000000000000000
[  750.178157] Code: b1 2f 49 8b 44 24 20 48 8b 40 18 48 85 c0 74 40 44 89 f6 4c 89 e7 e8 30 43 92 00 48 3d 00 f0 ff ff 48 89 c3 77 55 48 85 c0 74 03 <3e> ff 00 4c 89 e8 3e 49 0f b1 1f 49 39 c5 75 26 48 89 d8 5b 41
[  750.179468] RIP: get_acl+0x90/0x100 RSP: ffffb90740d27d18
[  750.179889] CR2: ffffffffc0c0c0c0
[  750.180295] ---[ end trace 1ecf08f3cdf242f0 ]---
Comment 1 Wen Xu 2018-03-22 21:06:51 UTC
Created attachment 274889 [details]
poc.c
Comment 2 Wen Xu 2018-03-22 21:07:30 UTC
The crash dump is generated from Ubuntu 18.04 which uses Linux 4.15.

Reported by Wen Xu from SSLab, Gatech.
Comment 3 Theodore Tso 2018-03-25 07:00:10 UTC
Thank you for the bug report.  The following should address the issue
you reported:

commit de57a63ea4389e39b1cdd1cef15e1ec9b58a964c
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Sun Mar 25 02:58:44 2018 -0400

    ext4: add better range checking for e_value_size in xattrs
    
    https://bugzilla.kernel.org/show_bug.cgi?id=199185
    
    Reported-by: Wen Xu <wen.xu@gatech.edu>
    Signed-off-by: Theodore Ts'o <tytso@mit.edu>
    Cc: stable@vger.kernel.org

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 63656dbafdc4..7604d750d234 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -523,10 +523,10 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 	if (error)
 		goto cleanup;
 	size = le32_to_cpu(entry->e_value_size);
+	error = -ERANGE;
+	if (size < 0 || size > buffer_size)
+		goto cleanup;
 	if (buffer) {
-		error = -ERANGE;
-		if (size > buffer_size)
-			goto cleanup;
 		if (entry->e_value_inum) {
 			error = ext4_xattr_inode_get(inode, entry, buffer,
 						     size);
@@ -572,10 +572,10 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 	if (error)
 		goto cleanup;
 	size = le32_to_cpu(entry->e_value_size);
+	error = -ERANGE;
+	if (size < 0 || size > buffer_size)
+		goto cleanup;
 	if (buffer) {
-		error = -ERANGE;
-		if (size > buffer_size)
-			goto cleanup;
 		if (entry->e_value_inum) {
 			error = ext4_xattr_inode_get(inode, entry, buffer,
 						     size);
Comment 4 Eric Biggers 2018-03-27 02:28:49 UTC
The above patch is wrong; it makes ext4_get_acl() always fail with ERANGE, because ext4_get_acl() uses buffer=NULL and buffer_size=0 to get the size of the xattr.  Likewise getxattr(..., NULL, 0) is broken.  I think we should check 'size' against XATTR_SIZE_MAX in the !buffer case instead.  Also checking 'size < 0' is unnecessary since 'size' is unsigned.
Comment 5 Theodore Tso 2018-03-29 21:51:11 UTC
Created attachment 275005 [details]
Revised patch: ext4: limit xattr size to INT_MAX

Here's a revised patch which should address Eric's concerns.

Note You need to log in before you can comment on or make changes to this bug.