linux-kernel - Re: [PATCH] jffs2: fix use after free in jffs2_sum_write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210104221607.GA1985645@ubuntu-m3-large-x86>
Date:   Mon, 4 Jan 2021 15:16:07 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     trix@...hat.com
Cc:     dwmw2@...radead.org, richard@....at, ndesaulniers@...gle.com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        clang-built-linux@...glegroups.com
Subject: Re: [PATCH] jffs2: fix use after free in jffs2_sum_write_data()

On Wed, Dec 30, 2020 at 06:56:04AM -0800, trix@...hat.com wrote:
> From: Tom Rix <trix@...hat.com>
> 
> clang static analysis reports this problem
> 
> fs/jffs2/summary.c:794:31: warning: Use of memory after it is freed
>                 c->summary->sum_list_head = temp->u.next;
>                                             ^~~~~~~~~~~~
> 
> In jffs2_sum_write_data(), in a loop summary data is handles a node at
> a time.  When it has written out the node it is removed the summary list,
> and the node is deleted.  In the corner case when a
> JFFS2_FEATURE_RWCOMPAT_COPY is seen, a call is made to
> jffs2_sum_disable_collecting().  jffs2_sum_disable_collecting() deletes
> the whole list which conflicts with the loop's deleting the list by parts.
> 
> To preserve the old behavior of stopping the write midway, bail out of
> the loop after disabling summary collection.
> 
> Fixes: 6171586a7ae5 ("[JFFS2] Correct handling of JFFS2_FEATURE_RWCOMPAT_COPY nodes.")
> Signed-off-by: Tom Rix <trix@...hat.com>

Reviewed-by: Nathan Chancellor <natechancellor@...il.com>

> ---
>  fs/jffs2/summary.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
> index be7c8a6a5748..4fe64519870f 100644
> --- a/fs/jffs2/summary.c
> +++ b/fs/jffs2/summary.c
> @@ -783,6 +783,8 @@ static int jffs2_sum_write_data(struct jffs2_sb_info *c, struct jffs2_eraseblock
>  					dbg_summary("Writing unknown RWCOMPAT_COPY node type %x\n",
>  						    je16_to_cpu(temp->u.nodetype));
>  					jffs2_sum_disable_collecting(c->summary);
> +					/* The above call removes the list, nothing more to do */
> +					goto bail_rwcompat;
>  				} else {
>  					BUG();	/* unknown node in summary information */
>  				}
> @@ -794,6 +796,7 @@ static int jffs2_sum_write_data(struct jffs2_sb_info *c, struct jffs2_eraseblock
>  
>  		c->summary->sum_num--;
>  	}
> + bail_rwcompat:
>  
>  	jffs2_sum_reset_collected(c->summary);
>  
> -- 
> 2.27.0
>