[CVE-2018-11412,Bionic,SRU,1/1] ext4: do not allow external inodes for inline data

Message ID 20180726084102.29175-2-po-hsu.lin@canonical.com
State New
Headers show
Series
  • Fix for CVE-2018-11412
Related show

Commit Message

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

CVE-2018-11412

The inline data feature was implemented before we added support for
external inodes for xattrs.  It makes no sense to support that
combination, but the problem is that there are a number of extended
attribute checks that are skipped if e_value_inum is non-zero.

Unfortunately, the inline data code is completely e_value_inum
unaware, and attempts to interpret the xattr fields as if it were an
inline xattr --- at which point, Hilarty Ensues.

This addresses CVE-2018-11412.

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

Reported-by: Jann Horn <jannh@google.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Fixes: e50e5129f384 ("ext4: xattr-in-inode support")
Cc: stable@kernel.org
(cherry picked from commit 117166efb1ee8f13c38f9e96b258f16d4923f888)
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 fs/ext4/inline.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Stefan Bader July 26, 2018, 12:40 p.m. | #1
On 26.07.2018 10:41, Po-Hsu Lin wrote:
> From: Theodore Ts'o <tytso@mit.edu>
> 
> CVE-2018-11412
> 
> The inline data feature was implemented before we added support for
> external inodes for xattrs.  It makes no sense to support that
> combination, but the problem is that there are a number of extended
> attribute checks that are skipped if e_value_inum is non-zero.
> 
> Unfortunately, the inline data code is completely e_value_inum
> unaware, and attempts to interpret the xattr fields as if it were an
> inline xattr --- at which point, Hilarty Ensues.
> 
> This addresses CVE-2018-11412.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199803
> 
> Reported-by: Jann Horn <jannh@google.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Fixes: e50e5129f384 ("ext4: xattr-in-inode support")
> Cc: stable@kernel.org
> (cherry picked from commit 117166efb1ee8f13c38f9e96b258f16d4923f888)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  fs/ext4/inline.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 1367553c43bb..1e10eb9aa6f8 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -151,6 +151,12 @@ int ext4_find_inline_data_nolock(struct inode *inode)
>  		goto out;
>  
>  	if (!is.s.not_found) {
> +		if (is.s.here->e_value_inum) {
> +			EXT4_ERROR_INODE(inode, "inline data xattr refers "
> +					 "to an external xattr inode");
> +			error = -EFSCORRUPTED;
> +			goto out;
> +		}
>  		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
>  					(void *)ext4_raw_inode(&is.iloc));
>  		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +
>
Kleber Souza July 26, 2018, 4:34 p.m. | #2
On 07/26/18 10:41, Po-Hsu Lin wrote:
> From: Theodore Ts'o <tytso@mit.edu>
> 
> CVE-2018-11412
> 
> The inline data feature was implemented before we added support for
> external inodes for xattrs.  It makes no sense to support that
> combination, but the problem is that there are a number of extended
> attribute checks that are skipped if e_value_inum is non-zero.
> 
> Unfortunately, the inline data code is completely e_value_inum
> unaware, and attempts to interpret the xattr fields as if it were an
> inline xattr --- at which point, Hilarty Ensues.
> 
> This addresses CVE-2018-11412.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199803
> 
> Reported-by: Jann Horn <jannh@google.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Fixes: e50e5129f384 ("ext4: xattr-in-inode support")
> Cc: stable@kernel.org
> (cherry picked from commit 117166efb1ee8f13c38f9e96b258f16d4923f888)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  fs/ext4/inline.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 1367553c43bb..1e10eb9aa6f8 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -151,6 +151,12 @@ int ext4_find_inline_data_nolock(struct inode *inode)
>  		goto out;
>  
>  	if (!is.s.not_found) {
> +		if (is.s.here->e_value_inum) {
> +			EXT4_ERROR_INODE(inode, "inline data xattr refers "
> +					 "to an external xattr inode");
> +			error = -EFSCORRUPTED;
> +			goto out;
> +		}
>  		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
>  					(void *)ext4_raw_inode(&is.iloc));
>  		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +
>

Patch

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 1367553c43bb..1e10eb9aa6f8 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -151,6 +151,12 @@  int ext4_find_inline_data_nolock(struct inode *inode)
 		goto out;
 
 	if (!is.s.not_found) {
+		if (is.s.here->e_value_inum) {
+			EXT4_ERROR_INODE(inode, "inline data xattr refers "
+					 "to an external xattr inode");
+			error = -EFSCORRUPTED;
+			goto out;
+		}
 		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
 					(void *)ext4_raw_inode(&is.iloc));
 		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +