Re: [Devel] [PATCH hci-8.1 v2 3/6] dm-ploop: set delta is_raw before applying its mappings

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

> [Severity: Low]
> With this change the raw branch of ploop_apply_delta_mappings() becomes
> reachable, and it looks like it can misbehave for a zero-length raw base
> delta.
>
> For a raw delta, nr_be comes straight from the file size:
>
> ploop_add_delta()
>      nr_be = POS_TO_CLU(ploop, file_size);
>
> ploop_check_delta_length() only rejects sizes that are not cluster
> aligned, so a size of 0 passes. The check against deltas[level + 1].nr_be
> cannot fail for 0 either.
>
> Then, with nr_be == 0, init_be_iter() computes:
>
>      u32 rem, last_page = ploop_bat_clu_to_page_nr(nr_be - 1);
>
> nr_be - 1 is 0xFFFFFFFF, and the u32 wrap in ploop_bat_clu_to_page_nr()
> gives last_page = 0. Page 0 then gets start=16 and end=15 and is
> skipped, while every other page iterates in full.
>
> In ploop_apply_delta_mappings() the stop condition can never trigger:
>
>      if (clu == nr_be - 1)
>          stop = true;
>      ...
>      dst_clu = clu;
>
> so can identity mappings be written at the raw level for every
> unpopulated slot on pages above 0, past EOF of the raw file, including
> tail slots of the last page beyond nr_bat_entries?
>
> Should ploop_add_delta() reject nr_be == 0 for a raw delta (or should
> init_be_iter() handle it)?
>
> The BAT arrays do not appear to be overrun since the loop stays within
> each md page. Reads of those clusters would hit EOF on the raw file and
> likely return zeros, like a hole. I did not trace the write/COW path for
> these mappings. Reaching this requires a table load with an empty
> "raw@fd" base, so it needs CAP_SYS_ADMIN and is an unusual
> misconfiguration. I did not find a check for this in later patches of
> the series.
Fixed by putting ploop_apply_delta_mappings() call under if (nr_be) 
condition.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel