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

Bill O'Donnell
2024-08-30T15:48:46Z

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
>