| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Dec 2012 13:23:40 +0530
From: Ashish Sangwan <ashishsangwan2@...il.com>
To: Forrest Liu <forrestl@...ology.com>
Cc: "Theodore Ts'o" <tytso@....edu>,
ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: fix extent tree corruption that incurred by hole punch
I have tested it and the problem is real.
It happens when depth => 2 and punch hole completely removes any of
the first extent indexes at depth 1.
The change in ei_block is not propagated backwards to index at depth 0.
However, AFAICS, the problematic call to ext4_ext_rm_idx is from
ext4_ext_rm_leaf and there is no need to change in
ext4_ext_remove_space or in ext4_ext_rm_idx.
Forrest, what do you think about below change ?
basically its your code, but its been added to ext4_ext_rm_leaf after
removing index.
@@ -2448,8 +2464,28 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
/* if this leaf is free, then we should
* remove it from index block above */
- if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
+ if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL) {
err = ext4_ext_rm_idx(handle, inode, path + depth);
+ /* If this was the first extent index in node,
+ * propagates the ei_block updation info to top */
+ if(!err) {
+ --depth;
+ path = path + depth;
+ while (--depth >= 0) {
+ if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr))
+ break;
+ path--;
+ err = ext4_ext_get_access(handle, inode, path);
+ if (err)
+ break;
+ path->p_idx->ei_block = border;
+ err = ext4_ext_dirty(handle, inode, path);
+ if (err)
+ break;
+ }
+ }
+ }
+
On Wed, Dec 5, 2012 at 11:43 AM, Forrest Liu <forrestl@...ology.com> wrote:
> 2012/12/4 forrest <forrestl@...ology.com>:
>> This problem is easily to reproduce
>>
>> Create a file with size larger than 1GB.
>>
>> dd if=/dev/zero of=/test_file bs=1M count=1024
>>
>> Punch every even block in test_file, and then use debugfs to dump
>> extents, followings is dumped result
>>
>> 2/ 2 339/340 231197 - 231197 3917597 - 3917597 1
>> 2/ 2 340/340 231199 - 231199 3917599 - 3917599 1
>> 0/ 2 2/ 2 231201 - 262143 3901486 30943
>> 1/ 2 1/ 46 231201 - 231880 3901488 680
>> 2/ 2 1/340 231201 - 231201 3917601 - 3917601 1
>> 2/ 2 2/340 231203 - 231203 3917603 - 3917603 1
>>
>> Punch blocks #231779 ~#231201 , to remove extent index, and then use
>> debugfs to dump extents, followings is dumped result
>>
>> 2/ 2 340/340 231199 - 231199 3917599 - 3917599 1
>> 0/ 2 2/ 2 231201 - 262143 3901486 30943
>> -------> logical block index didn't update when remove extent index
>> 1/ 2 1/ 45 231881 - 232560 3901490 680
>> 2/ 2 1/340 231881 - 231881 3918281 - 3918281 1
>> 2/ 2 2/340 231883 - 231883 3918283 - 3918283 1
>
> After blocks #231202~#231880 had been punch out, theses blocks can't map again.
> If we do map a block in #231202~#231880, we will get error messages
> like followings.
>
> EXT4-fs error (device md2): ext4_ext_search_left:1239: inode #13: comm
> flush-9:2: ix (231201) != EXT_FIRST_INDEX (1) (depth 0)!
> EXT4-fs (md2): delayed block allocation failed for inode 13 at logical
> offset 231203 with max blocks 1 with error -5
> EXT4-fs (md2): This should not happen!! Data will be lost
>
> Regards,
> Forrest
>
>>
>> 2012/12/4 Forrest Liu <forrestl@...ology.com>:
>>> Extent indexes didn't update correctly in ext4_ext_rm_idx, when depth
>>> of extent tree is greater than 1.
>>>
>>> Signed-off-by: Forrest Liu <forrestl@...ology.com>
>>> ---
>>> fs/ext4/extents.c | 24 ++++++++++++++++++++----
>>> 1 files changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>> index d3dd618..b10b8c0 100644
>>> --- a/fs/ext4/extents.c
>>> +++ b/fs/ext4/extents.c
>>> @@ -2190,13 +2190,15 @@ errout:
>>> * removes index from the index block.
>>> */
>>> static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
>>> - struct ext4_ext_path *path)
>>> + struct ext4_ext_path *path, int depth)
>>> {
>>> int err;
>>> ext4_fsblk_t leaf;
>>> + __le32 border;
>>>
>>> /* free index block */
>>> - path--;
>>> + depth--;
>>> + path = path + depth;
>>> leaf = ext4_idx_pblock(path->p_idx);
>>> if (unlikely(path->p_hdr->eh_entries == 0)) {
>>> EXT4_ERROR_INODE(inode, "path->p_hdr->eh_entries == 0");
>>> @@ -2221,6 +2223,20 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
>>>
>>> ext4_free_blocks(handle, inode, NULL, leaf, 1,
>>> EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
>>> +
>>> + border = path->p_idx->ei_block;
>>> + while (--depth >= 0) {
>>> + if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr))
>>> + break;
>>> + path--;
>>> + err = ext4_ext_get_access(handle, inode, path);
>>> + if (err)
>>> + break;
>>> + path->p_idx->ei_block = border;
>>> + err = ext4_ext_dirty(handle, inode, path);
>>> + if (err)
>>> + break;
>>> + }
>>> return err;
>>> }
>>>
>>> @@ -2557,7 +2573,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
>>> /* if this leaf is free, then we should
>>> * remove it from index block above */
>>> if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
>>> - err = ext4_ext_rm_idx(handle, inode, path + depth);
>>> + err = ext4_ext_rm_idx(handle, inode, path, depth);
>>>
>>> out:
>>> return err;
>>> @@ -2760,7 +2776,7 @@ again:
>>> /* index is empty, remove it;
>>> * handle must be already prepared by the
>>> * truncatei_leaf() */
>>> - err = ext4_ext_rm_idx(handle, inode, path + i);
>>> + err = ext4_ext_rm_idx(handle, inode, path, i);
>>> }
>>> /* root level has p_bh == NULL, brelse() eats this */
>>> brelse(path[i].p_bh);
>>> --
>>> 1.7.5.4
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists