[CVE-2018-10840,Bionic,SRU] ext4: correctly handle a zero-length xattr with a non-zero e_value_offs

Message ID 20180726042833.9697-2-po-hsu.lin@canonical.com
State New
Headers show
Series
  • [CVE-2018-10840,Bionic,SRU] ext4: correctly handle a zero-length xattr with a non-zero e_value_offs
Related show

Commit Message

Po-Hsu Lin July 26, 2018, 4:28 a.m.
From: Theodore Ts'o <tytso@mit.edu>

CVE-2018-10840

Ext4 will always create ext4 extended attributes which do not have a
value (where e_value_size is zero) with e_value_offs set to zero.  In
most places e_value_offs will not be used in a substantive way if
e_value_size is zero.

There was one exception to this, which is in ext4_xattr_set_entry(),
where if there is a maliciously crafted file system where there is an
extended attribute with e_value_offs is non-zero and e_value_size is
0, the attempt to remove this xattr will result in a negative value
getting passed to memmove, leading to the following sadness:

[   41.225365] EXT4-fs (loop0): mounted filesystem with ordered data mode. Opts: (null)
[   44.538641] BUG: unable to handle kernel paging request at ffff9ec9a3000000
[   44.538733] IP: __memmove+0x81/0x1a0
[   44.538755] PGD 1249bd067 P4D 1249bd067 PUD 1249c1067 PMD 80000001230000e1
[   44.538793] Oops: 0003 [#1] SMP PTI
[   44.539074] CPU: 0 PID: 1470 Comm: poc Not tainted 4.16.0-rc1+ #1
    ...
[   44.539475] Call Trace:
[   44.539832]  ext4_xattr_set_entry+0x9e7/0xf80
    ...
[   44.539972]  ext4_xattr_block_set+0x212/0xea0
    ...
[   44.540041]  ext4_xattr_set_handle+0x514/0x610
[   44.540065]  ext4_xattr_set+0x7f/0x120
[   44.540090]  __vfs_removexattr+0x4d/0x60
[   44.540112]  vfs_removexattr+0x75/0xe0
[   44.540132]  removexattr+0x4d/0x80
    ...
[   44.540279]  path_removexattr+0x91/0xb0
[   44.540300]  SyS_removexattr+0xf/0x20
[   44.540322]  do_syscall_64+0x71/0x120
[   44.540344]  entry_SYSCALL_64_after_hwframe+0x21/0x86

https://bugzilla.kernel.org/show_bug.cgi?id=199347

This addresses CVE-2018-10840.

Reported-by: "Xu, Wen" <wen.xu@gatech.edu>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Cc: stable@kernel.org
Fixes: dec214d00e0d7 ("ext4: xattr inode deduplication")
(cherry picked from commit 8a2b307c21d4b290e3cbe33f768f194286d07c23)
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 fs/ext4/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Bader July 26, 2018, 12:44 p.m. | #1
On 26.07.2018 06:28, Po-Hsu Lin wrote:
> From: Theodore Ts'o <tytso@mit.edu>
> 
> CVE-2018-10840
> 
> Ext4 will always create ext4 extended attributes which do not have a
> value (where e_value_size is zero) with e_value_offs set to zero.  In
> most places e_value_offs will not be used in a substantive way if
> e_value_size is zero.
> 
> There was one exception to this, which is in ext4_xattr_set_entry(),
> where if there is a maliciously crafted file system where there is an
> extended attribute with e_value_offs is non-zero and e_value_size is
> 0, the attempt to remove this xattr will result in a negative value
> getting passed to memmove, leading to the following sadness:
> 
> [   41.225365] EXT4-fs (loop0): mounted filesystem with ordered data mode. Opts: (null)
> [   44.538641] BUG: unable to handle kernel paging request at ffff9ec9a3000000
> [   44.538733] IP: __memmove+0x81/0x1a0
> [   44.538755] PGD 1249bd067 P4D 1249bd067 PUD 1249c1067 PMD 80000001230000e1
> [   44.538793] Oops: 0003 [#1] SMP PTI
> [   44.539074] CPU: 0 PID: 1470 Comm: poc Not tainted 4.16.0-rc1+ #1
>     ...
> [   44.539475] Call Trace:
> [   44.539832]  ext4_xattr_set_entry+0x9e7/0xf80
>     ...
> [   44.539972]  ext4_xattr_block_set+0x212/0xea0
>     ...
> [   44.540041]  ext4_xattr_set_handle+0x514/0x610
> [   44.540065]  ext4_xattr_set+0x7f/0x120
> [   44.540090]  __vfs_removexattr+0x4d/0x60
> [   44.540112]  vfs_removexattr+0x75/0xe0
> [   44.540132]  removexattr+0x4d/0x80
>     ...
> [   44.540279]  path_removexattr+0x91/0xb0
> [   44.540300]  SyS_removexattr+0xf/0x20
> [   44.540322]  do_syscall_64+0x71/0x120
> [   44.540344]  entry_SYSCALL_64_after_hwframe+0x21/0x86
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199347
> 
> This addresses CVE-2018-10840.
> 
> Reported-by: "Xu, Wen" <wen.xu@gatech.edu>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Cc: stable@kernel.org
> Fixes: dec214d00e0d7 ("ext4: xattr inode deduplication")
> (cherry picked from commit 8a2b307c21d4b290e3cbe33f768f194286d07c23)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  fs/ext4/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 1718354..ed1cf24 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1687,7 +1687,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
>  
>  	/* No failures allowed past this point. */
>  
> -	if (!s->not_found && here->e_value_offs) {
> +	if (!s->not_found && here->e_value_size && here->e_value_offs) {
>  		/* Remove the old value. */
>  		void *first_val = s->base + min_offs;
>  		size_t offs = le16_to_cpu(here->e_value_offs);
>
Kleber Souza July 26, 2018, 4:37 p.m. | #2
On 07/26/18 06:28, Po-Hsu Lin wrote:
> From: Theodore Ts'o <tytso@mit.edu>
> 
> CVE-2018-10840
> 
> Ext4 will always create ext4 extended attributes which do not have a
> value (where e_value_size is zero) with e_value_offs set to zero.  In
> most places e_value_offs will not be used in a substantive way if
> e_value_size is zero.
> 
> There was one exception to this, which is in ext4_xattr_set_entry(),
> where if there is a maliciously crafted file system where there is an
> extended attribute with e_value_offs is non-zero and e_value_size is
> 0, the attempt to remove this xattr will result in a negative value
> getting passed to memmove, leading to the following sadness:
> 
> [   41.225365] EXT4-fs (loop0): mounted filesystem with ordered data mode. Opts: (null)
> [   44.538641] BUG: unable to handle kernel paging request at ffff9ec9a3000000
> [   44.538733] IP: __memmove+0x81/0x1a0
> [   44.538755] PGD 1249bd067 P4D 1249bd067 PUD 1249c1067 PMD 80000001230000e1
> [   44.538793] Oops: 0003 [#1] SMP PTI
> [   44.539074] CPU: 0 PID: 1470 Comm: poc Not tainted 4.16.0-rc1+ #1
>     ...
> [   44.539475] Call Trace:
> [   44.539832]  ext4_xattr_set_entry+0x9e7/0xf80
>     ...
> [   44.539972]  ext4_xattr_block_set+0x212/0xea0
>     ...
> [   44.540041]  ext4_xattr_set_handle+0x514/0x610
> [   44.540065]  ext4_xattr_set+0x7f/0x120
> [   44.540090]  __vfs_removexattr+0x4d/0x60
> [   44.540112]  vfs_removexattr+0x75/0xe0
> [   44.540132]  removexattr+0x4d/0x80
>     ...
> [   44.540279]  path_removexattr+0x91/0xb0
> [   44.540300]  SyS_removexattr+0xf/0x20
> [   44.540322]  do_syscall_64+0x71/0x120
> [   44.540344]  entry_SYSCALL_64_after_hwframe+0x21/0x86
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199347
> 
> This addresses CVE-2018-10840.
> 
> Reported-by: "Xu, Wen" <wen.xu@gatech.edu>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Cc: stable@kernel.org
> Fixes: dec214d00e0d7 ("ext4: xattr inode deduplication")
> (cherry picked from commit 8a2b307c21d4b290e3cbe33f768f194286d07c23)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  fs/ext4/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 1718354..ed1cf24 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1687,7 +1687,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
>  
>  	/* No failures allowed past this point. */
>  
> -	if (!s->not_found && here->e_value_offs) {
> +	if (!s->not_found && here->e_value_size && here->e_value_offs) {
>  		/* Remove the old value. */
>  		void *first_val = s->base + min_offs;
>  		size_t offs = le16_to_cpu(here->e_value_offs);
>

Patch

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 1718354..ed1cf24 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1687,7 +1687,7 @@  static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 
 	/* No failures allowed past this point. */
 
-	if (!s->not_found && here->e_value_offs) {
+	if (!s->not_found && here->e_value_size && here->e_value_offs) {
 		/* Remove the old value. */
 		void *first_val = s->base + min_offs;
 		size_t offs = le16_to_cpu(here->e_value_offs);