On Sun, Dec 20, 2020 at 12:36:23PM +0100, Arnd Bergmann wrote: > On Sun, Dec 20, 2020 at 12:06 PM Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > When an IOCTL with argument size larger than 128 that also used array > > arguments were handled, two memory allocations were made but alas, only > > the latter one of them was released. This happened because there was only > > a single local variable to hold such a temporary allocation. > > > > Fix this by adding separate variables to hold the pointers to the > > temporary allocations. > > > > Reported-by: Arnd Bergmann <arnd@xxxxxxxxxx> > > Reported-by: syzbot+1115e79c8df6472c612b@xxxxxxxxxxxxxxxxxxxxxxxxx > > Fixes: d14e6d76ebf7 ("[media] v4l: Add multi-planar ioctl handling code") > > Cc: stable@xxxxxxxxxxxxxxx > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > Acked-by: Arnd Bergmann <arnd@xxxxxxxx> Thanks! > > > out: > > + kvfree(array_buf); > > kvfree(mbuf); > > I think it would make sense to change mbuf back to kzalloc()/kfree > after this, since the size of the ioctl argument has an upper bound > of 2^_IOC_SIZEBITS (16KB), which does not need the vmalloc > path, unlike the array args. Good point. I can send a patch for that, too. -- Sakari Ailus