From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: Avoid NULL deference when disabling a plane from userspace Date: Fri, 13 Jun 2014 16:42:26 +0200 Message-ID: <20140613144225.GR5821@phenom.ffwll.local> References: <1402669348-21457-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by gabe.freedesktop.org (Postfix) with ESMTP id C698C6E347 for ; Fri, 13 Jun 2014 07:42:32 -0700 (PDT) Received: by mail-wi0-f182.google.com with SMTP id bs8so1020852wib.3 for ; Fri, 13 Jun 2014 07:42:31 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1402669348-21457-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jun 13, 2014 at 03:22:28PM +0100, Chris Wilson wrote: > 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: [] 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:[] 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] [] ? mutex_lock+0x10/0x28 > [ 1296.467408] [] ? _object_find+0x5f/0x90 > [ 1296.467413] [] drm_mode_setplane+0x10d/0x1f0 > [ 1296.467416] [] ? drm_mode_getplane+0x100/0x100 > [ 1296.467420] [] drm_ioctl+0x1bd/0x4f0 > [ 1296.467423] [] ? drm_mode_getplane+0x100/0x100 > [ 1296.467427] [] ? handle_mm_fault+0x5d3/0xb30 > [ 1296.467431] [] ? tlb_finish_mmu+0x11/0x40 > [ 1296.467435] [] ? drm_ioctl_flags+0x40/0x40 > [ 1296.467438] [] do_vfs_ioctl+0x2f2/0x4d0 > [ 1296.467443] [] ? inode_has_perm.isra.32+0x32/0x40 > [ 1296.467446] [] ? file_has_perm+0x7f/0x90 > [ 1296.467449] [] ? selinux_file_ioctl+0x4c/0xf0 > [ 1296.467452] [] SyS_ioctl+0x60/0x90 > [ 1296.467456] [] 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: [] setplane_internal+0x11/0x280 SS:ESP 0068:c2 > > Fixes regression from > commit b02fd7fd8a541c3d590bfdda23365a927b507ceb > Author: Matt Roper > Date: Tue Jun 10 08:28:10 2014 -0700 > > drm: Support legacy cursor ioctls via universal planes when possible (v4) I'll add "While at it move the plane parameter to the first position in setplane_internal since that's the main object we're manipulating." when merging. Ok? -Daniel > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Pallavi G > Cc: Matt Roper > --- > drivers/gpu/drm/drm_crtc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 5efac1f3be2d..a9cdb56a5301 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -2189,8 +2189,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, > @@ -2198,7 +2198,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; > @@ -2350,7 +2350,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, > @@ -2686,7 +2686,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); > > -- > 2.0.0 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch