From: Daniel J Blueman <hidden> Date: 2011-04-10 11:10:04
When building in the eg vga16 and intelfb framebuffers and page
debugging, we can get hit by a use-after-free oops [1]; with page
debugging disabled, this slips by unnoticed.
When intelfb registers, the vga16 framebuffer is detected covering
it's I/O regions, so it is removed (in
remove_conflicting_framebuffers->unregister_framebuffer). The problem
is the fb_info structure is freed (in
unregister_framebuffer->vga16fb_destroy->framebuffer_release->kfree)
before all open files to it are closed, so we get a use-after-free
scenario.
Freeing should be deferred until the last user has gone away. What
approach would make sense here?
Daniel
From: Daniel J Blueman <hidden> Date: 2011-04-20 05:50:13
Any ideas on how best to address this issue [0], since it causes
silent corruption, or at best crashes?
Thanks,
Daniel
--- [0]
When building in the eg vga16 and intelfb framebuffers and page
debugging, we can get hit by a use-after-free oops [1]; with page
debugging disabled, this slips by unnoticed.
When intelfb registers, the vga16 framebuffer is detected covering
it's I/O regions, so it is removed (in
remove_conflicting_framebuffers->unregister_framebuffer). The problem
is the fb_info structure is freed (in
unregister_framebuffer->vga16fb_destroy->framebuffer_release->kfree)
before all open files to it are closed, so we get a use-after-free
scenario.
Freeing should be deferred until the last user has gone away. What
approach would make sense here?
Daniel
From: Bruno Prémont <bonbons@linux-vserver.org> Date: 2011-04-20 06:05:41
On Wed, 20 Apr 2011 13:50:10 Daniel J Blueman [off-list ref] wrote:
Any ideas on how best to address this issue [0], since it causes
silent corruption, or at best crashes?
There is probably no easy short-term fix to this... The complex fix
would be to implement some deferred free with driver-local ref-counting
though to be complete it would have to be done for pretty much every FB
driver!
The mid-term fix would be to have fb_info ref-counted by FB
subsystem (I'm working on it, slowly). This way freeing would be
deferred to when last references is being dropped.
The driver then just has to know when it can't touch underlying
hardware anymore (and do the right thing with framebuffer memory in
case it is still mmapped somewhere).
Bruno
quoted hunk
Thanks,
Daniel
--- [0]
When building in the eg vga16 and intelfb framebuffers and page
debugging, we can get hit by a use-after-free oops [1]; with page
debugging disabled, this slips by unnoticed.
When intelfb registers, the vga16 framebuffer is detected covering
it's I/O regions, so it is removed (in
remove_conflicting_framebuffers->unregister_framebuffer). The problem
is the fb_info structure is freed (in
unregister_framebuffer->vga16fb_destroy->framebuffer_release->kfree)
before all open files to it are closed, so we get a use-after-free
scenario.
Freeing should be deferred until the last user has gone away. What
approach would make sense here?
Daniel
On Wed, 20 Apr 2011 08:05:35 +0200
Bruno Prémont [off-list ref] wrote:
On Wed, 20 Apr 2011 13:50:10 Daniel J Blueman [off-list ref] wrote:
quoted
Any ideas on how best to address this issue [0], since it causes
silent corruption, or at best crashes?
There is probably no easy short-term fix to this...
The short term fix would be to deliberately leak the buffer. That should
go into 2.6.39-rc right now with a comment explaining the situation.
Otherwise who knows what corruption may occur to user data if unlucky.
The other 'cheat' might be to tweak the API so the removal API isn't a
'destroy' interface but a 'shut down' and has a matching 'restart' one
for when the intelfb unloads at which point vga16fb can carry on with the
original fb_info 8)
Alan
From: Daniel J Blueman <hidden> Date: 2011-05-06 02:39:00
On 20 April 2011 17:56, Alan Cox [off-list ref] wrote:
On Wed, 20 Apr 2011 08:05:35 +0200
Bruno Prémont [off-list ref] wrote:
quoted
On Wed, 20 Apr 2011 13:50:10 Daniel J Blueman [off-list ref] wrote:
quoted
Any ideas on how best to address this issue [0], since it causes
silent corruption, or at best crashes?
There is probably no easy short-term fix to this...
The short term fix would be to deliberately leak the buffer. That should
go into 2.6.39-rc right now with a comment explaining the situation.
Otherwise who knows what corruption may occur to user data if unlucky.
The other 'cheat' might be to tweak the API so the removal API isn't a
'destroy' interface but a 'shut down' and has a matching 'restart' one
for when the intelfb unloads at which point vga16fb can carry on with the
original fb_info 8)
From: Anca Emanuel <hidden> Date: 2011-05-07 15:24:13
Hi, Daniel J Blueman.
Did you test https://lkml.org/lkml/2011/5/5/208 ? And it works for you ?
Then please reply with your error and an Tested-by.
And CC: "Dave Airlie" [off-list ref]
From: Daniel J Blueman <hidden> Date: 2011-05-08 11:25:22
On 7 May 2011 23:24, Anca Emanuel [off-list ref] wrote:
Hi, Daniel J Blueman.
Did you test https://lkml.org/lkml/2011/5/5/208 ? And it works for you ?
Then please reply with your error and an Tested-by.
And CC: "Dave Airlie" [off-list ref]
Tested against 2.6.39-rc6. Instrumentation and debug catches the
(silent without debug) use-after-free case, which now doesn't show up
with this patch, so looks good. Probably good sense to get into
-stable too.
Tested-by: Daniel J Blueman <redacted>
Thanks,
Daniel
--
Daniel J Blueman