Bug 26232 - Multiple framebuffer oops and sysfs attribute deadlock
Multiple framebuffer oops and sysfs attribute deadlock
Status: CLOSED CODE_FIX
Product: Drivers
Classification: Unclassified
Component: Console/Framebuffers
All Linux
: P1 normal
Assigned To: James Simmons
https://qa.mandriva.com/show_bug.cgi?...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-06 20:01 UTC by Herton Ronaldo Krzesinski
Modified: 2012-06-13 15:01 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.37
Tree: Mainline
Regression: No


Attachments
Test case which explores using of old firmware framebuffer while another framebuffer replaces it (4.41 KB, text/plain)
2011-01-06 20:07 UTC, Herton Ronaldo Krzesinski
Details
Test case that deadlocks the kernel writing to state sysfs attr while it loads i915 (2.72 KB, text/plain)
2011-01-06 20:08 UTC, Herton Ronaldo Krzesinski
Details
Proposed patch to fix issues exposed by first test case (11.42 KB, patch)
2011-01-06 20:08 UTC, Herton Ronaldo Krzesinski
Details | Diff
Proposed patch to fix issue exposed by second test case (6.37 KB, patch)
2011-01-06 20:09 UTC, Herton Ronaldo Krzesinski
Details | Diff

Description Herton Ronaldo Krzesinski 2011-01-06 20:01:30 UTC
Current framebuffer code has problems with remove_conflicting_framebuffers usage. It introduces possible oops cases for applications that are sitting on old framebuffer that is removed if it conflicts with new framebuffer driver being loaded.

One example is bug report in the URL. plymouthd is sitting on old vesa framebuffer before i915 with modesetting is loaded. intelfb replaces vesa, but plymouthd still has file descriptor opened on vesa. When plymouthd closes the file descriptor, file->private_data which is struct fb_info of vesa is already freed/gone, so the oops happen. Also if the application calls functions which execute file_operations fb functions, kernel can oops also while unregister_framebuffer is being run, as registered_fb has no locking.

Another issue, is that fb_set_suspend can deadlock the kernel, in the same case, because it calls and releases fb_info lock + console semaphore in inverse order of what is done in the rest of framebuffer code.

More details follow on changelog patches I'll attach here. Also I'll provide test cases which triggers these two issues.
Comment 1 Herton Ronaldo Krzesinski 2011-01-06 20:07:05 UTC
Created attachment 42572 [details]
Test case which explores using of old firmware framebuffer while another framebuffer replaces it

- this testcase uses i915, make sure you run on a machine supported by i915 (changing the code to load another module with modesetting should work also)
- Boot with vesa enabled, and make sure your distro/linux system didn't load i915 yet. To achieve this, for example you can boot with vga=<vesa mode number> init=/bin/bash (assuming initrd doesn't load i915)
- I recommend to build kernel with CONFIG_SLUB_DEBUG=y and pass slub_debug at boot cmdline parameters, so use after free of struct fb_info will always trigger an oops (memory is zeroed after free). Also build with CONFIG_PREEMPT=y to make races more likely to happen?
Comment 2 Herton Ronaldo Krzesinski 2011-01-06 20:08:06 UTC
Created attachment 42582 [details]
Test case that deadlocks the kernel writing to state sysfs attr while it loads i915
Comment 3 Herton Ronaldo Krzesinski 2011-01-06 20:08:55 UTC
Created attachment 42592 [details]
Proposed patch to fix issues exposed by first test case
Comment 4 Herton Ronaldo Krzesinski 2011-01-06 20:09:20 UTC
Created attachment 42602 [details]
Proposed patch to fix issue exposed by second test case
Comment 5 sirius 2011-01-20 11:02:45 UTC
I haven't seen this with 2.6.37, but I'm almost always getting it with 2.6.38-rc1 on my Thinkpad T510 using i915. The patches seem to fix that problem.
Comment 6 Florian Mickler 2012-01-12 21:23:04 UTC
A patch referencing this bug report has been merged in Linux v3.2-rc1:

commit 9e769ff3f585db8f978f9113be83d36c7e3965dd
Author: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
Date:   Fri Jun 17 19:02:39 2011 +0000

    fb: avoid possible deadlock caused by fb_set_suspend

Note You need to log in before you can comment on or make changes to this bug.