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 <[email protected]> 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: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=48d935cf48a4a76be346 > > Fixes: 76c072b48e39 ("dm ioctl: move compat code") > > Cc: [email protected] > > Signed-off-by: Palla Raghunath <[email protected]> > > --- > > 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 >
