Hi Linus, Andrew,
Here are 2 more fixes for ps3fb:
[1] ps3fb: prevent use after free of fb_info
[2] ps3fb: fix deadlock on kexec()
The first one fixes a potential use after free.
The second one fixes a lockup when using kexec or shutdown while /dev/fb0 is
open (e.g. when using the petitboot graphical bootloader to load the second
stage kernel).
Can we please get these in 2.6.24? Thanks!
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
From: Jeremy Kerr <jk@ozlabs.org>
Since the introduction of the acquire_console_sem calls in
0333d83509c7d8496c8965b5ba9bc0c98e83c259, kexecing can cause the
kernel to deadlock:
ps3fb_shutdown()
-> unregister_framebuffer()
-> fb_notifier_call_chain(FB_EVENT_FB_UNBIND)
-> fbcon_fb_unbind()
-> unbind_con_driver()
-> bind_con_driver()
[ acquires console_sem ]
-> fbcon_deinit()
-> fbops->fb_release(newinfo, 0)
-> ps3fb_release()
-> ps3fb_sync()
[ acquires console_sem ]
This change avoids the deadlock by moving the acquire_console_sem()
out of ps3fb_sync(), and puts it into the two other callsites, leaving
ps3fb_release() to call ps3fb_sync() without the console semaphore.
[Geert]
- Corrected call sequence above
- ps3fb_release() may be called with and without console_sem held. This is an
inconsistency that should be fixed at the fb level, but for now, try to
acquire console_sem in ps3fb_release().
I think it's safer to let ps3fb_release() try to acquire console_sem and
not refresh the screen if it fails, than to call ps3fb_sync() without
holding console_sem, as ps3fb_par may be modified at the same time, causing
crashes or lockups.
Besides, ps3fb_release() only calls ps3fb_sync() to refresh the screen
when display flipping is disabled, which is an uncommon case (except during
shutdown/kexec).
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Geert Uytterhoeven <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/video/ps3fb.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -443,8 +443,6 @@ static int ps3fb_sync(struct fb_info *in
u32 ddr_line_length, xdr_line_length;
u64 ddr_base, xdr_base;
- acquire_console_sem();
-
if (frame > par->num_frames - 1) {
dev_dbg(info->device, "%s: invalid frame number (%u)\n",
__func__, frame);@@ -464,7 +462,6 @@ static int ps3fb_sync(struct fb_info *in
xdr_line_length);
out:
- release_console_sem();
return error;
}
@@ -479,7 +476,10 @@ static int ps3fb_release(struct fb_info
if (atomic_dec_and_test(&ps3fb.f_count)) {
if (atomic_read(&ps3fb.ext_flip)) {
atomic_set(&ps3fb.ext_flip, 0);
- ps3fb_sync(info, 0); /* single buffer */
+ if (!try_acquire_console_sem()) {
+ ps3fb_sync(info, 0); /* single buffer */
+ release_console_sem();
+ }
}
}
return 0;@@ -865,7 +865,9 @@ static int ps3fb_ioctl(struct fb_info *i
break;
dev_dbg(info->device, "PS3FB_IOCTL_FSEL:%d\n", val);
+ acquire_console_sem();
retval = ps3fb_sync(info, val);
+ release_console_sem();
break;
default:
@@ -885,7 +887,9 @@ static int ps3fbd(void *arg)
set_current_state(TASK_INTERRUPTIBLE);
if (ps3fb.is_kicked) {
ps3fb.is_kicked = 0;
+ acquire_console_sem();
ps3fb_sync(info, 0); /* single buffer */
+ release_console_sem();
}
schedule();
}
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
From: Jeremy Kerr <jk@ozlabs.org>
In ps3fb_shutdown, freeing the framebuffer will cause fb_info (in
dev->core.driver_data) to be free()ed, which we potentially access
from the ps3fbd kthread.
This change frees the framebuffer after stopping the ps3fbd kthread.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Geert Uytterhoeven <redacted>
---
drivers/video/ps3fb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -1234,12 +1234,6 @@ static int ps3fb_shutdown(struct ps3_sys
ps3fb_flip_ctl(0, &ps3fb); /* flip off */
ps3fb.dinfo->irq.mask = 0;
- if (info) {
- unregister_framebuffer(info);
- fb_dealloc_cmap(&info->cmap);
- framebuffer_release(info);
- }
-
ps3av_register_flip_ctl(NULL, NULL);
if (ps3fb.task) {
struct task_struct *task = ps3fb.task;@@ -1250,6 +1244,12 @@ static int ps3fb_shutdown(struct ps3_sys
free_irq(ps3fb.irq_no, &dev->core);
ps3_irq_plug_destroy(ps3fb.irq_no);
}
+ if (info) {
+ unregister_framebuffer(info);
+ fb_dealloc_cmap(&info->cmap);
+ framebuffer_release(info);
+ info = dev->core.driver_data = NULL;
+ }
iounmap((u8 __iomem *)ps3fb.dinfo);
status = lv1_gpu_context_free(ps3fb.context_handle);
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619