From patchwork Tue Aug 4 08:24:41 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Akira Fujita X-Patchwork-Id: 30707 Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id F19FFB6F1F for ; Tue, 4 Aug 2009 18:25:06 +1000 (EST) Received: by ozlabs.org (Postfix) id E4BA0DDD0C; Tue, 4 Aug 2009 18:25:06 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 55672DDD0B for ; Tue, 4 Aug 2009 18:25:06 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755256AbZHDIZA (ORCPT ); Tue, 4 Aug 2009 04:25:00 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755299AbZHDIZA (ORCPT ); Tue, 4 Aug 2009 04:25:00 -0400 Received: from TYO202.gate.nec.co.jp ([202.32.8.206]:36799 "EHLO tyo202.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755256AbZHDIY7 (ORCPT ); Tue, 4 Aug 2009 04:24:59 -0400 Received: from mailgate3.nec.co.jp ([10.7.69.197]) by tyo202.gate.nec.co.jp (8.13.8/8.13.4) with ESMTP id n748OsO6003585; Tue, 4 Aug 2009 17:24:54 +0900 (JST) Received: (from root@localhost) by mailgate3.nec.co.jp (8.11.7/3.7W-MAILGATE-NEC) id n748OrP03993; Tue, 4 Aug 2009 17:24:53 +0900 (JST) Received: from mail03.kamome.nec.co.jp (mail03.kamome.nec.co.jp [10.25.43.7]) by mailsv4.nec.co.jp (8.13.8/8.13.4) with ESMTP id n748O7k8015845; Tue, 4 Aug 2009 17:24:53 +0900 (JST) Received: from shoin.jp.nec.com ([10.26.220.3] [10.26.220.3]) by mail03.kamome.nec.co.jp with ESMTP id BT-MMP-1025954; Tue, 4 Aug 2009 17:24:42 +0900 Received: from [10.64.168.93] ([10.64.168.93] [10.64.168.93]) by mail.jp.nec.com with ESMTP; Tue, 4 Aug 2009 17:24:42 +0900 Message-ID: <4A77F049.4050301@rs.jp.nec.com> Date: Tue, 04 Aug 2009 17:24:41 +0900 From: Akira Fujita User-Agent: Thunderbird 2.0.0.22 (Windows/20090605) MIME-Version: 1.0 To: Peng Tao CC: linux-ext4@vger.kernel.org, "Theodore Ts'o" Subject: Re: [PATCH 2/2] ext4: fix null pointer bug in move_extent.c References: <1249213404-6277-1-git-send-email-bergwolf@gmail.com> <1249213404-6277-2-git-send-email-bergwolf@gmail.com> <4A76A043.20105@rs.jp.nec.com> <6149e97b0908030620w3a4b4d9em7d8b1aa4940e837d@mail.gmail.com> In-Reply-To: <6149e97b0908030620w3a4b4d9em7d8b1aa4940e837d@mail.gmail.com> Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Hi Peng, Peng Tao wrote: > Will there be situations where empty extents exist in the middle of an > extent tree? Because your patch only checks NULL extents once at the > begining. If some NULL extents are later found in the loop, the bug > still can be triggered and we should check NULL extents in > mext_replace_branches(). In the case of eh->entries is 0 in ext4_ext_find_extent(), the extent which ext4_ext_path indicates is null. This is happened if file blocks are not allocated yet. But an extent tree does not have empty extents in the middle of it, I think. I have no idea we should handle empty extents case in EXT4_IOC_MOVE_EXTENTS, but if yes, we should do null check at other places not only mext_replace_branches(). So I add this null check to get_ext_path macro which is used to get ext4_ext_path. Regards, Akira Fujita Signed-off-by: Akira Fujita --- move_extent.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- linux-2.6.31-rc4-a/fs/ext4/move_extent.c 2009-07-23 11:32:59.000000000 +0900 +++ linux-2.6.31-rc4-b/fs/ext4/move_extent.c 2009-08-04 15:51:04.000000000 +0900 @@ -25,6 +25,8 @@ if (IS_ERR(path)) { \ ret = PTR_ERR(path); \ path = NULL; \ + } else if (path[ext_depth(inode)].p_ext == NULL) { \ + ret = -ENOENT; \ } \ } while (0) @@ -284,7 +286,7 @@ mext_insert_across_blocks(handle_t *hand if (new_flag) { get_ext_path(orig_path, orig_inode, eblock, err); - if (orig_path == NULL) + if (err) goto out; if (ext4_ext_insert_extent(handle, orig_inode, @@ -295,7 +297,7 @@ mext_insert_across_blocks(handle_t *hand if (end_flag) { get_ext_path(orig_path, orig_inode, le32_to_cpu(end_ext->ee_block) - 1, err); - if (orig_path == NULL) + if (err) goto out; if (ext4_ext_insert_extent(handle, orig_inode, @@ -632,12 +634,12 @@ mext_replace_branches(handle_t *handle, /* Get the original extent for the block "orig_off" */ get_ext_path(orig_path, orig_inode, orig_off, err); - if (orig_path == NULL) + if (err) goto out; /* Get the donor extent for the head */ get_ext_path(donor_path, donor_inode, donor_off, err); - if (donor_path == NULL) + if (err) goto out; depth = ext_depth(orig_inode); oext = orig_path[depth].p_ext; @@ -679,7 +681,7 @@ mext_replace_branches(handle_t *handle, if (orig_path) ext4_ext_drop_refs(orig_path); get_ext_path(orig_path, orig_inode, orig_off, err); - if (orig_path == NULL) + if (err) goto out; depth = ext_depth(orig_inode); oext = orig_path[depth].p_ext; @@ -694,7 +696,7 @@ mext_replace_branches(handle_t *handle, ext4_ext_drop_refs(donor_path); get_ext_path(donor_path, donor_inode, donor_off, err); - if (donor_path == NULL) + if (err) goto out; depth = ext_depth(donor_inode); dext = donor_path[depth].p_ext; @@ -1138,7 +1140,7 @@ ext4_move_extents(struct file *o_filp, s donor_start, &len, *moved_len); mext_double_up_read(orig_inode, donor_inode); if (ret) - goto out2; + goto out; file_end = (i_size_read(orig_inode) - 1) >> orig_inode->i_blkbits; block_end = block_start + len - 1; @@ -1146,20 +1148,16 @@ ext4_move_extents(struct file *o_filp, s len -= block_end - file_end; get_ext_path(orig_path, orig_inode, block_start, ret); - if (orig_path == NULL) - goto out2; + if (ret) + goto out; /* Get path structure to check the hole */ get_ext_path(holecheck_path, orig_inode, block_start, ret); - if (holecheck_path == NULL) + if (ret) goto out; depth = ext_depth(orig_inode); ext_cur = holecheck_path[depth].p_ext; - if (ext_cur == NULL) { - ret = -EINVAL; - goto out; - } /* * Get proper extent whose ee_block is beyond block_start @@ -1282,7 +1280,7 @@ ext4_move_extents(struct file *o_filp, s ext4_ext_drop_refs(holecheck_path); get_ext_path(holecheck_path, orig_inode, seq_start, ret); - if (holecheck_path == NULL) + if (ret) break; depth = holecheck_path->p_depth; @@ -1290,7 +1288,7 @@ ext4_move_extents(struct file *o_filp, s if (orig_path) ext4_ext_drop_refs(orig_path); get_ext_path(orig_path, orig_inode, seq_start, ret); - if (orig_path == NULL) + if (ret) break; ext_cur = holecheck_path[depth].p_ext; @@ -1307,7 +1305,7 @@ out: ext4_ext_drop_refs(holecheck_path); kfree(holecheck_path); } -out2: + mext_inode_double_unlock(orig_inode, donor_inode); if (ret)