FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 03-25-2010, 05:27 PM
Bryce Harrington
 
Default UBUNTU: SAUCE: Don't register vga16fb framebuffer if other framebuffers are present

This is great to see. My one suggestion would be perhaps making the log
message be a warning (or just info) rather than an error, to prevent
users mistakenly thinking lack of vga16fb is causing some other random
bug they're having?

On Thu, Mar 25, 2010 at 12:53:30PM -0400, Chase Douglas wrote:
> Using the vga16fb framebuffer is not safe when other framebuffers are
> present. This fixes the case where the vga16fb module is loaded after
> a better framebuffer is loaded (since vga16fb is by definition the
> worst-case framebuffer).
>
> There does not appear to be any locking around the num_registered_fb, so
> this is a hack at best. However, in Lucid we build vga16fb as a module.
> Modules are loaded serially, so this should be ok for Lucid. In M, we
> will transition to efifb and drop vga16fb, so this is a one time hack.
>
> This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb
> through /dev/fb1.
>
> BugLink: http://bugs.launchpad.net/bugs/527369
>
> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> ---
> drivers/video/vga16fb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
> index efde41d..b1d62ef 100644
> --- a/drivers/video/vga16fb.c
> +++ b/drivers/video/vga16fb.c
> @@ -1355,7 +1355,7 @@ static int __init vga16fb_probe(struct platform_device *dev)
>
> vga16fb_update_fix(info);
>
> - if (register_framebuffer(info) < 0) {
> + if (num_registered_fb > 0 || register_framebuffer(info) < 0) {
> printk(KERN_ERR "vga16fb: unable to register framebuffer
");
> ret = -EINVAL;
> goto err_check_var;
> --
> 1.7.0
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-25-2010, 05:34 PM
Stefan Bader
 
Default UBUNTU: SAUCE: Don't register vga16fb framebuffer if other framebuffers are present

Bryce Harrington wrote:
> This is great to see. My one suggestion would be perhaps making the log
> message be a warning (or just info) rather than an error, to prevent
> users mistakenly thinking lack of vga16fb is causing some other random
> bug they're having?
>
> On Thu, Mar 25, 2010 at 12:53:30PM -0400, Chase Douglas wrote:
>> Using the vga16fb framebuffer is not safe when other framebuffers are
>> present. This fixes the case where the vga16fb module is loaded after
>> a better framebuffer is loaded (since vga16fb is by definition the
>> worst-case framebuffer).
>>
>> There does not appear to be any locking around the num_registered_fb, so
>> this is a hack at best. However, in Lucid we build vga16fb as a module.
>> Modules are loaded serially, so this should be ok for Lucid. In M, we
>> will transition to efifb and drop vga16fb, so this is a one time hack.
>>
>> This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb
>> through /dev/fb1.
>>
>> BugLink: http://bugs.launchpad.net/bugs/527369
>>
>> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
>> ---
>> drivers/video/vga16fb.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
>> index efde41d..b1d62ef 100644
>> --- a/drivers/video/vga16fb.c
>> +++ b/drivers/video/vga16fb.c
>> @@ -1355,7 +1355,7 @@ static int __init vga16fb_probe(struct platform_device *dev)
>>
>> vga16fb_update_fix(info);
>>
>> - if (register_framebuffer(info) < 0) {
>> + if (num_registered_fb > 0 || register_framebuffer(info) < 0) {
>> printk(KERN_ERR "vga16fb: unable to register framebuffer
");
>> ret = -EINVAL;
>> goto err_check_var;
>> --
>> 1.7.0
>>
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
This might be an idea. Ok the patch would get a little bigger but probably we
want at least to differentiate between the case where it cannot register
compared to the one where it does not want to.
And I want to sleep a night over it. It feels a bit like a case where dragons
might be hidden. Though we probably want to have it in quick to catch any
outfall rather sooner that later.

Stefan

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 03-26-2010, 07:09 AM
Amit Kucheria
 
Default UBUNTU: SAUCE: Don't register vga16fb framebuffer if other framebuffers are present

On 10 Mar 25, Chase Douglas wrote:
> Using the vga16fb framebuffer is not safe when other framebuffers are
> present. This fixes the case where the vga16fb module is loaded after
> a better framebuffer is loaded (since vga16fb is by definition the
> worst-case framebuffer).
>
> There does not appear to be any locking around the num_registered_fb, so
> this is a hack at best. However, in Lucid we build vga16fb as a module.
> Modules are loaded serially, so this should be ok for Lucid. In M, we
> will transition to efifb and drop vga16fb, so this is a one time hack.
>
> This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb
> through /dev/fb1.
>
> BugLink: http://bugs.launchpad.net/bugs/527369
>
> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> ---
> drivers/video/vga16fb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
> index efde41d..b1d62ef 100644
> --- a/drivers/video/vga16fb.c
> +++ b/drivers/video/vga16fb.c
> @@ -1355,7 +1355,7 @@ static int __init vga16fb_probe(struct platform_device *dev)
>
> vga16fb_update_fix(info);
>
> - if (register_framebuffer(info) < 0) {
> + if (num_registered_fb > 0 || register_framebuffer(info) < 0) {
> printk(KERN_ERR "vga16fb: unable to register framebuffer
");
> ret = -EINVAL;
> goto err_check_var;
> --
> 1.7.0

We should differentiate the case when there was a real error from the one
where we bailed out on purpose. I think the extra cost to patch size would be
justified.

Regards,
Amit

--
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || amit.kucheria@canonical.com
----------------------------------------------------------------------

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 06:43 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org