| Message ID | 20210222100643.400935-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/compat: Clear bounce structures | expand |
On Mon, Feb 22, 2021 at 11:06:43AM +0100, Daniel Vetter wrote: > Some of them have gaps, or fields we don't clear. Native ioctl code > does full copies plus zero-extends on size mismatch, so nothing can > leak. But compat is more hand-rolled so need to be careful. > > None of these matter for performance, so just memset. > > Also I didn't fix up the CONFIG_DRM_LEGACY or CONFIG_DRM_AGP ioctl, those > are security holes anyway. > > Reported-by: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com # vblank ioctl > Cc: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Acked-by: Maxime Ripard <mripard@kernel.org> Maxime
On Thu, Feb 25, 2021 at 05:49:11PM +0100, Maxime Ripard wrote: > On Mon, Feb 22, 2021 at 11:06:43AM +0100, Daniel Vetter wrote: > > Some of them have gaps, or fields we don't clear. Native ioctl code > > does full copies plus zero-extends on size mismatch, so nothing can > > leak. But compat is more hand-rolled so need to be careful. > > > > None of these matter for performance, so just memset. > > > > Also I didn't fix up the CONFIG_DRM_LEGACY or CONFIG_DRM_AGP ioctl, those > > are security holes anyway. > > > > Reported-by: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com # vblank ioctl > > Cc: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Acked-by: Maxime Ripard <mripard@kernel.org> Merged to drm-misc-next, thanks for taking a look. -Daniel
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index f86448ab1fe0..dc734d4828a1 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -99,6 +99,8 @@ static int compat_drm_version(struct file *file, unsigned int cmd, if (copy_from_user(&v32, (void __user *)arg, sizeof(v32))) return -EFAULT; + memset(&v, 0, sizeof(v)); + v = (struct drm_version) { .name_len = v32.name_len, .name = compat_ptr(v32.name), @@ -137,6 +139,9 @@ static int compat_drm_getunique(struct file *file, unsigned int cmd, if (copy_from_user(&uq32, (void __user *)arg, sizeof(uq32))) return -EFAULT; + + memset(&uq, 0, sizeof(uq)); + uq = (struct drm_unique){ .unique_len = uq32.unique_len, .unique = compat_ptr(uq32.unique), @@ -265,6 +270,8 @@ static int compat_drm_getclient(struct file *file, unsigned int cmd, if (copy_from_user(&c32, argp, sizeof(c32))) return -EFAULT; + memset(&client, 0, sizeof(client)); + client.idx = c32.idx; err = drm_ioctl_kernel(file, drm_getclient, &client, 0); @@ -852,6 +859,8 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd, if (copy_from_user(&req32, argp, sizeof(req32))) return -EFAULT; + memset(&req, 0, sizeof(req)); + req.request.type = req32.request.type; req.request.sequence = req32.request.sequence; req.request.signal = req32.request.signal; @@ -889,6 +898,8 @@ static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd, struct drm_mode_fb_cmd2 req64; int err; + memset(&req64, 0, sizeof(req64)); + if (copy_from_user(&req64, argp, offsetof(drm_mode_fb_cmd232_t, modifier))) return -EFAULT;
Some of them have gaps, or fields we don't clear. Native ioctl code does full copies plus zero-extends on size mismatch, so nothing can leak. But compat is more hand-rolled so need to be careful. None of these matter for performance, so just memset. Also I didn't fix up the CONFIG_DRM_LEGACY or CONFIG_DRM_AGP ioctl, those are security holes anyway. Reported-by: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com # vblank ioctl Cc: syzbot+620cf21140fc7e772a5d@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_ioc32.c | 11 +++++++++++ 1 file changed, 11 insertions(+)