LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: <ntfs3@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
	<kernel-janitors@vger.kernel.org>
Subject: Re: [PATCH 3/3] fs/ntfs3: Fix error handling in indx_insert_into_root()
Date: Fri, 27 Aug 2021 20:47:27 +0300	[thread overview]
Message-ID: <a154fe47-8634-415c-2f53-8dcd3ee67665@paragon-software.com> (raw)
In-Reply-To: <20210824075103.GC13096@kili>



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 <dan.carpenter@oracle.com>
> ---
>  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!

  parent reply	other threads:[~2021-08-27 17:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24  7:49 [PATCH 1/3] fs/ntfs3: Fix error code in indx_add_allocate() Dan Carpenter
2021-08-24  7:50 ` [PATCH 2/3] fs/ntfs3: Potential NULL dereference in hdr_find_split() Dan Carpenter
2021-08-24  9:35   ` Kari Argillander
2021-08-24  7:51 ` [PATCH 3/3] fs/ntfs3: Fix error handling in indx_insert_into_root() Dan Carpenter
2021-08-24 10:22   ` Kari Argillander
2021-08-27 17:47   ` Konstantin Komarov [this message]
2021-08-24  9:03 ` [PATCH 1/3] fs/ntfs3: Fix error code in indx_add_allocate() Kari Argillander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a154fe47-8634-415c-2f53-8dcd3ee67665@paragon-software.com \
    --to=almaz.alexandrovich@paragon-software.com \
    --cc=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --subject='Re: [PATCH 3/3] fs/ntfs3: Fix error handling in indx_insert_into_root()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).