| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1479928005-24084-1-git-send-email-longman@redhat.com>
Date: Wed, 23 Nov 2016 14:06:45 -0500
From: Waiman Long <longman@...hat.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>
Cc: linux-kernel@...r.kernel.org, Jan Stancek <jstancek@...hat.com>,
Waiman Long <longman@...hat.com>
Subject: [PATCH] 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@...hat.com>
---
drivers/tty/vt/consolemap.c | 115 ++++++++++++++++++++++++++++----------------
1 file changed, 74 insertions(+), 41 deletions(-)
diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 9d7ab7b..71e8140 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@...ra.linux.cz>, July 1998
*
* Fix bug in inverse translation. Stanislav Voronyi <stas@...i.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 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
*/
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);
}
--
1.8.3.1
Powered by blists - more mailing lists