drm: Avoid NULL deference when disabling a plane from userspace
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 13 Jun 2014 14:22:28 +0000 (15:22 +0100)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 13 Jun 2014 15:45:21 +0000 (17:45 +0200)
To disable a plane, userspace passes in an framebuffer id of 0. This
causes us to pass CRTC == NULL to setplane_internal, who promptly
deferences it to grab the struct drm_device. Oops.

[ 1296.467327] BUG: unable to handle kernel NULL pointer dereference at   (null)
[ 1296.467332] IP: [<c134dc51>] setplane_internal+0x11/0x280
[ 1296.467338] *pde = 00000000
[ 1296.467341] Oops: 0000 [#1] SMP
[ 1296.467344] Modules linked in: ccm bnep bluetooth snd_hda_codec_hdmi snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel arc4 iwldvm snd_hda_controller snd_hda_codec mac80211 snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer iwlwifi sdhci_pci snd cfg80211 x86_pkg_temp_thermal hp_wmi sdhci sparse_keymap mmc_core crc32c_intel rfkill microcode hp_accel lpc_ich lis3lv02d wmi mfd_core serio_raw input_polldev soundcore e1000e ptp pps_core
[ 1296.467367] CPU: 1 PID: 672 Comm: Xorg Tainted: G        W     3.15.0-rc8+ #351
[ 1296.467369] Hardware name: Hewlett-Packard HP ProBook 6360b/1620, BIOS 68SCF Ver. B.42 12/29/2010
[ 1296.467371] task: f423b5c0 ti: c2332000 task.ti: c2332000
[ 1296.467374] EIP: 0060:[<c134dc51>] EFLAGS: 00013286 CPU: 1
[ 1296.467376] EIP is at setplane_internal+0x11/0x280
[ 1296.467378] EAX: 00000000 EBX: c2333e90 ECX: 00000000 EDX: f3165600
[ 1296.467380] ESI: f430f400 EDI: 00000000 EBP: c2333e14 ESP: c2333dd4
[ 1296.467382]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 1296.467384] CR0: 80050033 CR2: 00000000 CR3: 00159000 CR4: 000407d0
[ 1296.467385] Stack:
[ 1296.467387]  000200da 00000002 c2333de8 c15dc4a0 f430f400 c2333e00 c134c54f eeeeeeee
[ 1296.467391]  f430f400 00000007 f416b480 c2333e14 00000000 c2333e90 f430f400 00000000
[ 1296.467396]  c2333e4c c1350aed 00000000 00000000 00000000 00000000 00000000 00000000
[ 1296.467400] Call Trace:
[ 1296.467406]  [<c15dc4a0>] ? mutex_lock+0x10/0x28
[ 1296.467408]  [<c134c54f>] ? _object_find+0x5f/0x90
[ 1296.467413]  [<c1350aed>] drm_mode_setplane+0x10d/0x1f0
[ 1296.467416]  [<c13509e0>] ? drm_mode_getplane+0x100/0x100
[ 1296.467420]  [<c1342e4d>] drm_ioctl+0x1bd/0x4f0
[ 1296.467423]  [<c13509e0>] ? drm_mode_getplane+0x100/0x100
[ 1296.467427]  [<c111c023>] ? handle_mm_fault+0x5d3/0xb30
[ 1296.467431]  [<c1118f31>] ? tlb_finish_mmu+0x11/0x40
[ 1296.467435]  [<c1342c90>] ? drm_ioctl_flags+0x40/0x40
[ 1296.467438]  [<c11593d2>] do_vfs_ioctl+0x2f2/0x4d0
[ 1296.467443]  [<c1226512>] ? inode_has_perm.isra.32+0x32/0x40
[ 1296.467446]  [<c122662f>] ? file_has_perm+0x7f/0x90
[ 1296.467449]  [<c1226fec>] ? selinux_file_ioctl+0x4c/0xf0
[ 1296.467452]  [<c1159610>] SyS_ioctl+0x60/0x90
[ 1296.467456]  [<c15e578c>] sysenter_do_call+0x12/0x22
[ 1296.467457] Code: 3f cf ff eb dd ba 3f 00 00 00 b8 d9 c9 7f c1 e8 e6 3f cf ff eb d9 8d 74 26 00 55 89 e5 57 56 53 83 ec 34 66 66 66 66 90 89 45 f0 <8b> 00 85 c9 89 d6 89 cb 89 45 ec 0f 84 16 01 00 00 8b 45 f0 e8
[ 1296.467485] EIP: [<c134dc51>] setplane_internal+0x11/0x280 SS:ESP 0068:c2

Fixes regression from
commit b02fd7fd8a541c3d590bfdda23365a927b507ceb
Author: Matt Roper <matthew.d.roper@intel.com>
Date:   Tue Jun 10 08:28:10 2014 -0700

    drm: Support legacy cursor ioctls via universal planes when possible (v4)

While at it move the plane parameter to the first position in
setplane_internal since that's the main object we're manipulating.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pallavi G<pallavi.g@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
[danvet: Add note about parameter reordering.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/drm_crtc.c

index b5bce5b..d86d254 100644 (file)
@@ -2131,8 +2131,8 @@ out:
  *
  * src_{x,y,w,h} are provided in 16.16 fixed point format
  */
-static int setplane_internal(struct drm_crtc *crtc,
-                            struct drm_plane *plane,
+static int setplane_internal(struct drm_plane *plane,
+                            struct drm_crtc *crtc,
                             struct drm_framebuffer *fb,
                             int32_t crtc_x, int32_t crtc_y,
                             uint32_t crtc_w, uint32_t crtc_h,
@@ -2140,7 +2140,7 @@ static int setplane_internal(struct drm_crtc *crtc,
                             uint32_t src_x, uint32_t src_y,
                             uint32_t src_w, uint32_t src_h)
 {
-       struct drm_device *dev = crtc->dev;
+       struct drm_device *dev = plane->dev;
        struct drm_framebuffer *old_fb = NULL;
        int ret = 0;
        unsigned int fb_width, fb_height;
@@ -2292,7 +2292,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
         * setplane_internal will take care of deref'ing either the old or new
         * framebuffer depending on success.
         */
-       return setplane_internal(crtc, plane, fb,
+       return setplane_internal(plane, crtc, fb,
                                 plane_req->crtc_x, plane_req->crtc_y,
                                 plane_req->crtc_w, plane_req->crtc_h,
                                 plane_req->src_x, plane_req->src_y,
@@ -2628,7 +2628,7 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
         * setplane_internal will take care of deref'ing either the old or new
         * framebuffer depending on success.
         */
-       ret = setplane_internal(crtc, crtc->cursor, fb,
+       ret = setplane_internal(crtc->cursor, crtc, fb,
                                crtc_x, crtc_y, crtc_w, crtc_h,
                                0, 0, src_w, src_h);