Skip to contentSkip to conversation

ext4: avoid running out of journal credits when appending to an inline file

PATCH Superseded Chenbo Feng 2 participants 6 messages Oct 12, 2018 – Oct 19, 2018 Stable branch

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Chenbo Feng

author
#0 lore raw

PATCH v2 1/1 ext4: avoid running out of journal credits when appending to an inline file

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

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

Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>
---
 fs/ext4/ext4.h   |  3 ---
 fs/ext4/inline.c | 38 +-------------------------------------
 fs/ext4/xattr.c  | 18 ++----------------
 3 files changed, 3 insertions(+), 56 deletions(-)
fs/ext4/ext4.h +0 -3
index f5d9f82b173a..b6e25d771eea 100644
@@ -3039,9 +3039,6 @@ extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode,
 extern int ext4_inline_data_fiemap(struct inode *inode,
 				   struct fiemap_extent_info *fieinfo,
 				   int *has_inline, __u64 start, __u64 len);
-extern int ext4_try_to_evict_inline_data(handle_t *handle,
-					 struct inode *inode,
-					 int needed);
 extern void ext4_inline_data_truncate(struct inode *inode, int *has_inline);
 
 extern int ext4_convert_inline_data(struct inode *inode);
fs/ext4/inline.c +1 -37
index 1e7a9774119c..5ead3b0f3d34 100644
@@ -888,11 +888,11 @@ retry_journal:
 	flags |= AOP_FLAG_NOFS;
 
 	if (ret == -ENOSPC) {
+		ext4_journal_stop(handle);
 		ret = ext4_da_convert_inline_data_to_extent(mapping,
 							    inode,
 							    flags,
 							    fsdata);
-		ext4_journal_stop(handle);
 		if (ret == -ENOSPC &&
 		    ext4_should_retry_alloc(inode->i_sb, &retries))
 			goto retry_journal;

@@ -1867,42 +1867,6 @@ out:
 	return (error < 0 ? error : 0);
 }
 
-/*
- * Called during xattr set, and if we can sparse space 'needed',
- * just create the extent tree evict the data to the outer block.
- *
- * We use jbd2 instead of page cache to move data to the 1st block
- * so that the whole transaction can be committed as a whole and
- * the data isn't lost because of the delayed page cache write.
- */
-int ext4_try_to_evict_inline_data(handle_t *handle,
-				  struct inode *inode,
-				  int needed)
-{
-	int error;
-	struct ext4_xattr_entry *entry;
-	struct ext4_inode *raw_inode;
-	struct ext4_iloc iloc;
-
-	error = ext4_get_inode_loc(inode, &iloc);
-	if (error)
-		return error;
-
-	raw_inode = ext4_raw_inode(&iloc);
-	entry = (struct ext4_xattr_entry *)((void *)raw_inode +
-					    EXT4_I(inode)->i_inline_off);
-	if (EXT4_XATTR_LEN(entry->e_name_len) +
-	    EXT4_XATTR_SIZE(le32_to_cpu(entry->e_value_size)) < needed) {
-		error = -ENOSPC;
-		goto out;
-	}
-
-	error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
-out:
-	brelse(iloc.bh);
-	return error;
-}
-
 void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
 {
 	handle_t *handle;
fs/ext4/xattr.c +2 -16
index b51bb73b06a6..b555b5ee0839 100644
@@ -1038,22 +1038,8 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 	if (EXT4_I(inode)->i_extra_isize == 0)
 		return -ENOSPC;
 	error = ext4_xattr_set_entry(i, s);
-	if (error) {
-		if (error == -ENOSPC &&
-		    ext4_has_inline_data(inode)) {
-			error = ext4_try_to_evict_inline_data(handle, inode,
-					EXT4_XATTR_LEN(strlen(i->name) +
-					EXT4_XATTR_SIZE(i->value_len)));
-			if (error)
-				return error;
-			error = ext4_xattr_ibody_find(inode, i, is);
-			if (error)
-				return error;
-			error = ext4_xattr_set_entry(i, s);
-		}
-		if (error)
-			return error;
-	}
+	if (error)
+		return error;
 	header = IHDR(inode, ext4_raw_inode(&is->iloc));
 	if (!IS_LAST_ENTRY(s->first)) {
 		header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
Signature · 1 line
2.19.0.605.g01d371f741-goog
author
#1 lore raw

PATCH v2 1/1 ext4: avoid running out of journal credits when appending to an inline file

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

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

Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>
---
 fs/ext4/ext4.h   |  3 ---
 fs/ext4/inline.c | 38 +-------------------------------------
 fs/ext4/xattr.c  | 18 ++----------------
 3 files changed, 3 insertions(+), 56 deletions(-)
fs/ext4/ext4.h +0 -3
index 43e27d8ec770..567a6c7af677 100644
@@ -3038,9 +3038,6 @@ extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode,
 extern int ext4_inline_data_fiemap(struct inode *inode,
 				   struct fiemap_extent_info *fieinfo,
 				   int *has_inline, __u64 start, __u64 len);
-extern int ext4_try_to_evict_inline_data(handle_t *handle,
-					 struct inode *inode,
-					 int needed);
 extern void ext4_inline_data_truncate(struct inode *inode, int *has_inline);
 
 extern int ext4_convert_inline_data(struct inode *inode);
fs/ext4/inline.c +1 -37
index 211539a7adfc..6779a9f1de3b 100644
@@ -889,11 +889,11 @@ int ext4_da_write_inline_data_begin(struct address_space *mapping,
 	flags |= AOP_FLAG_NOFS;
 
 	if (ret == -ENOSPC) {
+		ext4_journal_stop(handle);
 		ret = ext4_da_convert_inline_data_to_extent(mapping,
 							    inode,
 							    flags,
 							    fsdata);
-		ext4_journal_stop(handle);
 		if (ret == -ENOSPC &&
 		    ext4_should_retry_alloc(inode->i_sb, &retries))
 			goto retry_journal;

@@ -1865,42 +1865,6 @@ int ext4_inline_data_fiemap(struct inode *inode,
 	return (error < 0 ? error : 0);
 }
 
-/*
- * Called during xattr set, and if we can sparse space 'needed',
- * just create the extent tree evict the data to the outer block.
- *
- * We use jbd2 instead of page cache to move data to the 1st block
- * so that the whole transaction can be committed as a whole and
- * the data isn't lost because of the delayed page cache write.
- */
-int ext4_try_to_evict_inline_data(handle_t *handle,
-				  struct inode *inode,
-				  int needed)
-{
-	int error;
-	struct ext4_xattr_entry *entry;
-	struct ext4_inode *raw_inode;
-	struct ext4_iloc iloc;
-
-	error = ext4_get_inode_loc(inode, &iloc);
-	if (error)
-		return error;
-
-	raw_inode = ext4_raw_inode(&iloc);
-	entry = (struct ext4_xattr_entry *)((void *)raw_inode +
-					    EXT4_I(inode)->i_inline_off);
-	if (EXT4_XATTR_LEN(entry->e_name_len) +
-	    EXT4_XATTR_SIZE(le32_to_cpu(entry->e_value_size)) < needed) {
-		error = -ENOSPC;
-		goto out;
-	}
-
-	error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
-out:
-	brelse(iloc.bh);
-	return error;
-}
-
 void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
 {
 	handle_t *handle;
fs/ext4/xattr.c +2 -16
index c19c96840480..3b402c86565a 100644
@@ -1080,22 +1080,8 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 	if (EXT4_I(inode)->i_extra_isize == 0)
 		return -ENOSPC;
 	error = ext4_xattr_set_entry(i, s);
-	if (error) {
-		if (error == -ENOSPC &&
-		    ext4_has_inline_data(inode)) {
-			error = ext4_try_to_evict_inline_data(handle, inode,
-					EXT4_XATTR_LEN(strlen(i->name) +
-					EXT4_XATTR_SIZE(i->value_len)));
-			if (error)
-				return error;
-			error = ext4_xattr_ibody_find(inode, i, is);
-			if (error)
-				return error;
-			error = ext4_xattr_set_entry(i, s);
-		}
-		if (error)
-			return error;
-	}
+	if (error)
+		return error;
 	header = IHDR(inode, ext4_raw_inode(&is->iloc));
 	if (!IS_LAST_ENTRY(s->first)) {
 		header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
Signature · 1 line
2.19.0.605.g01d371f741-goog
maintainer
#2 lore raw
Quoted 17 lines · Chenbo Feng

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

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

Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>
Quoted 5 lines
---
 fs/ext4/ext4.h   |  3 ---
 fs/ext4/inline.c | 38 +-------------------------------------
 fs/ext4/xattr.c  | 18 ++----------------
 3 files changed, 3 insertions(+), 56 deletions(-)

This patch still does not apply to the latest 4.9 stable tree, can you rebase these again? There's been a lot of patches happening in this area recently, sorry.

greg k-h

author
#3 lore raw

Resend with plain text, I have rebased and sent out the 4.9.y patch in a separate email.

Hi Greg,

I think this is the back-port for 4.4.y not 4.9.y. Could you try apply it on 4.4.y tree instead? I will rebase and send you the 4.9 backport separately.

Thanks

Quoted 35 lines · Greg KH
Quoted 17 lines · Chenbo Feng

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

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

Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>
Quoted 5 lines
---
 fs/ext4/ext4.h   |  3 ---
 fs/ext4/inline.c | 38 +-------------------------------------
 fs/ext4/xattr.c  | 18 ++----------------
 3 files changed, 3 insertions(+), 56 deletions(-)

This patch still does not apply to the latest 4.9 stable tree, can you rebase these again? There's been a lot of patches happening in this area recently, sorry.

greg k-h

--
You received this message because you are subscribed to the Google Groups "kernel-team" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

maintainer
#4 lore raw
In reply to Chenbo Feng
Quoted 10 lines · Chenbo Feng

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

Quoted 7 lines

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

Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>

This also does not apply to the 4.4.y tree :(

author
#5 lore raw
Quoted 14 lines · Greg KH
Quoted 12 lines · Chenbo Feng

From: Theodore Ts'o <[email protected]>

commit 8bc1379b82b8e809eef77a9fedbb75c6c297be19 upstream.

Use a separate journal transaction if it turns out that we need to convert an inline file to use an data block. Otherwise we could end up failing due to not having journal credits.

This addresses CVE-2018-10883.

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

Quoted 8 lines
Quoted 5 lines
Change-Id Ifbe92e379f7a25fb252a2584356ccb91f902ea8f
Signed-off-by Theodore Ts'o <[email protected]>
Cc [email protected]
[[email protected]: 4.4 and 4.9 backport: adjust context]
Signed-off-by Chenbo Feng <[email protected]>

This also does not apply to the 4.4.y tree :(

That's weird, I will rebased and resend again, I didn't find any conflict while rebasing though.
Sorry for the trouble and confusion caused.

Quoted 4 lines

--
You received this message because you are subscribed to the Google Groups "kernel-team" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].