On 8/29/24 12:59 PM, 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().
I'm not sure how (or even if) the use after free happens -xfsdump is so hard
to read - but ...
> Coverity CID 1498039.
>
> Signed-off-by: Bill O'Donnell <bodonnel@xxxxxxxxxx>
> ---
> 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;
this copies the memory address stored in "list" into your new pointer var "list_cpy"
> + invidx_commit(win, d->children[i], list_cpy);
If invidx_commit() frees the 2nd argument, it frees the memory address pointed
to by both list and list_cpy.
Storing the same memory address in 2 pointer variables does not prevent that memory
from being freed.
> }
> + free(list_cpy);
and then this would double-free that same memory address.
> 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) {
