From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753648AbdEQHBX (ORCPT ); Wed, 17 May 2017 03:01:23 -0400 Received: from b.ns.miles-group.at ([95.130.255.144]:44724 "EHLO radon.swed.at" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753554AbdEQHBU (ORCPT ); Wed, 17 May 2017 03:01:20 -0400 Subject: Re: [PATCH 2/2] ubifs: Fix memory leak in RENAME_WHITEOUT error path in do_rename To: Hyunchul Lee References: <1494979082-19414-1-git-send-email-cheol.lee@lge.com> Cc: Artem Bityutskiy , adrian.hunter@intel.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mtd@lists.infradead.org, kernel-team@lge.com From: Richard Weinberger Message-ID: Date: Wed, 17 May 2017 09:01:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <1494979082-19414-1-git-send-email-cheol.lee@lge.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hyunchul, Am 17.05.2017 um 01:58 schrieb Hyunchul Lee: > in RENAME_WHITEOUT error path, fscrypt_name should be freed. > > Signed-off-by: Hyunchul Lee > --- > fs/ubifs/dir.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index df67236..0d5f8e7 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -1385,17 +1385,14 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry, > > dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS); > if (!dev) { > - ubifs_release_budget(c, &req); > - ubifs_release_budget(c, &ino_req); > - return -ENOMEM; > + err = -ENOMEM; > + goto out_release; > } > > err = do_tmpfile(old_dir, old_dentry, S_IFCHR | WHITEOUT_MODE, &whiteout); > if (err) { > - ubifs_release_budget(c, &req); > - ubifs_release_budget(c, &ino_req); > kfree(dev); > - return err; > + goto out_release; > } > > whiteout->i_state |= I_LINKABLE; > @@ -1483,12 +1480,10 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry, > > err = ubifs_budget_space(c, &wht_req); > if (err) { > - ubifs_release_budget(c, &req); > - ubifs_release_budget(c, &ino_req); > kfree(whiteout_ui->data); > whiteout_ui->data_len = 0; > iput(whiteout); > - return err; > + goto out_release; > } > > inc_nlink(whiteout); > @@ -1543,6 +1538,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry, > iput(whiteout); > } > unlock_4_inodes(old_dir, new_dir, new_inode, whiteout); > +out_release: > ubifs_release_budget(c, &ino_req); > ubifs_release_budget(c, &req); > fscrypt_free_filename(&old_nm); > Good find too. :-) This function needs a cleanup, it is much longer than it should be. Thanks, //richard