Hi Andrew, On 09/20/2011 09:11 PM, akpm@xxxxxxxxxx wrote: > From: Andrea Righi <arighi@xxxxxxxxxxx> > Subject: fb: fix potential deadlock between lock_fb_info and console_lock > > fb_set_suspend() must be called with the console semaphore held, which > means the code path coming in here will first take the console_lock() and > then call lock_fb_info(). > > However several framebuffer ioctl commands acquire these locks in reverse > order (lock_fb_info() and then console_lock()). This gives rise to > potential AB-BA deadlock. > > Fix this by changing the order of acquisition in the ioctl commands that > make use of console_lock(). I already have another patch [1] that fixes the same issue in a different way. It looks less risky than yours and got more feedback. Best regards, Florian Tobias Schandinat [1] http://marc.info/?l=linux-kernel&m=130833638508657&w=2 > > Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx> > Reported-by: Peter Nordström (Palm GBU) <peter.nordstrom@xxxxxxxx> > Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxx> > > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxx> > --- > > drivers/video/fbmem.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff -puN drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock drivers/video/fbmem.c > --- a/drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock > +++ a/drivers/video/fbmem.c > @@ -1076,14 +1076,16 @@ static long do_fb_ioctl(struct fb_info * > case FBIOPUT_VSCREENINFO: > if (copy_from_user(&var, argp, sizeof(var))) > return -EFAULT; > - if (!lock_fb_info(info)) > - return -ENODEV; > console_lock(); > + if (!lock_fb_info(info)) { > + console_unlock(); > + return -ENODEV; > + } > info->flags |= FBINFO_MISC_USEREVENT; > ret = fb_set_var(info, &var); > info->flags &= ~FBINFO_MISC_USEREVENT; > - console_unlock(); > unlock_fb_info(info); > + console_unlock(); > if (!ret && copy_to_user(argp, &var, sizeof(var))) > ret = -EFAULT; > break; > @@ -1112,12 +1114,14 @@ static long do_fb_ioctl(struct fb_info * > case FBIOPAN_DISPLAY: > if (copy_from_user(&var, argp, sizeof(var))) > return -EFAULT; > - if (!lock_fb_info(info)) > - return -ENODEV; > console_lock(); > + if (!lock_fb_info(info)) { > + console_unlock(); > + return -ENODEV; > + } > ret = fb_pan_display(info, &var); > - console_unlock(); > unlock_fb_info(info); > + console_unlock(); > if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) > return -EFAULT; > break; > @@ -1159,14 +1163,16 @@ static long do_fb_ioctl(struct fb_info * > unlock_fb_info(info); > break; > case FBIOBLANK: > - if (!lock_fb_info(info)) > - return -ENODEV; > console_lock(); > + if (!lock_fb_info(info)) { > + console_unlock(); > + return -ENODEV; > + } > info->flags |= FBINFO_MISC_USEREVENT; > ret = fb_blank(info, arg); > info->flags &= ~FBINFO_MISC_USEREVENT; > - console_unlock(); > unlock_fb_info(info); > + console_unlock(); > break; > default: > if (!lock_fb_info(info)) > _ > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
