Bug 199185
| Summary: | Invalid pointer dereference in get_acl (fs/posix_acl.c) when mounting and operating crafted ext4 image | ||
|---|---|---|---|
| Product: | File System | Reporter: | Wen Xu (wen.xu) |
| Component: | ext4 | Assignee: | fs_ext4 (fs_ext4) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | ebiggers3, kpraveen.lkml, tytso, wen.xu |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.15.x | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
The crafted image which causes kernel panic
poc.c Revised patch: ext4: limit xattr size to INT_MAX |
||
|
Description
Wen Xu
2018-03-22 21:06:17 UTC
Created attachment 274889 [details]
poc.c
The crash dump is generated from Ubuntu 18.04 which uses Linux 4.15. Reported by Wen Xu from SSLab, Gatech. 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); 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. Created attachment 275005 [details]
Revised patch: ext4: limit xattr size to INT_MAX
Here's a revised patch which should address Eric's concerns.
|