| Message ID | 20210908024605.16857-1-cymi20@fudan.edu.cn |
|---|---|
| State | Rejected |
| Headers | show |
| Series | ext4: Fix refcount leak bug in __ext4_new_inode() | expand |
On Wed 08-09-21 10:46:05, Chenyuan Mi wrote: > After successfully creating handle by __ext4_journal_start_sb(), > the function forgets to decrease the refcount of handle in several > paths, causing refcount leak. > > Fix this issue by recording a flag when successfully getting handle > by __ext4_journal_start_sb(), and decrease the refcount of handle > when exiting this function if holding the flag. > > Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn> > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> I'm sorry but this patch is wrong. We are expected to return with handle running from __ext4_new_inode(). Please have a look into callers of ext4_new_inode() and ext4_new_inode_start_handle() how they use and then stop the handle. And similarly as for the second patch I'd note that even a small amount of testing would show you that this patch does not work. Honza > > --- > fs/ext4/ialloc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c > index f73e5eb43eae..7563b892c64f 100644 > --- a/fs/ext4/ialloc.c > +++ b/fs/ext4/ialloc.c > @@ -944,6 +944,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, > ext4_group_t flex_group; > struct ext4_group_info *grp = NULL; > bool encrypt = false; > + bool create_handle = false; > > /* Cannot create files in a deleted directory */ > if (!dir || !dir->i_nlink) > @@ -1085,6 +1086,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, > ext4_std_error(sb, err); > goto out; > } > + create_handle = true; > } > BUFFER_TRACE(inode_bitmap_bh, "get_write_access"); > err = ext4_journal_get_write_access(handle, sb, inode_bitmap_bh, > @@ -1345,7 +1347,8 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, > ext4_std_error(sb, err); > goto fail_free_drop; > } > - > + if (create_handle) > + ext4_journal_stop(handle); > ext4_debug("allocating inode %lu\n", inode->i_ino); > trace_ext4_allocate_inode(inode, dir, mode); > brelse(inode_bitmap_bh); > @@ -1357,6 +1360,8 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, > clear_nlink(inode); > unlock_new_inode(inode); > out: > + if (create_handle) > + ext4_journal_stop(handle); > dquot_drop(inode); > inode->i_flags |= S_NOQUOTA; > iput(inode); > -- > 2.17.1 >
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index f73e5eb43eae..7563b892c64f 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -944,6 +944,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, ext4_group_t flex_group; struct ext4_group_info *grp = NULL; bool encrypt = false; + bool create_handle = false; /* Cannot create files in a deleted directory */ if (!dir || !dir->i_nlink) @@ -1085,6 +1086,7 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, ext4_std_error(sb, err); goto out; } + create_handle = true; } BUFFER_TRACE(inode_bitmap_bh, "get_write_access"); err = ext4_journal_get_write_access(handle, sb, inode_bitmap_bh, @@ -1345,7 +1347,8 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, ext4_std_error(sb, err); goto fail_free_drop; } - + if (create_handle) + ext4_journal_stop(handle); ext4_debug("allocating inode %lu\n", inode->i_ino); trace_ext4_allocate_inode(inode, dir, mode); brelse(inode_bitmap_bh); @@ -1357,6 +1360,8 @@ struct inode *__ext4_new_inode(struct user_namespace *mnt_userns, clear_nlink(inode); unlock_new_inode(inode); out: + if (create_handle) + ext4_journal_stop(handle); dquot_drop(inode); inode->i_flags |= S_NOQUOTA; iput(inode);