Patchwork Patch not upstream: fix race between open and removal of framebuffers

login
register
mail settings
Submitter Anca Emanuel
Date April 2, 2011, 3:37 p.m.
Message ID <BANLkTikAc66kTknEKCX4Mbg7_9CGw+qDfQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/89451/
State New
Headers show

Comments

Anca Emanuel - April 2, 2011, 3:37 p.m.
This patch: http://is.gd/otIfGc is not in mainline. Any reason for that ?

see: https://lkml.org/lkml/2011/2/24/445

Please post c5a742b5f78e161d6a13853a7e3e6e1dfa429e69
UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers
to lkml with CC: stable@kernel.org

-----------------------------------------------------------------


From c5a742b5f78e161d6a13853a7e3e6e1dfa429e69 Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <apw@canonical.com>
Date: Thu, 29 Jul 2010 16:48:20 +0100
Subject: [PATCH 1/1] UBUNTU: SAUCE: fbcon -- fix race between open and
removal of framebuffers

Currently there is no locking for updates to the registered_fb list.
This allows an open through /dev/fbN to pick up a registered framebuffer
pointer in parallel with it being released, as happens when a conflicting
framebuffer is ejected or on module unload.  There is also no reference
counting on the framebuffer descriptor which is referenced from all open
files, leading to references to released or reused memory to persist on
these open files.

This patch adds a reference count to the framebuffer descriptor to prevent
it from being released until after all pending opens are closed.  This
allows the pending opens to detect the closed status and unmap themselves.
It also adds locking to the framebuffer lookup path, locking it against
the removal path such that it is possible to atomically lookup and take a
reference to the descriptor.  It also adds locking to the read and write
paths which currently could access the framebuffer descriptor after it
has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
indicate that all access should be errored for this device.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
---
 drivers/video/fbmem.c |  132 ++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fb.h    |    2 +
 2 files changed, 105 insertions(+), 29 deletions(-)
Tim Gardner - April 7, 2011, 2 p.m.
On 04/02/2011 09:37 AM, Anca Emanuel wrote:
> This patch: http://is.gd/otIfGc is not in mainline. Any reason for that ?
>
> see: https://lkml.org/lkml/2011/2/24/445
>
> Please post c5a742b5f78e161d6a13853a7e3e6e1dfa429e69
> UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers
> to lkml with CC: stable@kernel.org
>
> -----------------------------------------------------------------
>
>
>  From c5a742b5f78e161d6a13853a7e3e6e1dfa429e69 Mon Sep 17 00:00:00 2001
> From: Andy Whitcroft<apw@canonical.com>
> Date: Thu, 29 Jul 2010 16:48:20 +0100
> Subject: [PATCH 1/1] UBUNTU: SAUCE: fbcon -- fix race between open and
> removal of framebuffers
>
> Currently there is no locking for updates to the registered_fb list.
> This allows an open through /dev/fbN to pick up a registered framebuffer
> pointer in parallel with it being released, as happens when a conflicting
> framebuffer is ejected or on module unload.  There is also no reference
> counting on the framebuffer descriptor which is referenced from all open
> files, leading to references to released or reused memory to persist on
> these open files.
>
> This patch adds a reference count to the framebuffer descriptor to prevent
> it from being released until after all pending opens are closed.  This
> allows the pending opens to detect the closed status and unmap themselves.
> It also adds locking to the framebuffer lookup path, locking it against
> the removal path such that it is possible to atomically lookup and take a
> reference to the descriptor.  It also adds locking to the read and write
> paths which currently could access the framebuffer descriptor after it
> has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
> indicate that all access should be errored for this device.
>
> Signed-off-by: Andy Whitcroft<apw@canonical.com>
> Acked-by: Stefan Bader<stefan.bader@canonical.com>
> Signed-off-by: Leann Ogasawara<leann.ogasawara@canonical.com>
> ---
>   drivers/video/fbmem.c |  132 ++++++++++++++++++++++++++++++++++++++-----------
>   include/linux/fb.h    |    2 +
>   2 files changed, 105 insertions(+), 29 deletions(-)
>

The patch author is away on vacation. I'm gonna leave it up to him to 
pursue upstreaming upon his return.

rtg
Anca Emanuel - May 5, 2011, 4:54 p.m.
On Thu, Apr 7, 2011 at 5:00 PM, Tim Gardner <tim.gardner@canonical.com> wrote:
> On 04/02/2011 09:37 AM, Anca Emanuel wrote:
>>
>> This patch: http://is.gd/otIfGc is not in mainline. Any reason for that ?
>>
>> see: https://lkml.org/lkml/2011/2/24/445
>>
>> Please post c5a742b5f78e161d6a13853a7e3e6e1dfa429e69
>> UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers
>> to lkml with CC: stable@kernel.org
>>
>> -----------------------------------------------------------------
>>
>>
>>  From c5a742b5f78e161d6a13853a7e3e6e1dfa429e69 Mon Sep 17 00:00:00 2001
>> From: Andy Whitcroft<apw@canonical.com>
>> Date: Thu, 29 Jul 2010 16:48:20 +0100
>> Subject: [PATCH 1/1] UBUNTU: SAUCE: fbcon -- fix race between open and
>> removal of framebuffers
>>
>> Currently there is no locking for updates to the registered_fb list.
>> This allows an open through /dev/fbN to pick up a registered framebuffer
>> pointer in parallel with it being released, as happens when a conflicting
>> framebuffer is ejected or on module unload.  There is also no reference
>> counting on the framebuffer descriptor which is referenced from all open
>> files, leading to references to released or reused memory to persist on
>> these open files.
>>
>> This patch adds a reference count to the framebuffer descriptor to prevent
>> it from being released until after all pending opens are closed.  This
>> allows the pending opens to detect the closed status and unmap themselves.
>> It also adds locking to the framebuffer lookup path, locking it against
>> the removal path such that it is possible to atomically lookup and take a
>> reference to the descriptor.  It also adds locking to the read and write
>> paths which currently could access the framebuffer descriptor after it
>> has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
>> indicate that all access should be errored for this device.
>>
>> Signed-off-by: Andy Whitcroft<apw@canonical.com>
>> Acked-by: Stefan Bader<stefan.bader@canonical.com>
>> Signed-off-by: Leann Ogasawara<leann.ogasawara@canonical.com>
>> ---
>>  drivers/video/fbmem.c |  132
>> ++++++++++++++++++++++++++++++++++++++-----------
>>  include/linux/fb.h    |    2 +
>>  2 files changed, 105 insertions(+), 29 deletions(-)
>>
>
> The patch author is away on vacation. I'm gonna leave it up to him to pursue
> upstreaming upon his return.
>
> rtg
> --
> Tim Gardner tim.gardner@canonical.com
>

I'm using 2.6.39-rc6 now

I still get:
[   21.964367] BUG: unable to handle kernel paging request at 0000010a00000010
[   21.964396] IP: [<ffffffff8130abe0>] fb_release+0x30/0x70
[   21.964410] PGD 0
[   21.964416] Oops: 0000 [#1] SMP
[   21.964424] last sysfs file: /sys/devices/virtual/vtconsole/vtcon1/uevent
[   21.964434] CPU 1
[   21.964438] Modules linked in: parport_pc ppdev
snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm
adt7475 hwmon_vid snd_seq_midi snd_rawmidi snd_seq_midi_event nouveau
snd_seq snd_timer snd_seq_device ttm drm_kms_helper snd intel_agp
psmouse soundcore serio_raw intel_gtt snd_page_alloc drm i2c_algo_bit
video lp parport pata_marvell ahci libahci r8169 mii
[   21.964528]
[   21.964533] Pid: 221, comm: plymouthd Not tainted 2.6.39-rc6 #7
MICRO-STAR INTERNATIONAL CO.,LTD MS-7360/MS-7360
[   21.964548] RIP: 0010:[<ffffffff8130abe0>]  [<ffffffff8130abe0>]
fb_release+0x30/0x70
[   21.964560] RSP: 0018:ffff880037211eb8  EFLAGS: 00010286
[   21.964566] RAX: ffff880037210000 RBX: ffff88007f817000 RCX: 0000000000000001
[   21.964573] RDX: 0000010a00000000 RSI: ffff8800370f5540 RDI: ffff88007f817008
[   21.964580] RBP: ffff880037211ec8 R08: 0000000000000000 R09: 0000000000000000
[   21.964588] R10: ffff8800370f5550 R11: 0000000000000246 R12: ffff88007f817008
[   21.964595] R13: ffff88007d3db540 R14: ffff88007be34d90 R15: ffff88007be34d90
[   21.964604] FS:  00007fb335025720(0000) GS:ffff88007fc80000(0000)
knlGS:0000000000000000
[   21.964739] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.964746] CR2: 0000010a00000010 CR3: 000000007b41a000 CR4: 00000000000006e0
[   21.964754] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   21.964762] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   21.964770] Process plymouthd (pid: 221, threadinfo
ffff880037210000, task ffff880036cd16c0)
[   21.964778] Stack:
[   21.964782]  ffff8800370f5540 0000000000000008 ffff880037211f18
ffffffff8115cfaa
[   21.964797]  ffff8800370f5550 ffff8800793c7b00 ffff88006744fcd0
ffff8800370f5540
[   21.964811]  ffff88007c3b9080 0000000000000000 000000000000000b
0000000000000000
[   21.964825] Call Trace:
[   21.964834]  [<ffffffff8115cfaa>] fput+0xea/0x220
[   21.964842]  [<ffffffff811591f6>] filp_close+0x66/0x90
[   21.964849]  [<ffffffff811597c7>] sys_close+0xb7/0x120
[   21.964858]  [<ffffffff815b3002>] system_call_fastpath+0x16/0x1b
[   21.964865] Code: 83 ec 10 48 89 1c 24 4c 89 64 24 08 0f 1f 44 00
00 48 8b 9e a0 00 00 00 4c 8d 63 08 4c 89 e7 e8 d7 ea 29 00 48 8b 93
b8 03 00 00
[   21.964944]  8b 42 10 48 85 c0 74 11 be 01 00 00 00 48 89 df ff d0 48 8b
[   21.964983] RIP  [<ffffffff8130abe0>] fb_release+0x30/0x70
[   21.964992]  RSP <ffff880037211eb8>
[   21.964997] CR2: 0000010a00000010

I can use de PC, but when it wake up from S3, hangs.
full dmesg at: http://pastebin.com/rhMJrF2x
uname -a
Linux ubuntu 2.6.39-rc6 #7 SMP Wed May 4 12:26:39 EEST 2011 x86_64
x86_64 x86_64 GNU/Linux

I read that Ubuntu have something like 150 patches NOT upstreamed. Why ?
And other guys complaining about the hard work they do to maintain
stable and mainline.

If you not upstream your work, then what is the ideea ? Keep it only
for Ubuntu users ?

I have the latest Linus git tree, and I applied the patch like this:
wget http://is.gd/otIfGc
git apply otIfGc

Linus, if nobody ask you, please apply the patch.
With Tested-by: Anca Emanuel <anca.emanuel@gmail.com>

full dmesg after the patch: http://pastebin.com/XtNXzgPc
Tested sleep and wake up from S3.
Jesse Barnes - May 5, 2011, 4:56 p.m.
On Thu, 5 May 2011 19:54:20 +0300
Anca Emanuel <anca.emanuel@gmail.com> wrote:
> I'm using 2.6.39-rc6 now
> 
> I still get:
> [   21.964367] BUG: unable to handle kernel paging request at 0000010a00000010
> [   21.964396] IP: [<ffffffff8130abe0>] fb_release+0x30/0x70
> [   21.964410] PGD 0
> [   21.964416] Oops: 0000 [#1] SMP
> [   21.964424] last sysfs file: /sys/devices/virtual/vtconsole/vtcon1/uevent
> [   21.964434] CPU 1
> [   21.964438] Modules linked in: parport_pc ppdev
> snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm
> adt7475 hwmon_vid snd_seq_midi snd_rawmidi snd_seq_midi_event nouveau
> snd_seq snd_timer snd_seq_device ttm drm_kms_helper snd intel_agp
> psmouse soundcore serio_raw intel_gtt snd_page_alloc drm i2c_algo_bit
> video lp parport pata_marvell ahci libahci r8169 mii
> [   21.964528]
> [   21.964533] Pid: 221, comm: plymouthd Not tainted 2.6.39-rc6 #7
> MICRO-STAR INTERNATIONAL CO.,LTD MS-7360/MS-7360
> [   21.964548] RIP: 0010:[<ffffffff8130abe0>]  [<ffffffff8130abe0>]
> fb_release+0x30/0x70
> [   21.964560] RSP: 0018:ffff880037211eb8  EFLAGS: 00010286
> [   21.964566] RAX: ffff880037210000 RBX: ffff88007f817000 RCX: 0000000000000001
> [   21.964573] RDX: 0000010a00000000 RSI: ffff8800370f5540 RDI: ffff88007f817008
> [   21.964580] RBP: ffff880037211ec8 R08: 0000000000000000 R09: 0000000000000000
> [   21.964588] R10: ffff8800370f5550 R11: 0000000000000246 R12: ffff88007f817008
> [   21.964595] R13: ffff88007d3db540 R14: ffff88007be34d90 R15: ffff88007be34d90
> [   21.964604] FS:  00007fb335025720(0000) GS:ffff88007fc80000(0000)
> knlGS:0000000000000000
> [   21.964739] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   21.964746] CR2: 0000010a00000010 CR3: 000000007b41a000 CR4: 00000000000006e0
> [   21.964754] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   21.964762] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [   21.964770] Process plymouthd (pid: 221, threadinfo
> ffff880037210000, task ffff880036cd16c0)
> [   21.964778] Stack:
> [   21.964782]  ffff8800370f5540 0000000000000008 ffff880037211f18
> ffffffff8115cfaa
> [   21.964797]  ffff8800370f5550 ffff8800793c7b00 ffff88006744fcd0
> ffff8800370f5540
> [   21.964811]  ffff88007c3b9080 0000000000000000 000000000000000b
> 0000000000000000
> [   21.964825] Call Trace:
> [   21.964834]  [<ffffffff8115cfaa>] fput+0xea/0x220
> [   21.964842]  [<ffffffff811591f6>] filp_close+0x66/0x90
> [   21.964849]  [<ffffffff811597c7>] sys_close+0xb7/0x120
> [   21.964858]  [<ffffffff815b3002>] system_call_fastpath+0x16/0x1b
> [   21.964865] Code: 83 ec 10 48 89 1c 24 4c 89 64 24 08 0f 1f 44 00
> 00 48 8b 9e a0 00 00 00 4c 8d 63 08 4c 89 e7 e8 d7 ea 29 00 48 8b 93
> b8 03 00 00
> [   21.964944]  8b 42 10 48 85 c0 74 11 be 01 00 00 00 48 89 df ff d0 48 8b
> [   21.964983] RIP  [<ffffffff8130abe0>] fb_release+0x30/0x70
> [   21.964992]  RSP <ffff880037211eb8>
> [   21.964997] CR2: 0000010a00000010
> 
> I can use de PC, but when it wake up from S3, hangs.
> full dmesg at: http://pastebin.com/rhMJrF2x
> uname -a
> Linux ubuntu 2.6.39-rc6 #7 SMP Wed May 4 12:26:39 EEST 2011 x86_64
> x86_64 x86_64 GNU/Linux
> 
> I read that Ubuntu have something like 150 patches NOT upstreamed. Why ?
> And other guys complaining about the hard work they do to maintain
> stable and mainline.
> 
> If you not upstream your work, then what is the ideea ? Keep it only
> for Ubuntu users ?
> 
> I have the latest Linus git tree, and I applied the patch like this:
> wget http://is.gd/otIfGc
> git apply otIfGc
> 
> Linus, if nobody ask you, please apply the patch.
> With Tested-by: Anca Emanuel <anca.emanuel@gmail.com>
> 
> full dmesg after the patch: http://pastebin.com/XtNXzgPc
> Tested sleep and wake up from S3.

Yeah, I'd like to see this fixed too.  Without it, everyone on
the latest Ubuntu release will see this bug whenever they try to boot
an upstream kernel.
Tim Gardner - May 5, 2011, 6 p.m.
On 05/05/2011 10:56 AM, Jesse Barnes wrote:
> On Thu, 5 May 2011 19:54:20 +0300
> Anca Emanuel<anca.emanuel@gmail.com>  wrote:
>> I'm using 2.6.39-rc6 now
>>
>> I still get:
>> [   21.964367] BUG: unable to handle kernel paging request at 0000010a00000010
>> [   21.964396] IP: [<ffffffff8130abe0>] fb_release+0x30/0x70
>> [   21.964410] PGD 0
>> [   21.964416] Oops: 0000 [#1] SMP
>> [   21.964424] last sysfs file: /sys/devices/virtual/vtconsole/vtcon1/uevent
>> [   21.964434] CPU 1
>> [   21.964438] Modules linked in: parport_pc ppdev
>> snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm
>> adt7475 hwmon_vid snd_seq_midi snd_rawmidi snd_seq_midi_event nouveau
>> snd_seq snd_timer snd_seq_device ttm drm_kms_helper snd intel_agp
>> psmouse soundcore serio_raw intel_gtt snd_page_alloc drm i2c_algo_bit
>> video lp parport pata_marvell ahci libahci r8169 mii
>> [   21.964528]
>> [   21.964533] Pid: 221, comm: plymouthd Not tainted 2.6.39-rc6 #7
>> MICRO-STAR INTERNATIONAL CO.,LTD MS-7360/MS-7360
>> [   21.964548] RIP: 0010:[<ffffffff8130abe0>]  [<ffffffff8130abe0>]
>> fb_release+0x30/0x70
>> [   21.964560] RSP: 0018:ffff880037211eb8  EFLAGS: 00010286
>> [   21.964566] RAX: ffff880037210000 RBX: ffff88007f817000 RCX: 0000000000000001
>> [   21.964573] RDX: 0000010a00000000 RSI: ffff8800370f5540 RDI: ffff88007f817008
>> [   21.964580] RBP: ffff880037211ec8 R08: 0000000000000000 R09: 0000000000000000
>> [   21.964588] R10: ffff8800370f5550 R11: 0000000000000246 R12: ffff88007f817008
>> [   21.964595] R13: ffff88007d3db540 R14: ffff88007be34d90 R15: ffff88007be34d90
>> [   21.964604] FS:  00007fb335025720(0000) GS:ffff88007fc80000(0000)
>> knlGS:0000000000000000
>> [   21.964739] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   21.964746] CR2: 0000010a00000010 CR3: 000000007b41a000 CR4: 00000000000006e0
>> [   21.964754] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [   21.964762] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> [   21.964770] Process plymouthd (pid: 221, threadinfo
>> ffff880037210000, task ffff880036cd16c0)
>> [   21.964778] Stack:
>> [   21.964782]  ffff8800370f5540 0000000000000008 ffff880037211f18
>> ffffffff8115cfaa
>> [   21.964797]  ffff8800370f5550 ffff8800793c7b00 ffff88006744fcd0
>> ffff8800370f5540
>> [   21.964811]  ffff88007c3b9080 0000000000000000 000000000000000b
>> 0000000000000000
>> [   21.964825] Call Trace:
>> [   21.964834]  [<ffffffff8115cfaa>] fput+0xea/0x220
>> [   21.964842]  [<ffffffff811591f6>] filp_close+0x66/0x90
>> [   21.964849]  [<ffffffff811597c7>] sys_close+0xb7/0x120
>> [   21.964858]  [<ffffffff815b3002>] system_call_fastpath+0x16/0x1b
>> [   21.964865] Code: 83 ec 10 48 89 1c 24 4c 89 64 24 08 0f 1f 44 00
>> 00 48 8b 9e a0 00 00 00 4c 8d 63 08 4c 89 e7 e8 d7 ea 29 00 48 8b 93
>> b8 03 00 00
>> [   21.964944]  8b 42 10 48 85 c0 74 11 be 01 00 00 00 48 89 df ff d0 48 8b
>> [   21.964983] RIP  [<ffffffff8130abe0>] fb_release+0x30/0x70
>> [   21.964992]  RSP<ffff880037211eb8>
>> [   21.964997] CR2: 0000010a00000010
>>
>> I can use de PC, but when it wake up from S3, hangs.
>> full dmesg at: http://pastebin.com/rhMJrF2x
>> uname -a
>> Linux ubuntu 2.6.39-rc6 #7 SMP Wed May 4 12:26:39 EEST 2011 x86_64
>> x86_64 x86_64 GNU/Linux
>>
>> I read that Ubuntu have something like 150 patches NOT upstreamed. Why ?
>> And other guys complaining about the hard work they do to maintain
>> stable and mainline.
>>
>> If you not upstream your work, then what is the ideea ? Keep it only
>> for Ubuntu users ?
>>
>> I have the latest Linus git tree, and I applied the patch like this:
>> wget http://is.gd/otIfGc
>> git apply otIfGc
>>
>> Linus, if nobody ask you, please apply the patch.
>> With Tested-by: Anca Emanuel<anca.emanuel@gmail.com>
>>
>> full dmesg after the patch: http://pastebin.com/XtNXzgPc
>> Tested sleep and wake up from S3.
>
> Yeah, I'd like to see this fixed too.  Without it, everyone on
> the latest Ubuntu release will see this bug whenever they try to boot
> an upstream kernel.
>

OK, I've sent it to the references indicated by scripts/get_maintainers.pl

rtg
John Johansen - May 7, 2011, 5:04 a.m.
On 05/05/2011 09:54 AM, Anca Emanuel wrote:
> On Thu, Apr 7, 2011 at 5:00 PM, Tim Gardner <tim.gardner@canonical.com> wrote:
>> On 04/02/2011 09:37 AM, Anca Emanuel wrote:

<< snip >>

> I can use de PC, but when it wake up from S3, hangs.
> full dmesg at: http://pastebin.com/rhMJrF2x
> uname -a
> Linux ubuntu 2.6.39-rc6 #7 SMP Wed May 4 12:26:39 EEST 2011 x86_64
> x86_64 x86_64 GNU/Linux
> 
> I read that Ubuntu have something like 150 patches NOT upstreamed. Why ?
> And other guys complaining about the hard work they do to maintain
> stable and mainline.
> 
> If you not upstream your work, then what is the ideea ? Keep it only
> for Ubuntu users ?
> 
No, it a diff of about 150 patches for natty from the base 2.6.38 kernel.
We upstream our patches.  Sometimes some patches will slip through for a
while, but they will go up.

Of the 150 or so patches not "upstream" there are only a few that are
ubuntu specific.  Some of the patches are backports from patches upstream
for 2.6.39, some are from linux-next or other development branches,
and some are from projects that aren't upstream for one reason or another
(eg. aufs).

Our delta is public, well documented and available to be examined any
time.  Just pull our kernels from

git://kernel.ubuntu.com/ubuntu/ubuntu-oneiric.git
git://kernel.ubuntu.com/ubuntu/ubuntu-natty.git
..

Patch

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e2bf953..877200b 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -42,6 +42,8 @@ 

 #define FBPIXMAPSIZE	(1024 * 8)

+/* Protects the registered framebuffer list and count. */
+static DEFINE_SPINLOCK(registered_lock);
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
 int num_registered_fb __read_mostly;

@@ -694,9 +696,7 @@  static ssize_t
 fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info *info = file->private_data;
 	u8 *buffer, *dst;
 	u8 __iomem *src;
 	int c, cnt = 0, err = 0;
@@ -705,19 +705,28 @@  fb_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
 	if (!info || ! info->screen_base)
 		return -ENODEV;

-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
+
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}

-	if (info->fbops->fb_read)
-		return info->fbops->fb_read(info, buf, count, ppos);
+	if (info->fbops->fb_read) {
+		err = info->fbops->fb_read(info, buf, count, ppos);
+		goto out_fb_info;
+	}
 	
 	total_size = info->screen_size;

 	if (total_size == 0)
 		total_size = info->fix.smem_len;

-	if (p >= total_size)
-		return 0;
+	if (p >= total_size) {
+		err = 0;
+		goto out_fb_info;
+	}

 	if (count >= total_size)
 		count = total_size;
@@ -727,8 +736,10 @@  fb_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)

 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}

 	src = (u8 __iomem *) (info->screen_base + p);

@@ -751,19 +762,21 @@  fb_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (!err)
+		err = cnt;

 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);

-	return (err) ? err : cnt;
+	return err;
 }

 static ssize_t
 fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info *info = file->private_data;
 	u8 *buffer, *src;
 	u8 __iomem *dst;
 	int c, cnt = 0, err = 0;
@@ -772,8 +785,13 @@  fb_write(struct file *file, const char __user
*buf, size_t count, loff_t *ppos)
 	if (!info || !info->screen_base)
 		return -ENODEV;

-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
+
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}

 	if (info->fbops->fb_write)
 		return info->fbops->fb_write(info, buf, count, ppos);
@@ -783,8 +801,10 @@  fb_write(struct file *file, const char __user
*buf, size_t count, loff_t *ppos)
 	if (total_size == 0)
 		total_size = info->fix.smem_len;

-	if (p > total_size)
-		return -EFBIG;
+	if (p > total_size) {
+		err = -EFBIG;
+		goto out_fb_info;
+	}

 	if (count > total_size) {
 		err = -EFBIG;
@@ -800,8 +820,10 @@  fb_write(struct file *file, const char __user
*buf, size_t count, loff_t *ppos)

 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}

 	dst = (u8 __iomem *) (info->screen_base + p);

@@ -825,10 +847,14 @@  fb_write(struct file *file, const char __user
*buf, size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (cnt)
+		err = cnt;

 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);

-	return (cnt) ? cnt : err;
+	return err;
 }

 int
@@ -1303,8 +1329,7 @@  static long fb_compat_ioctl(struct file *file,
unsigned int cmd,
 static int
 fb_mmap(struct file *file, struct vm_area_struct * vma)
 {
-	int fbidx = iminor(file->f_path.dentry->d_inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info * const info = file->private_data;
 	struct fb_ops *fb = info->fbops;
 	unsigned long off;
 	unsigned long start;
@@ -1316,6 +1341,11 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 	if (!fb)
 		return -ENODEV;
 	mutex_lock(&info->mm_lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		mutex_unlock(&info->mm_lock);
+		return -ENODEV;
+	}
+
 	if (fb->fb_mmap) {
 		int res;
 		res = fb->fb_mmap(info, vma);
@@ -1352,6 +1382,34 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 	return 0;
 }

+static struct fb_info *get_framebuffer_info(int idx)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	struct fb_info *fb_info;
+
+	spin_lock(&registered_lock);
+	fb_info = registered_fb[idx];
+	fb_info->ref_count++;
+	spin_unlock(&registered_lock);
+
+	return fb_info;
+}
+
+static void put_framebuffer_info(struct fb_info *fb_info)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	int keep;
+
+	spin_lock(&registered_lock);
+	keep = --fb_info->ref_count;
+	spin_unlock(&registered_lock);
+	
+	if (!keep && fb_info->fbops->fb_destroy)
+		fb_info->fbops->fb_destroy(fb_info);
+}
+
 static int
 fb_open(struct inode *inode, struct file *file)
 __acquires(&info->lock)
@@ -1363,13 +1421,17 @@  __releases(&info->lock)

 	if (fbidx >= FB_MAX)
 		return -ENODEV;
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		request_module("fb%d", fbidx);
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		return -ENODEV;
 	mutex_lock(&info->lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		res = -ENODEV;
+		goto out;
+	}
 	if (!try_module_get(info->fbops->owner)) {
 		res = -ENODEV;
 		goto out;
@@ -1386,6 +1448,8 @@  __releases(&info->lock)
 #endif
 out:
 	mutex_unlock(&info->lock);
+	if (res)
+		put_framebuffer_info(info);
 	return res;
 }

@@ -1401,6 +1465,7 @@  __releases(&info->lock)
 		info->fbops->fb_release(info,1);
 	module_put(info->fbops->owner);
 	mutex_unlock(&info->lock);
+	put_framebuffer_info(info);
 	return 0;
 }

@@ -1549,6 +1614,7 @@  register_framebuffer(struct fb_info *fb_info)
 	fb_info->node = i;
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
+	fb_info->ref_count = 1;

 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
@@ -1592,7 +1658,6 @@  register_framebuffer(struct fb_info *fb_info)
 	return 0;
 }

-
 /**
  *	unregister_framebuffer - releases a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1627,6 +1692,16 @@  unregister_framebuffer(struct fb_info *fb_info)
 		return -ENODEV;
 	event.info = fb_info;
 	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+	if (!ret) {
+		mutex_lock(&fb_info->mm_lock);
+		/*
+		 * We must prevent any operations for this transition, we
+		 * already have info->lock so grab the info->mm_lock to hold
+		 * the remainder.
+		 */
+		fb_info->state = FBINFO_STATE_REMOVED;
+		mutex_unlock(&fb_info->mm_lock);
+	}
 	unlock_fb_info(fb_info);

 	if (ret) {
@@ -1646,8 +1721,7 @@  unregister_framebuffer(struct fb_info *fb_info)
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);

 	/* this may free fb info */
-	if (fb_info->fbops->fb_destroy)
-		fb_info->fbops->fb_destroy(fb_info);
+	put_framebuffer_info(fb_info);
 done:
 	return ret;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 68ba85a..1e8b785 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -832,6 +832,7 @@  struct fb_tile_ops {
 struct fb_info {
 	int node;
 	int flags;
+	int ref_count;
 	struct mutex lock;		/* Lock for open/release/ioctl funcs */
 	struct mutex mm_lock;		/* Lock for fb_mmap and smem_* fields */
 	struct fb_var_screeninfo var;	/* Current var */
@@ -871,6 +872,7 @@  struct fb_info {
 	void *pseudo_palette;		/* Fake palette of 16 colors */
 #define FBINFO_STATE_RUNNING	0
 #define FBINFO_STATE_SUSPENDED	1
+#define FBINFO_STATE_REMOVED	2
 	u32 state;			/* Hardware state i.e suspend */
 	void *fbcon_par;                /* fbcon use-only private area */
 	/* From here on everything is device dependent */