From patchwork Mon Sep 1 04:06:42 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Theodore Ts'o X-Patchwork-Id: 384632 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 59B28140087 for ; Mon, 1 Sep 2014 14:07:19 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752012AbaIAEHD (ORCPT ); Mon, 1 Sep 2014 00:07:03 -0400 Received: from imap.thunk.org ([74.207.234.97]:58475 "EHLO imap.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751542AbaIAEG5 (ORCPT ); Mon, 1 Sep 2014 00:06:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=thunk.org; s=ef5046eb; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=GuJp8z8i34r/4N+xzqZiX1jzmFw1fv+LZgL5UZ4I+e8=; b=eydqzXGMk7pVeYa11RlUM7sjeft+shcPdUwfAcTaR8PJ1cl3lXVhNPb2UrkKvI5eR0PtiHQrduRDPlPx2Qr+/j9hN7SrkGHJluK9qGV8AfHwgEvCgi9OY/15CxhAzPbGwU9A5WU3UXgrMw3bP+T0MvF82mFuqHfbHgti69+uDjc=; Received: from root (helo=closure.thunk.org) by imap.thunk.org with local-esmtp (Exim 4.80) (envelope-from ) id 1XOItL-0007Td-K5; Mon, 01 Sep 2014 04:06:55 +0000 Received: by closure.thunk.org (Postfix, from userid 15806) id 159F35812B1; Mon, 1 Sep 2014 00:06:55 -0400 (EDT) From: Theodore Ts'o To: Ext4 Developers List Cc: Theodore Ts'o Subject: [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error Date: Mon, 1 Sep 2014 00:06:42 -0400 Message-Id: <1409544411-31544-2-git-send-email-tytso@mit.edu> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1409544411-31544-1-git-send-email-tytso@mit.edu> References: <1409544411-31544-1-git-send-email-tytso@mit.edu> X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: tytso@thunk.org X-SA-Exim-Scanned: No (on imap.thunk.org); SAEximRunCond expanded to false Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Right now, there are a places where it is all to easy to leak memory on an error path, via a usage like this: struct ext4_ext_path *path = NULL while (...) { ... path = ext4_ext_find_extent(inode, block, path, 0); if (IS_ERR(path)) { /* oops, if path was non-NULL before the call to ext4_ext_find_extent, we've leaked it! :-( */ ... return PTR_ERR(path); } ... } Unfortunately, there some code paths where we are doing the following instead: path = ext4_ext_find_extent(inode, block, orig_path, 0); and where it's important that we _not_ free orig_path in the case where ext4_ext_find_extent() returns an error. So change the function signature of ext4_ext_find_extent() so that it takes a struct ext4_ext_path ** for its third argument, and by default, on an error, it will free the struct ext4_ext_path, and then zero out the struct ext4_ext_path * pointer. In order to avoid causing problems, we add a flag EXT4_EX_NOFREE_ON_ERR which causes ext4_ext_find_extent() to use the original behavior of forcing the caller to deal with freeing the original path pointer on the error case. The goal is to get rid of EXT4_EX_NOFREE_ON_ERR entirely, but this allows for a gentle transition and makes the patches easier to verify. Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 3 ++- fs/ext4/extents.c | 28 ++++++++++++++++++---------- fs/ext4/move_extent.c | 2 +- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 550b4f9..696e51a 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -582,6 +582,7 @@ enum { */ #define EXT4_EX_NOCACHE 0x0800 #define EXT4_EX_FORCE_CACHE 0x1000 +#define EXT4_EX_NOFREE_ON_ERR 0x2000 /* * Flags used by ext4_free_blocks @@ -2733,7 +2734,7 @@ extern int ext4_ext_insert_extent(handle_t *, struct inode *, struct ext4_ext_path *, struct ext4_extent *, int); extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t, - struct ext4_ext_path *, + struct ext4_ext_path **, int flags); extern void ext4_ext_drop_refs(struct ext4_ext_path *); extern int ext4_ext_check_inode(struct inode *inode); diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index bc3b49f..fc76be8 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -848,11 +848,13 @@ int ext4_ext_tree_init(handle_t *handle, struct inode *inode) struct ext4_ext_path * ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, - struct ext4_ext_path *path, int flags) + struct ext4_ext_path **orig_path, int flags) { struct ext4_extent_header *eh; struct buffer_head *bh; - short int depth, i, ppos = 0, alloc = 0; + struct ext4_ext_path *path = orig_path ? *orig_path : NULL; + short int depth, i, ppos = 0; + short free_on_err = (flags & EXT4_EX_NOFREE_ON_ERR) == 0; int ret; eh = ext_inode_hdr(inode); @@ -864,7 +866,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, GFP_NOFS); if (unlikely(!path)) return ERR_PTR(-ENOMEM); - alloc = 1; + free_on_err = 1; } path[0].p_hdr = eh; path[0].p_bh = NULL; @@ -916,8 +918,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, err: ext4_ext_drop_refs(path); - if (alloc) + if (free_on_err) { kfree(path); + if (orig_path) + *orig_path = NULL; + } return ERR_PTR(ret); } @@ -1349,7 +1354,7 @@ repeat: ext4_ext_drop_refs(path); path = ext4_ext_find_extent(inode, (ext4_lblk_t)le32_to_cpu(newext->ee_block), - path, gb_flags); + &path, gb_flags | EXT4_EX_NOFREE_ON_ERR); if (IS_ERR(path)) err = PTR_ERR(path); } else { @@ -1362,7 +1367,7 @@ repeat: ext4_ext_drop_refs(path); path = ext4_ext_find_extent(inode, (ext4_lblk_t)le32_to_cpu(newext->ee_block), - path, gb_flags); + &path, gb_flags | EXT4_EX_NOFREE_ON_ERR); if (IS_ERR(path)) { err = PTR_ERR(path); goto out; @@ -2145,7 +2150,7 @@ static int ext4_fill_fiemap_extents(struct inode *inode, path = NULL; } - path = ext4_ext_find_extent(inode, block, path, 0); + path = ext4_ext_find_extent(inode, block, &path, 0); if (IS_ERR(path)) { up_read(&EXT4_I(inode)->i_data_sem); err = PTR_ERR(path); @@ -3319,7 +3324,8 @@ static int ext4_split_extent(handle_t *handle, * result in split of original leaf or extent zeroout. */ ext4_ext_drop_refs(path); - path = ext4_ext_find_extent(inode, map->m_lblk, path, 0); + path = ext4_ext_find_extent(inode, map->m_lblk, &path, + EXT4_EX_NOFREE_ON_ERR); if (IS_ERR(path)) return PTR_ERR(path); depth = ext_depth(inode); @@ -3703,7 +3709,8 @@ static int ext4_convert_initialized_extents(handle_t *handle, if (err < 0) goto out; ext4_ext_drop_refs(path); - path = ext4_ext_find_extent(inode, map->m_lblk, path, 0); + path = ext4_ext_find_extent(inode, map->m_lblk, &path, + EXT4_EX_NOFREE_ON_ERR); if (IS_ERR(path)) { err = PTR_ERR(path); goto out; @@ -3775,7 +3782,8 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle, if (err < 0) goto out; ext4_ext_drop_refs(path); - path = ext4_ext_find_extent(inode, map->m_lblk, path, 0); + path = ext4_ext_find_extent(inode, map->m_lblk, &path, + EXT4_EX_NOFREE_ON_ERR); if (IS_ERR(path)) { err = PTR_ERR(path); goto out; diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c index c8f895b..5e2465a 100644 --- a/fs/ext4/move_extent.c +++ b/fs/ext4/move_extent.c @@ -37,7 +37,7 @@ get_ext_path(struct inode *inode, ext4_lblk_t lblock, int ret = 0; struct ext4_ext_path *path; - path = ext4_ext_find_extent(inode, lblock, *orig_path, EXT4_EX_NOCACHE); + path = ext4_ext_find_extent(inode, lblock, orig_path, EXT4_EX_NOCACHE); if (IS_ERR(path)) ret = PTR_ERR(path); else if (path[ext_depth(inode)].p_ext == NULL)