diff options
author | Waiman Long <longman@redhat.com> | 2016-11-23 14:06:45 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2016-11-29 21:42:40 +0100 |
commit | 7edd7e82b96bcbcc2f9437449df29fde0f08295e (patch) | |
tree | 56a205cd9e02c8a7606f46d287997140d25f8559 | |
parent | 5020ded78348092eac5e9909018f6d53e24eadb6 (diff) | |
download | linux-7edd7e82b96bcbcc2f9437449df29fde0f08295e.tar.bz2 |
console: Move userspace I/O out of console_lock to fix lockdep warning
When running certain workload on a debug kernel with lockdep turned on,
a ppc64 kvm guest could sometimes hit the following lockdep warning:
[ INFO: possible circular locking dependency detected ]
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&mm->mmap_sem);
lock(console_lock);
lock(&mm->mmap_sem);
lock(cpu_hotplug.lock);
*** DEADLOCK ***
Looking at the console code, the console_lock-->mmap_sem scenario will
only happen when reading or writing the console unicode map leading to
a page fault.
To break this circular locking dependency, all the userspace I/O
operations in consolemap.c are now moved outside of the console_lock
critical sections so that the mmap_sem won't be acquired when holding
the console_lock.
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r-- | drivers/tty/vt/consolemap.c | 115 |
1 files changed, 74 insertions, 41 deletions
diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index 9d7ab7b66a8a..71e81406ef71 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -9,6 +9,17 @@ * Support for multiple unimaps by Jakub Jelinek <jj@ultra.linux.cz>, July 1998 * * Fix bug in inverse translation. Stanislav Voronyi <stas@cnti.uanet.kharkov.ua>, Dec 1998 + * + * In order to prevent the following circular lock dependency: + * &mm->mmap_sem --> cpu_hotplug.lock --> console_lock --> &mm->mmap_sem + * + * We cannot allow page fault to happen while holding the console_lock. + * Therefore, all the userspace copy operations have to be done outside + * the console_lock critical sections. + * + * As all the affected functions are all called directly from vt_ioctl(), we + * can allocate some small buffers directly on stack without worrying about + * stack overflow. */ #include <linux/module.h> @@ -22,6 +33,7 @@ #include <linux/console.h> #include <linux/consolemap.h> #include <linux/vt_kern.h> +#include <linux/string.h> static unsigned short translations[][256] = { /* 8-bit Latin-1 mapped to Unicode -- trivial mapping */ @@ -309,18 +321,19 @@ static void update_user_maps(void) int con_set_trans_old(unsigned char __user * arg) { int i; - unsigned short *p = translations[USER_MAP]; + unsigned short inbuf[E_TABSZ]; if (!access_ok(VERIFY_READ, arg, E_TABSZ)) return -EFAULT; - console_lock(); - for (i=0; i<E_TABSZ ; i++) { + for (i = 0; i < E_TABSZ ; i++) { unsigned char uc; __get_user(uc, arg+i); - p[i] = UNI_DIRECT_BASE | uc; + inbuf[i] = UNI_DIRECT_BASE | uc; } + console_lock(); + memcpy(translations[USER_MAP], inbuf, sizeof(inbuf)); update_user_maps(); console_unlock(); return 0; @@ -330,35 +343,37 @@ int con_get_trans_old(unsigned char __user * arg) { int i, ch; unsigned short *p = translations[USER_MAP]; + unsigned char outbuf[E_TABSZ]; if (!access_ok(VERIFY_WRITE, arg, E_TABSZ)) return -EFAULT; console_lock(); - for (i=0; i<E_TABSZ ; i++) + for (i = 0; i < E_TABSZ ; i++) { ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]); - __put_user((ch & ~0xff) ? 0 : ch, arg+i); + outbuf[i] = (ch & ~0xff) ? 0 : ch; } console_unlock(); + + for (i = 0; i < E_TABSZ ; i++) + __put_user(outbuf[i], arg+i); return 0; } int con_set_trans_new(ushort __user * arg) { int i; - unsigned short *p = translations[USER_MAP]; + unsigned short inbuf[E_TABSZ]; if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short))) return -EFAULT; - console_lock(); - for (i=0; i<E_TABSZ ; i++) { - unsigned short us; - __get_user(us, arg+i); - p[i] = us; - } + for (i = 0; i < E_TABSZ ; i++) + __get_user(inbuf[i], arg+i); + console_lock(); + memcpy(translations[USER_MAP], inbuf, sizeof(inbuf)); update_user_maps(); console_unlock(); return 0; @@ -367,16 +382,17 @@ int con_set_trans_new(ushort __user * arg) int con_get_trans_new(ushort __user * arg) { int i; - unsigned short *p = translations[USER_MAP]; + unsigned short outbuf[E_TABSZ]; if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short))) return -EFAULT; console_lock(); - for (i=0; i<E_TABSZ ; i++) - __put_user(p[i], arg+i); + memcpy(outbuf, translations[USER_MAP], sizeof(outbuf)); console_unlock(); - + + for (i = 0; i < E_TABSZ ; i++) + __put_user(outbuf[i], arg+i); return 0; } @@ -536,10 +552,20 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) { int err = 0, err1, i; struct uni_pagedir *p, *q; + struct unipair *unilist, *plist; if (!ct) return 0; + unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); + if (!unilist) + return -ENOMEM; + + for (i = ct, plist = unilist; i; i--, plist++, list++) { + __get_user(plist->unicode, &list->unicode); + __get_user(plist->fontpos, &list->fontpos); + } + console_lock(); /* Save original vc_unipagdir_loc in case we allocate a new one */ @@ -557,8 +583,8 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) err1 = con_do_clear_unimap(vc); if (err1) { - console_unlock(); - return err1; + err = err1; + goto out_unlock; } /* @@ -592,8 +618,8 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) *vc->vc_uni_pagedir_loc = p; con_release_unimap(q); kfree(q); - console_unlock(); - return err1; + err = err1; + goto out_unlock; } } } else { @@ -617,22 +643,17 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) /* * Insert user specified unicode pairs into new table. */ - while (ct--) { - unsigned short unicode, fontpos; - __get_user(unicode, &list->unicode); - __get_user(fontpos, &list->fontpos); - if ((err1 = con_insert_unipair(p, unicode,fontpos)) != 0) + for (plist = unilist; ct; ct--, plist++) { + err1 = con_insert_unipair(p, plist->unicode, plist->fontpos); + if (err1) err = err1; - list++; } /* * Merge with fontmaps of any other virtual consoles. */ - if (con_unify_unimap(vc, p)) { - console_unlock(); - return err; - } + if (con_unify_unimap(vc, p)) + goto out_unlock; for (i = 0; i <= 3; i++) set_inverse_transl(vc, p, i); /* Update inverse translations */ @@ -640,6 +661,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) out_unlock: console_unlock(); + kfree(unilist); return err; } @@ -735,9 +757,15 @@ EXPORT_SYMBOL(con_copy_unimap); */ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list) { - int i, j, k, ect; + int i, j, k; + ushort ect; u16 **p1, *p2; struct uni_pagedir *p; + struct unipair *unilist, *plist; + + unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); + if (!unilist) + return -ENOMEM; console_lock(); @@ -750,21 +778,26 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni for (j = 0; j < 32; j++) { p2 = *(p1++); if (p2) - for (k = 0; k < 64; k++) { - if (*p2 < MAX_GLYPH && ect++ < ct) { - __put_user((u_short)((i<<11)+(j<<6)+k), - &list->unicode); - __put_user((u_short) *p2, - &list->fontpos); - list++; + for (k = 0; k < 64; k++, p2++) { + if (*p2 >= MAX_GLYPH) + continue; + if (ect < ct) { + unilist[ect].unicode = + (i<<11)+(j<<6)+k; + unilist[ect].fontpos = *p2; } - p2++; + ect++; } } } } - __put_user(ect, uct); console_unlock(); + for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) { + __put_user(plist->unicode, &list->unicode); + __put_user(plist->fontpos, &list->fontpos); + } + __put_user(ect, uct); + kfree(unilist); return ((ect <= ct) ? 0 : -ENOMEM); } |