Bug 26232

Summary:	Multiple framebuffer oops and sysfs attribute deadlock
Product:	Drivers	Reporter:	Herton Ronaldo Krzesinski (herton)
Component:	Console/Framebuffers	Assignee:	James Simmons (jsimmons)
Status:	CLOSED CODE_FIX
Severity:	normal	CC:	alan, florian, sirius
Priority:	P1
Hardware:	All
OS:	Linux
URL:	https://qa.mandriva.com/show_bug.cgi?id=59260
Kernel Version:	2.6.37	Subsystem:
Regression:	No	Bisected commit-id:
Attachments:	Test case which explores using of old firmware framebuffer while another framebuffer replaces it Test case that deadlocks the kernel writing to state sysfs attr while it loads i915 Proposed patch to fix issues exposed by first test case Proposed patch to fix issue exposed by second test case

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