Re: [PATCH] dm ioctl: don't let data_start run past the buffer

[Raghunath Palla]

Hi Mikulas,

Thanks for applying the patch and for the cleanup. The change looks good
to me.

Thanks,
Palla Raghunath


On Mon, 21 Sept 2026 at 15:29, Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
>
>
> On Sat, 19 Sep 2026, Palla Raghunath wrote:
>
> > struct dm_ioctl has seven bytes of trailing padding, so the smallest
> > buffer userspace is allowed to ask for, offsetof(struct dm_ioctl, data),
> > is seven bytes shorter than sizeof(struct dm_ioctl).
> >
> > get_result_buffer() sets data_start to sizeof(struct dm_ioctl) without
> > looking at how big the buffer actually is. Ask for the minimum and
> > data_start ends up seven bytes past the end. Callers add it to
> > data_size, and ctl_ioctl() copies that much back to userspace:
> >
> >   BUG: KASAN: slab-out-of-bounds in _copy_to_user+0xad/0xd0
> >   Read of size 312 at addr ffff8880169e4c00 by task repro-dm/140
> >    ctl_ioctl+0x5e3/0xcb0
> >    dm_ctl_ioctl+0x25/0x40
> >   allocated 305-byte region [ffff8880169e4c00, ffff8880169e4d31)
> >
> > A DM_LIST_DEVICES with data_size set to 305 is enough to hit it, and the
> > seven bytes that leak are whatever sat next to the allocation.
> >
> > Clamp data_start to the buffer size.
> >
> > Reported-by: syzbot+48d935cf48a4a76be346@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=48d935cf48a4a76be346
> > Fixes: 76c072b48e39 ("dm ioctl: move compat code")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Palla Raghunath <raghunathpalla.0209@xxxxxxxxx>
> > ---
> >  drivers/md/dm-ioctl.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > index a6b8e97755cd..211569dd3c54 100644
> > --- a/drivers/md/dm-ioctl.c
> > +++ b/drivers/md/dm-ioctl.c
> > @@ -656,6 +656,13 @@ static void *get_result_buffer(struct dm_ioctl *param, size_t param_size,
> >  {
> >       param->data_start = align_ptr(param + 1) - (void *) param;
> >
> > +     /*
> > +      * The buffer can be as small as offsetof(*param, data), which is
> > +      * less than sizeof(*param), so don't run off the end of it.
> > +      */
> > +     if (param->data_start > param_size)
> > +             param->data_start = param_size;
> > +
> >       if (param->data_start < param_size)
> >               *len = param_size - param->data_start;
> >       else
> > --
> > 2.34.1
>
> Hi
>
> I accepted the patch, but I changed it slightly, so that there is just one
> condition in get_result_buffer. The resulting function is:
>
> static void *get_result_buffer(struct dm_ioctl *param, size_t param_size,
>                                size_t *len)
> {
>         param->data_start = align_ptr(param + 1) - (void *) param;
>
>         if (param->data_start < param_size) {
>                 *len = param_size - param->data_start;
>         } else {
>                 /*
>                  * The buffer can be as small as offsetof(*param, data), which
>                  * is less than sizeof(*param), so don't run off the end of it.
>                  */
>                 *len = 0;
>                 param->data_start = param_size;
>         }
>
>         return ((void *) param) + param->data_start;
> }
>
> Mikulas
>