Patch "ext4: enforce buffer head state assertion in ext4_da_map_blocks" has been added to the 5.14-stable tree

DISCUSSION Eric Whitney 2 participants 2 messages Oct 13, 2021 1 message not in the archive Stable branch

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Eric Whitney

author

2021-10-13 13:53 UTC #1 lore raw

* Sasha Levin <[email protected]>:

Quoted 55 lines

This is a note to let you know that I've just added the patch titled
ext4: enforce buffer head state assertion in ext4_da_map_blocks
to the 5.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ext4-enforce-buffer-head-state-assertion-in-ext4_da_.patch
and it can be found in the queue-5.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
commit b2838e02c515366e8452370fcda5baa2dcc8be68
Author: Eric Whitney <[email protected]>
Date:   Thu Aug 19 10:49:27 2021 -0400
ext4: enforce buffer head state assertion in ext4_da_map_blocks
[ Upstream commit 948ca5f30e1df0c11eb5b0f410b9ceb97fa77ad9 ]
Remove the code that re-initializes a buffer head with an invalid block number and BH_New and BH_Delay bits when a matching delayed and unwritten block has been found in the extent status cache. Replace it with assertions that verify the buffer head already has this state correctly set. The current code masked an inline data truncation bug that left stale entries in the extent status cache. With this change, generic/130 can be used to reproduce and detect that bug.
Signed-off-by Eric Whitney <[email protected]>
Signed-off-by Theodore Ts'o <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Theodore Ts'o <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
fs/ext4/inode.c +5 -3
index fc6ea56de77c..d204688b32a3 100644
@@ -1726,13 +1726,16 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
 		}
 
 		/*
-		 * Delayed extent could be allocated by fallocate.
-		 * So we need to check it.
+		 * the buffer head associated with a delayed and not unwritten
+		 * block found in the extent status cache must contain an
+		 * invalid block number and have its BH_New and BH_Delay bits
+		 * set, reflecting the state assigned when the block was
+		 * initially delayed allocated
 		 */
-		if (ext4_es_is_delayed(&es) && !ext4_es_is_unwritten(&es)) {

fs/ext4/inode.c static int ext4_da_map_blocks(struct inode *inode, sector_t iblock, · line 1741

Quoted 10 lines

fs/ext4/inode.c +4 -3

-			map_bh(bh, inode->i_sb, invalid_block);
-			set_buffer_new(bh);
-			set_buffer_delay(bh);
+		if (ext4_es_is_delonly(&es)) {
+			BUG_ON(bh->b_blocknr != invalid_block);
+			BUG_ON(!buffer_new(bh));
+			BUG_ON(!buffer_delay(bh));
 			return 0;
 		}

This patch should not be added to the stable tree, as it will be reverted in 5.15.

There have been two reports of unexpected kernel panics triggered by this code in kernels derived from 5.15-rc4, and the code will be removed for the time being until the root cause can be determined and corrected in a future release.

Thanks,
Eric

Greg KH

2021-10-13 14:10 UTC #2 lore raw

Quoted 71 lines · Eric Whitney

* Sasha Levin <[email protected]>:
Quoted 55 lines
This is a note to let you know that I've just added the patch titled
ext4: enforce buffer head state assertion in ext4_da_map_blocks
to the 5.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ext4-enforce-buffer-head-state-assertion-in-ext4_da_.patch
and it can be found in the queue-5.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
commit b2838e02c515366e8452370fcda5baa2dcc8be68
Author: Eric Whitney <[email protected]>
Date:   Thu Aug 19 10:49:27 2021 -0400
ext4: enforce buffer head state assertion in ext4_da_map_blocks
[ Upstream commit 948ca5f30e1df0c11eb5b0f410b9ceb97fa77ad9 ]
Remove the code that re-initializes a buffer head with an invalid block number and BH_New and BH_Delay bits when a matching delayed and unwritten block has been found in the extent status cache. Replace it with assertions that verify the buffer head already has this state correctly set. The current code masked an inline data truncation bug that left stale entries in the extent status cache. With this change, generic/130 can be used to reproduce and detect that bug.
Signed-off-by Eric Whitney <[email protected]>
Signed-off-by Theodore Ts'o <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Theodore Ts'o <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
fs/ext4/inode.c +5 -3
index fc6ea56de77c..d204688b32a3 100644
@@ -1726,13 +1726,16 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
 		}
 
 		/*
-		 * Delayed extent could be allocated by fallocate.
-		 * So we need to check it.
+		 * the buffer head associated with a delayed and not unwritten
+		 * block found in the extent status cache must contain an
+		 * invalid block number and have its BH_New and BH_Delay bits
+		 * set, reflecting the state assigned when the block was
+		 * initially delayed allocated
 		 */
-		if (ext4_es_is_delayed(&es) && !ext4_es_is_unwritten(&es)) {
fs/ext4/inode.c static int ext4_da_map_blocks(struct inode *inode, sector_t iblock, · line 1741
Quoted 10 lines
fs/ext4/inode.c +4 -3
-			map_bh(bh, inode->i_sb, invalid_block);
-			set_buffer_new(bh);
-			set_buffer_delay(bh);
+		if (ext4_es_is_delonly(&es)) {
+			BUG_ON(bh->b_blocknr != invalid_block);
+			BUG_ON(!buffer_new(bh));
+			BUG_ON(!buffer_delay(bh));
 			return 0;
 		}
 
This patch should not be added to the stable tree, as it will be reverted in 5.15.

Quoted 3 lines

There have been two reports of unexpected kernel panics triggered by this code in kernels derived from 5.15-rc4, and the code will be removed for the time being until the root cause can be determined and corrected in a future release.

Now dropped from all stable queues, thanks.

greg k-h