From 7edd7e82b96bcbcc2f9437449df29fde0f08295e Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Wed, 23 Nov 2016 14:06:45 -0500 Subject: 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 Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/consolemap.c | 115 ++++++++++++++++++++++++++++---------------- 1 file changed, 74 insertions(+), 41 deletions(-) (limited to 'drivers/tty/vt') 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 , July 1998 * * Fix bug in inverse translation. Stanislav Voronyi , 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 @@ -22,6 +33,7 @@ #include #include #include +#include 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; iunicode, &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); } -- cgit v1.2.3