ext4: teach ext4_ext_find_extent() to free path on error
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 <tytso@mit.edu>
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index bf205f7..0ced78c 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -855,11 +855,13 @@
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);
@@ -871,7 +873,7 @@
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;
@@ -923,8 +925,11 @@
err:
ext4_ext_drop_refs(path);
- if (alloc)
+ if (free_on_err) {
kfree(path);
+ if (orig_path)
+ *orig_path = NULL;
+ }
return ERR_PTR(ret);
}
@@ -1356,7 +1361,7 @@
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 {
@@ -1369,7 +1374,7 @@
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;
@@ -2152,7 +2157,7 @@
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);
@@ -3313,7 +3318,8 @@
* 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);
@@ -3697,7 +3703,8 @@
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;
@@ -3769,7 +3776,8 @@
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;