fbdev: fix info->lock deadlock in fbcon_event_notify()

fb_notifier_call_chain() is called with info->lock held, i.e. in do_fb_ioctl() => FBIOPUT_VSCREENINFO => fb_set_var() and the some notifier callbacks, like fbcon_event_notify(), try to re-acquire info->lock again. Remove the lock/unlock_fb_info() in all the framebuffer notifier callbacks' and be sure to always call fb_notifier_call_chain() with info->lock held. Reported-by: Pavel Roskin <proski@gnu.org> Reported-by: Eric Miao <eric.y.miao@gmail.com> Signed-off-by: Andrea Righi <righi.andrea@gmail.com> Cc: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: Krzysztof Helt <krzysztof.h1@poczta.fm> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

fbdev: fix info->lock deadlock in fbcon_event_notify()
fb_notifier_call_chain() is called with info->lock held, i.e. in do_fb_ioctl() => FBIOPUT_VSCREENINFO => fb_set_var() and the some notifier callbacks, like fbcon_event_notify(), try to re-acquire info->lock again. Remove the lock/unlock_fb_info() in all the framebuffer notifier callbacks' and be sure to always call fb_notifier_call_chain() with info->lock held. Reported-by: Pavel Roskin <proski@gnu.org> Reported-by: Eric Miao <eric.y.miao@gmail.com> Signed-off-by: Andrea Righi <righi.andrea@gmail.com> Cc: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: Krzysztof Helt <krzysztof.h1@poczta.fm> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Andrea Righi · Linus Torvalds
1 parent b52bb3712a
Showing 4 changed files with 22 additions and 58 deletions Side-by-side Diff
drivers/video/backlight/backlight.c
drivers/video/backlight/lcd.c
drivers/video/console/fbcon.c
drivers/video/fbmem.c
@@ -35,8 +35,6 @@
 		return 0;
  
 	bd = container_of(self, struct backlight_device, fb_notif);
-	if (!lock_fb_info(evdata->info))
-		return -ENODEV;
 	mutex_lock(&bd->ops_lock);
 	if (bd->ops)
 		if (!bd->ops->check_fb ||
@@ -49,7 +47,6 @@
 			backlight_update_status(bd);
 		}
 	mutex_unlock(&bd->ops_lock);
-	unlock_fb_info(evdata->info);
 	return 0;
 }
  
@@ -40,8 +40,6 @@
 	if (!ld->ops)
 		return 0;
  
-	if (!lock_fb_info(evdata->info))
-		return -ENODEV;
 	mutex_lock(&ld->ops_lock);
 	if (!ld->ops->check_fb || ld->ops->check_fb(ld, evdata->info)) {
 		if (event == FB_EVENT_BLANK) {
@@ -53,7 +51,6 @@
 		}
 	}
 	mutex_unlock(&ld->ops_lock);
-	unlock_fb_info(evdata->info);
 	return 0;
 }
  
@@ -2263,9 +2263,12 @@
 	}
  
  
+	if (!lock_fb_info(info))
+		return;
 	event.info = info;
 	event.data = &blank;
 	fb_notifier_call_chain(FB_EVENT_CONBLANK, &event);
+	unlock_fb_info(info);
 }
  
 static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
@@ -2956,8 +2959,6 @@
 {
 	int i, idx;
  
-	if (!lock_fb_info(info))
-		return -ENODEV;
 	idx = info->node;
 	for (i = first_fb_vc; i <= last_fb_vc; i++) {
 		if (con2fb_map[i] == idx)
@@ -2985,8 +2986,6 @@
 	if (primary_device == idx)
 		primary_device = -1;
  
-	unlock_fb_info(info);
-
 	if (!num_registered_fb)
 		unregister_con_driver(&fb_con);
  
  
@@ -3027,11 +3026,8 @@
 {
 	int ret = 0, i, idx;
  
-	if (!lock_fb_info(info))
-		return -ENODEV;
 	idx = info->node;
 	fbcon_select_primary(info);
-	unlock_fb_info(info);
  
 	if (info_idx == -1) {
 		for (i = first_fb_vc; i <= last_fb_vc; i++) {
  
  
  
  
  
  
  
  
  
  
  
@@ -3152,53 +3148,23 @@
  
 	switch(action) {
 	case FB_EVENT_SUSPEND:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_suspended(info);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_RESUME:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_resumed(info);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_MODE_CHANGE:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_modechanged(info);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_MODE_CHANGE_ALL:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_set_all_vcs(info);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_MODE_DELETE:
 		mode = event->data;
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		ret = fbcon_mode_deleted(info, mode);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_FB_UNBIND:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		idx = info->node;
-		unlock_fb_info(info);
 		ret = fbcon_fb_unbind(idx);
 		break;
 	case FB_EVENT_FB_REGISTERED:
  
  
  
  
  
@@ -3217,29 +3183,14 @@
 		con2fb->framebuffer = con2fb_map[con2fb->console - 1];
 		break;
 	case FB_EVENT_BLANK:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_fb_blanked(info, *(int *)event->data);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_NEW_MODELIST:
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_new_modelist(info);
-		unlock_fb_info(info);
 		break;
 	case FB_EVENT_GET_REQ:
 		caps = event->data;
-		if (!lock_fb_info(info)) {
-			ret = -ENODEV;
-			goto done;
-		}
 		fbcon_get_requirement(info, caps);
-		unlock_fb_info(info);
 		break;
 	}
 done:
@@ -1097,8 +1097,11 @@
 			return -EINVAL;
 		con2fb.framebuffer = -1;
 		event.data = &con2fb;
+		if (!lock_fb_info(info))
+			return -ENODEV;
 		event.info = info;
 		fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
+		unlock_fb_info(info);
 		ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
 		break;
 	case FBIOPUT_CON2FBMAP:
  
@@ -1115,8 +1118,11 @@
 			break;
 		}
 		event.data = &con2fb;
+		if (!lock_fb_info(info))
+			return -ENODEV;
 		event.info = info;
 		ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
+		unlock_fb_info(info);
 		break;
 	case FBIOBLANK:
 		if (!lock_fb_info(info))
  
@@ -1521,7 +1527,10 @@
 	registered_fb[i] = fb_info;
  
 	event.info = fb_info;
+	if (!lock_fb_info(fb_info))
+		return -ENODEV;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+	unlock_fb_info(fb_info);
 	return 0;
 }
  
  
@@ -1555,8 +1564,12 @@
 		goto done;
 	}
  
+
+	if (!lock_fb_info(fb_info))
+		return -ENODEV;
 	event.info = fb_info;
 	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+	unlock_fb_info(fb_info);
  
 	if (ret) {
 		ret = -EINVAL;
@@ -1590,6 +1603,8 @@
 {
 	struct fb_event event;
  
+	if (!lock_fb_info(info))
+		return;
 	event.info = info;
 	if (state) {
 		fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
@@ -1598,6 +1613,7 @@
 		info->state = FBINFO_STATE_RUNNING;
 		fb_notifier_call_chain(FB_EVENT_RESUME, &event);
 	}
+	unlock_fb_info(info);
 }
  
 /**
  
@@ -1667,8 +1683,11 @@
 	err = 1;
  
 	if (!list_empty(&info->modelist)) {
+		if (!lock_fb_info(info))
+			return -ENODEV;
 		event.info = info;
 		err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event);
+		unlock_fb_info(info);
 	}
  
 	return err;
...	...	@@ -35,8 +35,6 @@
35	35	return 0;
36	36
37	37	bd = container_of(self, struct backlight_device, fb_notif);
38		- if (!lock_fb_info(evdata->info))
39		- return -ENODEV;
40	38	mutex_lock(&bd->ops_lock);
41	39	if (bd->ops)
42	40	if (!bd->ops->check_fb \|\|
...	...	@@ -49,7 +47,6 @@
49	47	backlight_update_status(bd);
50	48	}
51	49	mutex_unlock(&bd->ops_lock);
52		- unlock_fb_info(evdata->info);
53	50	return 0;
54	51	}
55	52
...	...	@@ -40,8 +40,6 @@
40	40	if (!ld->ops)
41	41	return 0;
42	42
43		- if (!lock_fb_info(evdata->info))
44		- return -ENODEV;
45	43	mutex_lock(&ld->ops_lock);
46	44	if (!ld->ops->check_fb \|\| ld->ops->check_fb(ld, evdata->info)) {
47	45	if (event == FB_EVENT_BLANK) {
...	...	@@ -53,7 +51,6 @@
53	51	}
54	52	}
55	53	mutex_unlock(&ld->ops_lock);
56		- unlock_fb_info(evdata->info);
57	54	return 0;
58	55	}
59	56
...	...	@@ -2263,9 +2263,12 @@
2263	2263	}
2264	2264
2265	2265
	2266	+ if (!lock_fb_info(info))
	2267	+ return;
2266	2268	event.info = info;
2267	2269	event.data = &blank;
2268	2270	fb_notifier_call_chain(FB_EVENT_CONBLANK, &event);
	2271	+ unlock_fb_info(info);
2269	2272	}
2270	2273
2271	2274	static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
...	...	@@ -2956,8 +2959,6 @@
2956	2959	{
2957	2960	int i, idx;
2958	2961
2959		- if (!lock_fb_info(info))
2960		- return -ENODEV;
2961	2962	idx = info->node;
2962	2963	for (i = first_fb_vc; i <= last_fb_vc; i++) {
2963	2964	if (con2fb_map[i] == idx)
...	...	@@ -2985,8 +2986,6 @@
2985	2986	if (primary_device == idx)
2986	2987	primary_device = -1;
2987	2988
2988		- unlock_fb_info(info);
2989		-
2990	2989	if (!num_registered_fb)
2991	2990	unregister_con_driver(&fb_con);
2992	2991
2993	2992
...	...	@@ -3027,11 +3026,8 @@
3027	3026	{
3028	3027	int ret = 0, i, idx;
3029	3028
3030		- if (!lock_fb_info(info))
3031		- return -ENODEV;
3032	3029	idx = info->node;
3033	3030	fbcon_select_primary(info);
3034		- unlock_fb_info(info);
3035	3031
3036	3032	if (info_idx == -1) {
3037	3033	for (i = first_fb_vc; i <= last_fb_vc; i++) {
3038	3034
3039	3035
3040	3036
3041	3037
3042	3038
3043	3039
3044	3040
3045	3041
3046	3042
3047	3043
3048	3044
...	...	@@ -3152,53 +3148,23 @@
3152	3148
3153	3149	switch(action) {
3154	3150	case FB_EVENT_SUSPEND:
3155		- if (!lock_fb_info(info)) {
3156		- ret = -ENODEV;
3157		- goto done;
3158		- }
3159	3151	fbcon_suspended(info);
3160		- unlock_fb_info(info);
3161	3152	break;
3162	3153	case FB_EVENT_RESUME:
3163		- if (!lock_fb_info(info)) {
3164		- ret = -ENODEV;
3165		- goto done;
3166		- }
3167	3154	fbcon_resumed(info);
3168		- unlock_fb_info(info);
3169	3155	break;
3170	3156	case FB_EVENT_MODE_CHANGE:
3171		- if (!lock_fb_info(info)) {
3172		- ret = -ENODEV;
3173		- goto done;
3174		- }
3175	3157	fbcon_modechanged(info);
3176		- unlock_fb_info(info);
3177	3158	break;
3178	3159	case FB_EVENT_MODE_CHANGE_ALL:
3179		- if (!lock_fb_info(info)) {
3180		- ret = -ENODEV;
3181		- goto done;
3182		- }
3183	3160	fbcon_set_all_vcs(info);
3184		- unlock_fb_info(info);
3185	3161	break;
3186	3162	case FB_EVENT_MODE_DELETE:
3187	3163	mode = event->data;
3188		- if (!lock_fb_info(info)) {
3189		- ret = -ENODEV;
3190		- goto done;
3191		- }
3192	3164	ret = fbcon_mode_deleted(info, mode);
3193		- unlock_fb_info(info);
3194	3165	break;
3195	3166	case FB_EVENT_FB_UNBIND:
3196		- if (!lock_fb_info(info)) {
3197		- ret = -ENODEV;
3198		- goto done;
3199		- }
3200	3167	idx = info->node;
3201		- unlock_fb_info(info);
3202	3168	ret = fbcon_fb_unbind(idx);
3203	3169	break;
3204	3170	case FB_EVENT_FB_REGISTERED:
3205	3171
3206	3172
3207	3173
3208	3174
3209	3175
...	...	@@ -3217,29 +3183,14 @@
3217	3183	con2fb->framebuffer = con2fb_map[con2fb->console - 1];
3218	3184	break;
3219	3185	case FB_EVENT_BLANK:
3220		- if (!lock_fb_info(info)) {
3221		- ret = -ENODEV;
3222		- goto done;
3223		- }
3224	3186	fbcon_fb_blanked(info, (int )event->data);
3225		- unlock_fb_info(info);
3226	3187	break;
3227	3188	case FB_EVENT_NEW_MODELIST:
3228		- if (!lock_fb_info(info)) {
3229		- ret = -ENODEV;
3230		- goto done;
3231		- }
3232	3189	fbcon_new_modelist(info);
3233		- unlock_fb_info(info);
3234	3190	break;
3235	3191	case FB_EVENT_GET_REQ:
3236	3192	caps = event->data;
3237		- if (!lock_fb_info(info)) {
3238		- ret = -ENODEV;
3239		- goto done;
3240		- }
3241	3193	fbcon_get_requirement(info, caps);
3242		- unlock_fb_info(info);
3243	3194	break;
3244	3195	}
3245	3196	done:
...	...	@@ -1097,8 +1097,11 @@
1097	1097	return -EINVAL;
1098	1098	con2fb.framebuffer = -1;
1099	1099	event.data = &con2fb;
	1100	+ if (!lock_fb_info(info))
	1101	+ return -ENODEV;
1100	1102	event.info = info;
1101	1103	fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
	1104	+ unlock_fb_info(info);
1102	1105	ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
1103	1106	break;
1104	1107	case FBIOPUT_CON2FBMAP:
1105	1108
...	...	@@ -1115,8 +1118,11 @@
1115	1118	break;
1116	1119	}
1117	1120	event.data = &con2fb;
	1121	+ if (!lock_fb_info(info))
	1122	+ return -ENODEV;
1118	1123	event.info = info;
1119	1124	ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
	1125	+ unlock_fb_info(info);
1120	1126	break;
1121	1127	case FBIOBLANK:
1122	1128	if (!lock_fb_info(info))
1123	1129
...	...	@@ -1521,7 +1527,10 @@
1521	1527	registered_fb[i] = fb_info;
1522	1528
1523	1529	event.info = fb_info;
	1530	+ if (!lock_fb_info(fb_info))
	1531	+ return -ENODEV;
1524	1532	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
	1533	+ unlock_fb_info(fb_info);
1525	1534	return 0;
1526	1535	}
1527	1536
1528	1537
...	...	@@ -1555,8 +1564,12 @@
1555	1564	goto done;
1556	1565	}
1557	1566
	1567	+
	1568	+ if (!lock_fb_info(fb_info))
	1569	+ return -ENODEV;
1558	1570	event.info = fb_info;
1559	1571	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
	1572	+ unlock_fb_info(fb_info);
1560	1573
1561	1574	if (ret) {
1562	1575	ret = -EINVAL;
...	...	@@ -1590,6 +1603,8 @@
1590	1603	{
1591	1604	struct fb_event event;
1592	1605
	1606	+ if (!lock_fb_info(info))
	1607	+ return;
1593	1608	event.info = info;
1594	1609	if (state) {
1595	1610	fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
...	...	@@ -1598,6 +1613,7 @@
1598	1613	info->state = FBINFO_STATE_RUNNING;
1599	1614	fb_notifier_call_chain(FB_EVENT_RESUME, &event);
1600	1615	}
	1616	+ unlock_fb_info(info);
1601	1617	}
1602	1618
1603	1619	/**
1604	1620
...	...	@@ -1667,8 +1683,11 @@
1667	1683	err = 1;
1668	1684
1669	1685	if (!list_empty(&info->modelist)) {
	1686	+ if (!lock_fb_info(info))
	1687	+ return -ENODEV;
1670	1688	event.info = info;
1671	1689	err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event);
	1690	+ unlock_fb_info(info);
1672	1691	}
1673	1692
1674	1693	return err;