Messages in this thread | | | Date | Mon, 19 Jan 2009 21:54:27 +0100 | From | Krzysztof Helt <> | Subject | Re: [PATCH] fbmem: copy_from/to_user() with mutex held (v2) |
| |
On Mon, 19 Jan 2009 12:04:04 +0100 Andrea Righi <righi.andrea@gmail.com> wrote:
> Avoid to call copy_from/to_user() with fb_info->lock mutex held in fbmem > ioctl(). > > fb_mmap() is called under mm->mmap_sem (A) held, that also acquires > fb_info->lock (B); fb_ioctl() takes fb_info->lock (B) and does > copy_from/to_user() that might acquire mm->mmap_sem (A), causing a > deadlock. > > NOTE: it doesn't push down the fb_info->lock in each own driver's > fb_ioctl(), so there're still potential deadlocks somewhere. >
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c > index 756efeb..fe6033e 100644 > --- a/drivers/video/fbmem.c > +++ b/drivers/video/fbmem.c > @@ -1013,25 +1013,26 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, (...) > case FBIOPUT_VSCREENINFO: > - if (copy_from_user(&var, argp, sizeof(var))) { > - ret = -EFAULT; > - break; > - } > + if (copy_from_user(&var, argp, sizeof(var))) > + return -EFAULT; > + if (!lock_fb_info(info)) > + return -ENODEV; > acquire_console_sem(); > info->flags |= FBINFO_MISC_USEREVENT; > ret = fb_set_var(info, &var); > @@ -1041,104 +1042,109 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, > ret = -EFAULT; > break;
I don't see a call to unlock_fb_info(info) in the case for FBIOPUT_VSCREENINFO. Is it intentional?
Kind regards, Krzysztof
| |