New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 695299 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

kernel crash when reading i915_gem_framebuffer

Project Member Reported by dbehr@chromium.org, Feb 23 2017

Issue description

on peppy

cat  /run/debugfs_gpu/i915_gem_framebuffer

results in

[   93.056907] BUG: unable to handle kernel NULL pointer dereference at           (null)
[   93.056928] IP: [<ffffffffbaea535d>] i915_gem_framebuffer_info+0x4d/0xbc
[   93.056948] PGD 0
[   93.056957] Oops: 0000 [#1] SMP
[   93.059194] gsmi: Log Shutdown Reason 0x03
[   93.059203] Modules linked in: uinput rfcomm ath3k memconsole snd_hda_codec_realtek btusb btrtl btbcm btintel snd_hda_codec_hdmi uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core i2c_dev isl29018(C) industrialio snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_page_alloc bluetooth fuse zram(C) zsmalloc(C) nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer cdc_ether usbnet ath9k_btcoex ath9k_common_btcoex ath9k_hw_btcoex ath mac80211 cfg80211 joydev
[   93.059398] CPU 0
[   93.059407] Pid: 2936, comm: less Tainted: G        WC   3.8.11 #1
[   93.059420] RIP: 0010:[<ffffffffbaea535d>]  [<ffffffffbaea535d>] i915_gem_framebuffer_info+0x4d/0xbc
[   93.059452] RSP: 0018:ffff88005d2a1e88  EFLAGS: 00010293
[   93.059468] RAX: ffff88007092ab50 RBX: ffff88007092ab40 RCX: 0000000000001000
[   93.059486] RDX: 0000000000001000 RSI: 0000000000000001 RDI: ffff88010035f578
[   93.059504] RBP: ffff88005d2a1ea8 R08: 00000000000000d0 R09: 0000000000000000
[   93.059522] R10: 0000000000000021 R11: 0000000000000246 R12: ffff88010035f5b8
[   93.059540] R13: ffff88005d25ed80 R14: ffff88010035f578 R15: 0000000000000000
[   93.059560] FS:  00007f8f91566700(0000) GS:ffff880100200000(0000) knlGS:0000000000000000
[   93.059581] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   93.059597] CR2: 0000000000000000 CR3: 0000000050762000 CR4: 00000000000407f0
[   93.059616] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   93.059634] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   93.059654] Process less (pid: 2936, threadinfo ffff88005d2a0000, task ffff88004e03f800)
[   93.059673] Stack:
[   93.059683]  ffff88005d25ed80 ffff88005d25ed80 0000000000000001 0000000000000001
[   93.059723]  ffff88005d2a1ef0 ffffffffbad11fca 0000000000000000 00000000d5f0c7b3
[   93.059760]  0000000000000000 ffff88005d25ed80 ffff88005d25edb8 0000000000000001
[   93.059798] Call Trace:
[   93.059816]  [<ffffffffbad11fca>] traverse+0xe2/0x1b2
[   93.059835]  [<ffffffffbad12101>] seq_lseek+0x67/0xda
[   93.059856]  [<ffffffffbacf5a1b>] vfs_llseek+0x33/0x35
[   93.059875]  [<ffffffffbacf5ffb>] sys_lseek+0x4d/0x6c
[   93.059895]  [<ffffffffbb0cb302>] system_call_fastpath+0x16/0x1b
[   93.059911] Code: 02 00 00 4d 8d b4 24 78 05 00 00 49 81 c4 b8 05 00 00 4c 89 f7 e8 5f 2e 22 00 49 8b 04 24 48 8d 58 f0 48 8d 43 10 4c 39 e0 74 58 <48> 3b 1c 25 00 00 00 00 74 44 8b 43 08 8b 4b 64 48 c7 c6 5a 0b
[   93.060271] RIP  [<ffffffffbaea535d>] i915_gem_framebuffer_info+0x4d/0xbc
[   93.060300]  RSP <ffff88005d2a1e88>
[   93.060313] CR2: 0000000000000000
[   93.060381] ---[ end trace dc1489788192ac15 ]---
[   93.069622] Kernel panic - not syncing: Fatal exception
[   93.069669] Kernel Offset: 0x39c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[   93.069802] gsmi: Log Shutdown Reason 0x02
[   93.078987] ACPI MEMORY or I/O RESET_REG.

 

Comment 1 by dbehr@chromium.org, Feb 23 2017

Status: Started (was: Assigned)
We need this
commit b13b8402523cb2717d4e9b9c39538b1fd48b29d5
Author: Namrta Salonie <namrta.salonie@intel.com>
Date:   Fri Nov 27 13:43:11 2015 +0530

    drm/i915: Fix possible null dereference in framebuffer_info debugfs function
    
    Found by static code analysis tool.
    
    v2: Inserted block instead of goto & renamed variables (Chris)
    v3: Aligned code as per the opening brace (Chris)
        Rebased on top of nightly (Daniel)
    
    Signed-off-by: Namrta Salonie <namrta.salonie@intel.com>
    Signed-off-by: Deepak S <deepak.s@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

In kernels 3.8, 3.10, 3.14 and 3.18. 4.4 already has it.

Comment 2 by dbehr@chromium.org, Feb 23 2017

Also, in some kernels need prerequisites
commit 75e2acf24ab5e4bbf6a19b0f5460456ea16a2e5d
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu May 29 23:27:00 2014 +0200

    drm/i915: Drop locking around fbdev-fb in debugfs
    
    All the date we print is invariant for the lifetime of the driver.
    And none of it would be protected by the mode_config.mutex anyway.
    So drop it.
    
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

to_i915
commit 8ab90958aa57b92616d27dc01b6a864f074dc19e
Author: Imre Deak <imre.deak@intel.com>
Date:   Thu Jan 8 17:54:13 2015 +0200

    drm/i915: add dev_to_i915 helper
    
    This will be needed by later patches, so factor it out.
    
    No functional change.
    
    v2:
    - s/dev_to_i915_priv/dev_to_i915/ (Jani)
    - don't use the helper in i915_pm_suspend (Chris)
    - simplify the helper (Chris)
    v3:
    - remove redundant upcasting in the helper (Daniel)
    
    Signed-off-by: Imre Deak <imre.deak@intel.com>
    Reviewed-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    (cherry picked from commit 888d0d421663313739a8bf93459c6ba61fd4b121)
    Signed-off-by: Marc Herbert <marc.herbert@intel.com>


Comment 3 by dbehr@chromium.org, Feb 23 2017

Also, pre-requisite in some kernels
commit 267f0c90ac6728f70fade74ab89932a00e5e5a7e
Author: Damien Lespiau <damien.lespiau@intel.com>
Date:   Mon Jun 24 22:59:48 2013 +0100

    drm/i915: Use seq_puts/seq_putc when possible
    
    Caught with checkpatch.pl.
    
    Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
    Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Comment 4 by dbehr@chromium.org, Feb 23 2017

actually we need to_i915 not dev_to_i915
commit 2c1792a10b10e41dcf34c97304fb8f75e52e7112
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Aug 1 18:39:55 2013 +0100

    drm/i915: Tidy the macro casting by using an inline function
    
    Some of our macros we trying to convert from an drm_device to a
    drm_i915_private and then use the pointer inline. This is not only
    cumbersome but prone to error. Replacing it with a typesafe function
    should help catch those errors in future.
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
    [danvet: Squash in fixup to correctly order static vs. inline
    qualifiers, static comes first. Also fix up another offender.]
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Comment 5 by dbehr@chromium.org, Feb 23 2017

And maybe more for drm_for_each*  ??

commit 6295d607ad34ee4e43aab3f20714c2ef7a6adea1
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jul 9 23:44:25 2015 +0200

    drm: Add modeset object iterators
    
    And roll them out across drm_* files. The point here isn't code
    prettification (it helps with that too) but that some of these lists
    aren't static any more. And having macros will gives us a convenient
    place to put locking checks into.
    
    I didn't add an iterator for props since that's only used by a
    list_for_each_entry_safe in the driver teardown code.
    
    Search&replace was done with the below cocci spatch. Note that there's
    a bunch more places that didn't match and which would need some manual
    changes, but I've intentially left these out for this mostly automated
    patch.
    
    iterator name drm_for_each_crtc;
    struct drm_crtc *crtc;
    struct drm_device *dev;
    expression head;
    @@
    - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
    + drm_for_each_crtc (crtc, dev) {
    ...
    }
    
    @@
    iterator name drm_for_each_encoder;
    struct drm_encoder *encoder;
    struct drm_device *dev;
    expression head;
    @@
    - list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
    + drm_for_each_encoder (encoder, dev) {
    ...
    }
    
    @@
    iterator name drm_for_each_fb;
    struct drm_framebuffer *fb;
    struct drm_device *dev;
    expression head;
    @@
    - list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
    + drm_for_each_fb (fb, dev) {
    ...
    }
    
    @@
    iterator name drm_for_each_connector;
    struct drm_connector *connector;
    struct drm_device *dev;
    expression head;
    @@
    - list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
    + drm_for_each_connector (connector, dev) {
    ...
    }
    
    Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>


commit 373701b1fc7d7c0013ae4fffd8103615c150751e
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Tue Nov 24 21:21:55 2015 +0200

    drm: fix potential dangling else problems in for_each_ macros
    
    We have serious dangling else bugs waiting to happen in our for_each_
    style macros with ifs. Consider, for example,
    
     #define drm_for_each_plane_mask(plane, dev, plane_mask) \
             list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
                     if ((plane_mask) & (1 << drm_plane_index(plane)))
    
    If this is used in context:
    
        if (condition)
                drm_for_each_plane_mask(plane, dev, plane_mask);
        else
                foo();
    
    foo() will be called for each plane *not* in plane_mask, if condition
    holds, and not at all if condition doesn't hold.
    
    Fix this by reversing the conditions in the macros, and adding an else
    branch for the "for each" block, so that other if/else blocks can't
    interfere. Provide a "for_each_if" helper macro to make it easier to get
    this right.
    
    Signed-off-by: Jani Nikula <jani.nikula@intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/1448392916-2281-1-git-send-email-jani.nikula@intel.com
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Comment 6 by dbehr@chromium.org, Feb 24 2017

Or preferably, we can merge this simpler change 
commit 131a56dc41b8c026f97466341167d86deb25357b
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Oct 17 14:35:31 2013 +0200

    drm/i915: don't Oops in debugfs for I915_FBDEV=n
    
    Failed to properly test this.
    
    Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Chris Wilson <chris@chris-wilson.co.uk>
    Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
    Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5fce5d89b68a..7811bf40dd2c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1456,7 +1456,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
 
        mutex_lock(&dev->mode_config.fb_lock);
        list_for_each_entry(fb, &dev->mode_config.fb_list, base.head) {
-               if (&fb->base == ifbdev->helper.fb)
+               if (ifbdev && &fb->base == ifbdev->helper.fb)
                        continue;
 
                seq_printf(m, "user size: %d x %d, depth %d, %d bpp, refcount %d, obj ",

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 24 2017

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d3425edeec4c35e5c27a76b49bc8d734b06499c7

commit d3425edeec4c35e5c27a76b49bc8d734b06499c7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Fri Feb 24 05:25:23 2017

UPSTREAM: drm/i915: don't Oops in debugfs for I915_FBDEV=n

Failed to properly test this.

BUG= chromium:695299 
TEST=ssh to peppy or lumpy and run cat /run/debugfs_gpu/i915_gem_framebuffer

Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit 131a56dc41b8c026f97466341167d86deb25357b)
Signed-off-by: Dominik Behr <dbehr@chromium.org>

Change-Id: Icab99b331945250815cc5e8b7067c234d3f453ed
Reviewed-on: https://chromium-review.googlesource.com/446666
Commit-Ready: Dominik Behr <dbehr@chromium.org>
Tested-by: Dominik Behr <dbehr@chromium.org>
Reviewed-by: Ilja H. Friedel <ihf@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/d3425edeec4c35e5c27a76b49bc8d734b06499c7/drivers/gpu/drm/i915/i915_debugfs.c

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d3425edeec4c35e5c27a76b49bc8d734b06499c7

commit d3425edeec4c35e5c27a76b49bc8d734b06499c7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Fri Feb 24 05:25:23 2017

UPSTREAM: drm/i915: don't Oops in debugfs for I915_FBDEV=n

Failed to properly test this.

BUG= chromium:695299 
TEST=ssh to peppy or lumpy and run cat /run/debugfs_gpu/i915_gem_framebuffer

Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit 131a56dc41b8c026f97466341167d86deb25357b)
Signed-off-by: Dominik Behr <dbehr@chromium.org>

Change-Id: Icab99b331945250815cc5e8b7067c234d3f453ed
Reviewed-on: https://chromium-review.googlesource.com/446666
Commit-Ready: Dominik Behr <dbehr@chromium.org>
Tested-by: Dominik Behr <dbehr@chromium.org>
Reviewed-by: Ilja H. Friedel <ihf@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/d3425edeec4c35e5c27a76b49bc8d734b06499c7/drivers/gpu/drm/i915/i915_debugfs.c

Comment 9 by dbehr@chromium.org, Feb 24 2017

Status: Fixed (was: Started)

Comment 10 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 11 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61
Status: Verified (was: Fixed)
Verified in R62-9799.0.0. 

Sign in to add a comment