Bug 231513 - off-by-one overflow in drm_ioctl (sys/dev/drm/drm_drv.c)
Summary: off-by-one overflow in drm_ioctl (sys/dev/drm/drm_drv.c)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Oleksandr Tymoshenko
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2018-09-20 13:17 UTC by Young
Modified: 2019-02-03 14:57 UTC (History)
3 users (show)

See Also:


Attachments
patch_for_drm_out-of-bouns_access (1.05 KB, application/mbox)
2018-09-20 13:17 UTC, Young
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Young 2018-09-20 13:17:01 UTC
Created attachment 197277 [details]
patch_for_drm_out-of-bouns_access

There is a off-by-one overflow in drm_ioctl (sys/dev/drm/drm_drv.c).

701 int drm_ioctl(struct cdev *kdev, u_long cmd, caddr_t data, int flags,
702     DRM_STRUCTPROC *p)
703 {
704         struct drm_device *dev = drm_get_device_from_kdev(kdev);
705         int retcode = 0;
706         drm_ioctl_desc_t *ioctl;
707         int (*func)(struct drm_device *dev, void *data, struct drm_file *file_priv);
708         int nr = DRM_IOCTL_NR(cmd);
709         int is_driver_ioctl = 0;
710         struct drm_file *file_priv;
711 
... 
743         ioctl = &drm_ioctls[nr];
744         /* It's not a core DRM ioctl, try driver-specific. */
745         if (ioctl->func == NULL && nr >= DRM_COMMAND_BASE) {
746                 /* The array entries begin at DRM_COMMAND_BASE ioctl nr */
747                 nr -= DRM_COMMAND_BASE;
748                 if (nr > dev->driver->max_ioctl) {
749                         DRM_DEBUG("Bad driver ioctl number, 0x%x (of 0x%x)\n",
750                             nr, dev->driver->max_ioctl);
751                         return EINVAL;
752                 }
753                 ioctl = &dev->driver->ioctls[nr];
754                 is_driver_ioctl = 1;
755         }
756         func = ioctl->func;
757 
758         if (func == NULL) {
759                 DRM_DEBUG("no function\n");
760                 return EINVAL;
761         }
762 
763         if (((ioctl->flags & DRM_ROOT_ONLY) && !DRM_SUSER(p)) ||
764             ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) ||
765             ((ioctl->flags & DRM_MASTER) && !file_priv->master))
766                 return EACCES;
767 
768         if (is_driver_ioctl) {
769                 DRM_LOCK();
770                 /* shared code returns -errno */
771                 retcode = -func(dev, data, file_priv);
772                 DRM_UNLOCK();
773         } else {
774                 retcode = func(dev, data, file_priv);
775         }
776 
777         if (retcode != 0)
778                 DRM_DEBUG("    returning %d\n", retcode);
779 
780         return retcode;
781 }

The correct logic for line 748 is nr >= dev->driver->max_ioctl. Otherwise, there would be out-of-bounds access in line 753. Then, ioctl is used in line 756.

The attachment is the proposal patch.
Comment 1 Young 2018-11-01 12:32:08 UTC
ping
Comment 2 Warner Losh freebsd_committer freebsd_triage 2019-01-15 20:52:35 UTC
I think this is good.
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-01-15 21:07:45 UTC
A commit references this bug:

Author: gonzo
Date: Tue Jan 15 21:06:59 UTC 2019
New revision: 343060
URL: https://svnweb.freebsd.org/changeset/base/343060

Log:
  [drm] Fix off-by-one error when accessing driver-specific ioctl handlers array

  PR:		231513
  Submitted by:	Young_X <YangX92@hotmail.com>
  Approved by:	imp
  MFC after:	1 week

Changes:
  head/sys/dev/drm/drm_drv.c
Comment 4 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-15 21:25:51 UTC
Thanks for submitting the patch. Committed as base r343060, I'll handle MFC in a week.
Comment 5 commit-hook freebsd_committer freebsd_triage 2019-02-03 14:57:19 UTC
A commit references this bug:

Author: gonzo
Date: Sun Feb  3 14:56:39 UTC 2019
New revision: 343717
URL: https://svnweb.freebsd.org/changeset/base/343717

Log:
  MFC r343060:

  [drm] Fix off-by-one error when accessing driver-specific ioctl handlers array

  PR:		231513
  Submitted by:	Young_X <YangX92@hotmail.com>
  Approved by:	imp

Changes:
_U  stable/12/
  stable/12/sys/dev/drm/drm_drv.c