[Devel] dm: ploop: arithemtic overflow in ploop

[Alexey Kuznetsov]

May be we do. I do such mistakes all the time, as I am paranoid about
saving memory, using minimal possible type.
I checked code since this morning, this is the only place, which I
found. And it fixed this urgent problem.
Full audit can be done later, after everyone patched and damage is isolated.

General note. The code looks very good. It needs some testing, as we
see, but it is good and this fiasco
must not be interpreted as a sign of badness.

On Fri, May 10, 2024 at 9:09 PM Denis V. Lunev <den at virtuozzo.com> wrote:
>
> On 5/10/24 14:54, Alexey Kuznetsov wrote:
> > Images of size > 2TB are corrupted!
> >
> > https://pmc.acronis.work/browse/TTASK-68430
> >
> > Signed-off-by: Alexey Kuznetsov <kuznet at acronis.com>
> > ---
> >   drivers/md/dm-ploop.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-ploop.h b/drivers/md/dm-ploop.h
> > index db36687..e693d0e 100644
> > --- a/drivers/md/dm-ploop.h
> > +++ b/drivers/md/dm-ploop.h
> > @@ -317,7 +317,7 @@ static inline void ploop_remap_to_cluster(struct ploop *ploop,
> >                                         struct pio *pio, u32 clu)
> >   {
> >       pio->bi_iter.bi_sector &= ((1 << ploop->cluster_log) - 1);
> > -     pio->bi_iter.bi_sector |= (clu << ploop->cluster_log);
> > +     pio->bi_iter.bi_sector |= ((u64)clu << ploop->cluster_log);
> >   }
> >
> >   static inline bool ploop_whole_cluster(struct ploop *ploop, struct pio *pio)
> this is really integer overflow. Validated that with the simple
> test program.
>
> iris ~ $ cat 1.c
> #include <stdint.h>
> #include <stdio.h>
>
> struct s
> {
>      uint64_t x;
> };
>
> int main()
> {
>      uint32_t clu = 0x200000, log = 20;
>      struct s st = {
>          .x = 0,
>      };
>
>      st.x |= clu << log;
>      printf("%lu\n", st.x);
>      st.x |= (uint64_t)clu << log;
>      printf("%lu\n", st.x);
>      return 0;
> }
> iris ~ $ ./a.out
> 0
> 2199023255552
> iris ~ $
>
> The most important question is that do we have other similar
> places or not?
>
> Den