2018-09-04 04:18:22

by Larry Chen

[permalink] [raw]
Subject: [PATCH 0/2] fix cluster leakage in ocfs2_defrag_extent

ocfs2_defrag_extent might leak clusters allocated.
When file system has no enough space, the number of claimed clusters
might less than the caller wants. If that happens, the original code
might directly commit trans without returning clusters.

This patch refered a lot to ocfs2_add_clusters_in_btree.

Larry Chen (2):
add declaration of ocfs2_free_local_alloc_bits
fix clusters leak in ocfs2_defrag_extent

fs/ocfs2/alloc.h | 6 ++++++
fs/ocfs2/move_extents.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+)

--
2.13.7



2018-09-04 04:18:10

by Larry Chen

[permalink] [raw]
Subject: [PATCH 1/2] add declaration of ocfs2_free_local_alloc_bits

Signed-off-by: Larry Chen <[email protected]>
---
fs/ocfs2/alloc.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/fs/ocfs2/alloc.h b/fs/ocfs2/alloc.h
index 250bcacdf9e9..5770503b0e36 100644
--- a/fs/ocfs2/alloc.h
+++ b/fs/ocfs2/alloc.h
@@ -323,4 +323,10 @@ int ocfs2_find_cpos_for_left_leaf(struct super_block *sb,
int ocfs2_find_subtree_root(struct ocfs2_extent_tree *et,
struct ocfs2_path *left,
struct ocfs2_path *right);
+
+int ocfs2_free_local_alloc_bits(struct ocfs2_super *osb,
+ handle_t *handle,
+ struct ocfs2_alloc_context *ac,
+ u32 bit_off,
+ u32 num_bits);
#endif /* OCFS2_ALLOC_H */
--
2.13.7


2018-09-04 04:18:38

by Larry Chen

[permalink] [raw]
Subject: [PATCH 2/2] fix clusters leak in ocfs2_defrag_extent

Signed-off-by: Larry Chen <[email protected]>
---
fs/ocfs2/move_extents.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
index d85dc8a02bd6..4f2ad054b419 100644
--- a/fs/ocfs2/move_extents.c
+++ b/fs/ocfs2/move_extents.c
@@ -226,6 +226,8 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
struct ocfs2_refcount_tree *ref_tree = NULL;
u32 new_phys_cpos, new_len;
u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
+ int need_free = 0;
+ struct ocfs2_alloc_context *data_ac;

if ((ext_flags & OCFS2_EXT_REFCOUNTED) && *len) {
BUG_ON(!ocfs2_is_refcount_inode(inode));
@@ -317,6 +319,7 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
if (!partial) {
context->range->me_flags &= ~OCFS2_MOVE_EXT_FL_COMPLETE;
ret = -ENOSPC;
+ need_free = 1;
goto out_commit;
}
}
@@ -341,6 +344,19 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
mlog_errno(ret);

out_commit:
+ if (need_free && context->data_ac) {
+ data_ac = context->data_ac;
+ if (context->data_ac->ac_which == OCFS2_AC_USE_LOCAL)
+ ocfs2_free_local_alloc_bits(osb, handle, data_ac,
+ new_phys_cpos, new_len);
+ else
+ ocfs2_free_clusters(handle,
+ data_ac->ac_inode,
+ data_ac->ac_bh,
+ ocfs2_clusters_to_blocks(osb->sb, new_phys_cpos),
+ new_len);
+ }
+
ocfs2_commit_trans(osb, handle);

out_unlock_mutex:
--
2.13.7


2018-09-04 21:58:08

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/2] add declaration of ocfs2_free_local_alloc_bits

On Tue, 4 Sep 2018 12:16:20 +0800 Larry Chen <[email protected]> wrote:

> Signed-off-by: Larry Chen <[email protected]>
> ---
> fs/ocfs2/alloc.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/ocfs2/alloc.h b/fs/ocfs2/alloc.h
> index 250bcacdf9e9..5770503b0e36 100644
> --- a/fs/ocfs2/alloc.h
> +++ b/fs/ocfs2/alloc.h
> @@ -323,4 +323,10 @@ int ocfs2_find_cpos_for_left_leaf(struct super_block *sb,
> int ocfs2_find_subtree_root(struct ocfs2_extent_tree *et,
> struct ocfs2_path *left,
> struct ocfs2_path *right);
> +
> +int ocfs2_free_local_alloc_bits(struct ocfs2_super *osb,
> + handle_t *handle,
> + struct ocfs2_alloc_context *ac,
> + u32 bit_off,
> + u32 num_bits);
> #endif /* OCFS2_ALLOC_H */

It's already declared in fs/ocfs2/localalloc.h - why not include that?

I did this with the other patch and all seems well:


From: Andrew Morton <[email protected]>
Subject: fix-clusters-leak-in-ocfs2_defrag_extent-fix

include localalloc.h, reduce scope of data_ac

Cc: Larry Chen <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

fs/ocfs2/move_extents.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

--- a/fs/ocfs2/move_extents.c~fix-clusters-leak-in-ocfs2_defrag_extent-fix
+++ a/fs/ocfs2/move_extents.c
@@ -25,6 +25,7 @@
#include "ocfs2_ioctl.h"

#include "alloc.h"
+#include "localalloc.h"
#include "aops.h"
#include "dlmglue.h"
#include "extent_map.h"
@@ -227,7 +228,6 @@ static int ocfs2_defrag_extent(struct oc
u32 new_phys_cpos, new_len;
u64 phys_blkno = ocfs2_clusters_to_blocks(inode->i_sb, phys_cpos);
int need_free = 0;
- struct ocfs2_alloc_context *data_ac;

if ((ext_flags & OCFS2_EXT_REFCOUNTED) && *len) {
BUG_ON(!ocfs2_is_refcount_inode(inode));
@@ -345,7 +345,8 @@ static int ocfs2_defrag_extent(struct oc

out_commit:
if (need_free && context->data_ac) {
- data_ac = context->data_ac;
+ struct ocfs2_alloc_context *data_ac = context->data_ac;
+
if (context->data_ac->ac_which == OCFS2_AC_USE_LOCAL)
ocfs2_free_local_alloc_bits(osb, handle, data_ac,
new_phys_cpos, new_len);
_