From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9C6DC432BE for ; Fri, 27 Aug 2021 17:47:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8EBB960FD9 for ; Fri, 27 Aug 2021 17:47:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239095AbhH0Rs2 (ORCPT ); Fri, 27 Aug 2021 13:48:28 -0400 Received: from relaydlg-01.paragon-software.com ([81.5.88.159]:39395 "EHLO relaydlg-01.paragon-software.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236883AbhH0RsX (ORCPT ); Fri, 27 Aug 2021 13:48:23 -0400 Received: from dlg2.mail.paragon-software.com (vdlg-exch-02.paragon-software.com [172.30.1.105]) by relaydlg-01.paragon-software.com (Postfix) with ESMTPS id 485D08221E; Fri, 27 Aug 2021 20:47:28 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paragon-software.com; s=mail; t=1630086448; bh=cQHGVALsR0wITXNG82rls7Ltz5TIhHWjctlfp9KdpL0=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=laIFR4i5oregS0eTBrhg8j22S1NlqhDlfjJP0n0L+30rksGU3LS9A2uD0CQxcrLEe 4Jy1rMkhIubwvkzpazqphNmVoTdsJ/TwQOQoTy+4KCZ97mRzqx33zzdDWGIdN/Hj+i TlgUuf738m/x5Rx0sVdjnIhBRI/r9xtgU+X6lF0I= Received: from [192.168.211.135] (192.168.211.135) by vdlg-exch-02.paragon-software.com (172.30.1.105) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Fri, 27 Aug 2021 20:47:27 +0300 Subject: Re: [PATCH 3/3] fs/ntfs3: Fix error handling in indx_insert_into_root() To: Dan Carpenter CC: , , References: <20210824075103.GC13096@kili> From: Konstantin Komarov Message-ID: Date: Fri, 27 Aug 2021 20:47:27 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20210824075103.GC13096@kili> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [192.168.211.135] X-ClientProxiedBy: vdlg-exch-02.paragon-software.com (172.30.1.105) To vdlg-exch-02.paragon-software.com (172.30.1.105) Precedence: bulk List-ID: X-Mailing-List: kernel-janitors@vger.kernel.org On 24.08.2021 10:51, Dan Carpenter wrote: > There are three bugs in this code: > 1) If indx_get_root() fails, then return -EINVAL instead of success. > 2) On the "/* make root external */" -EOPNOTSUPP; error path it should > free "re" but it has a memory leak. > 3) If indx_new() fails then it will lead to an error pointer dereference > when we call put_indx_node(). > > I've re-written the error handling to be more clear. > > Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") > Signed-off-by: Dan Carpenter > --- > fs/ntfs3/index.c | 36 ++++++++++++++++-------------------- > 1 file changed, 16 insertions(+), 20 deletions(-) > > diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c > index 489e0fffbc75..4f2d24010386 100644 > --- a/fs/ntfs3/index.c > +++ b/fs/ntfs3/index.c > @@ -1554,12 +1554,12 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > u32 root_size, new_root_size; > struct ntfs_sb_info *sbi; > int ds_root; > - struct INDEX_ROOT *root, *a_root = NULL; > + struct INDEX_ROOT *root, *a_root; > > /* Get the record this root placed in */ > root = indx_get_root(indx, ni, &attr, &mi); > if (!root) > - goto out; > + return -EINVAL; > > /* > * Try easy case: > @@ -1591,10 +1591,8 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > > /* Make a copy of root attribute to restore if error */ > a_root = ntfs_memdup(attr, asize); > - if (!a_root) { > - err = -ENOMEM; > - goto out; > - } > + if (!a_root) > + return -ENOMEM; > > /* copy all the non-end entries from the index root to the new buffer.*/ > to_move = 0; > @@ -1604,7 +1602,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > for (e = e0;; e = hdr_next_de(hdr, e)) { > if (!e) { > err = -EINVAL; > - goto out; > + goto out_free_root; > } > > if (de_is_last(e)) > @@ -1612,14 +1610,13 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > to_move += le16_to_cpu(e->size); > } > > - n = NULL; > if (!to_move) { > re = NULL; > } else { > re = ntfs_memdup(e0, to_move); > if (!re) { > err = -ENOMEM; > - goto out; > + goto out_free_root; > } > } > > @@ -1636,7 +1633,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > if (ds_root > 0 && used + ds_root > sbi->max_bytes_per_attr) { > /* make root external */ > err = -EOPNOTSUPP; > - goto out; > + goto out_free_re; > } > > if (ds_root) > @@ -1666,7 +1663,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > /* bug? */ > ntfs_set_state(sbi, NTFS_DIRTY_ERROR); > err = -EINVAL; > - goto out1; > + goto out_free_re; > } > > if (err) { > @@ -1677,7 +1674,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > /* bug? */ > ntfs_set_state(sbi, NTFS_DIRTY_ERROR); > } > - goto out1; > + goto out_free_re; > } > > e = (struct NTFS_DE *)(root + 1); > @@ -1688,7 +1685,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > n = indx_new(indx, ni, new_vbn, sub_vbn); > if (IS_ERR(n)) { > err = PTR_ERR(n); > - goto out1; > + goto out_free_re; > } > > hdr = &n->index->ihdr; > @@ -1715,7 +1712,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > put_indx_node(n); > fnd_clear(fnd); > err = indx_insert_entry(indx, ni, new_de, ctx, fnd); > - goto out; > + goto out_free_root; > } > > /* > @@ -1725,7 +1722,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > e = hdr_insert_de(indx, hdr, new_de, NULL, ctx); > if (!e) { > err = -EINVAL; > - goto out1; > + goto out_put_n; > } > fnd_push(fnd, n, e); > > @@ -1734,12 +1731,11 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > > n = NULL; > > -out1: > +out_put_n: > + put_indx_node(n); > +out_free_re: > ntfs_free(re); > - if (n) > - put_indx_node(n); > - > -out: > +out_free_root: > ntfs_free(a_root); > return err; > } > Hi, Dan! Applied all 3 patches, thanks!