Re: [PATCH] xfsdump: prevent use-after-free in fstab_commit()

[Bill O'Donnell <bodonnel@redhat.com>]

On Thu, Aug 29, 2024 at 12:59:25PM -0500, Bill O'Donnell wrote:
> Fix potential use-after-free of list pointer in fstab_commit().
> Use a copy of the pointer when calling invidx_commit().
> 
> Coverity CID 1498039.
> 
> Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>

NACK. Deeming this finding as a false positive.

> ---
>  invutil/fstab.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/invutil/fstab.c b/invutil/fstab.c
> index 88d849e..fe2b1f9 100644
> --- a/invutil/fstab.c
> +++ b/invutil/fstab.c
> @@ -66,6 +66,7 @@ fstab_commit(WINDOW *win, node_t *current, node_t *list)
>      data_t *d;
>      invt_fstab_t *fstabentry;
>      int fstabentry_idx;
> +    node_t *list_cpy = list;
>  
>      n = current;
>      if(n == NULL || n->data == NULL)
> @@ -77,8 +78,10 @@ fstab_commit(WINDOW *win, node_t *current, node_t *list)
>  
>      if(d->deleted == BOOL_TRUE && d->imported == BOOL_FALSE) {
>  	for(i = 0; i < d->nbr_children; i++) {
> -	    invidx_commit(win, d->children[i], list);
> +		list_cpy = list;
> +		invidx_commit(win, d->children[i], list_cpy);
>  	}
> +	free(list_cpy);
>  	mark_all_children_commited(current);
>  
>  	fstabentry_idx = (int)(((long)fstabentry - (long)fstab_file[fidx].mapaddr - sizeof(invt_counter_t)) / sizeof(invt_fstab_t));
> @@ -101,8 +104,10 @@ fstab_commit(WINDOW *win, node_t *current, node_t *list)
>  	invt_fstab_t *dest;
>  
>  	for(i = 0; i < d->nbr_children; i++) {
> -	    invidx_commit(win, d->children[i], list);
> +		list_cpy = list;
> +		invidx_commit(win, d->children[i], list_cpy);
>  	}
> +	free(list_cpy);
>  	mark_all_children_commited(current);
>  
>  	if(find_matching_fstab(0, fstabentry) >= 0) {
> -- 
> 2.46.0
>