Skip to content

Commit

Permalink
vt: push the tty_lock down into the map handling
Browse files Browse the repository at this point in the history 
When we do this it becomes clear the lock we should be holding is the vc
lock, and in fact many of our other helpers are properly invoked this way.

We don't at this point guarantee not to race the keyboard code but the results
of that appear harmless and that was true before we started as well.

We now have no users of tty_lock in the console driver...

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Alan Cox authored and Greg Kroah-Hartman committed Apr 24, 2012
1 parent 10af77c commit 5d1a33f
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 53 deletions.
123 changes: 91 additions & 32 deletions drivers/tty/vt/consolemap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/tty.h>
#include <asm/uaccess.h>
#include <linux/console.h>
#include <linux/consolemap.h>
#include <linux/vt_kern.h>

Expand Down Expand Up @@ -312,13 +313,15 @@ int con_set_trans_old(unsigned char __user * arg)
if (!access_ok(VERIFY_READ, arg, E_TABSZ))
return -EFAULT;

console_lock();
for (i=0; i<E_TABSZ ; i++) {
unsigned char uc;
__get_user(uc, arg+i);
p[i] = UNI_DIRECT_BASE | uc;
}

update_user_maps();
console_unlock();
return 0;
}

Expand All @@ -330,11 +333,13 @@ int con_get_trans_old(unsigned char __user * arg)
if (!access_ok(VERIFY_WRITE, arg, E_TABSZ))
return -EFAULT;

console_lock();
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);
}
{
ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
__put_user((ch & ~0xff) ? 0 : ch, arg+i);
}
console_unlock();
return 0;
}

Expand All @@ -346,13 +351,15 @@ int con_set_trans_new(ushort __user * arg)
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;
}

update_user_maps();
console_unlock();
return 0;
}

Expand All @@ -364,8 +371,10 @@ int con_get_trans_new(ushort __user * arg)
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);
console_unlock();

return 0;
}
Expand Down Expand Up @@ -407,6 +416,7 @@ static void con_release_unimap(struct uni_pagedir *p)
}
}

/* Caller must hold the console lock */
void con_free_unimap(struct vc_data *vc)
{
struct uni_pagedir *p;
Expand Down Expand Up @@ -487,17 +497,21 @@ con_insert_unipair(struct uni_pagedir *p, u_short unicode, u_short fontpos)
return 0;
}

/* ui is a leftover from using a hashtable, but might be used again */
int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
/* ui is a leftover from using a hashtable, but might be used again
Caller must hold the lock */
static int con_do_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
{
struct uni_pagedir *p, *q;

p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
if (p && p->readonly) return -EIO;
if (p && p->readonly)
return -EIO;

if (!p || --p->refcount) {
q = kzalloc(sizeof(*p), GFP_KERNEL);
if (!q) {
if (p) p->refcount++;
if (p)
p->refcount++;
return -ENOMEM;
}
q->refcount=1;
Expand All @@ -511,23 +525,43 @@ int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
return 0;
}

int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui)
{
int ret;
console_lock();
ret = con_do_clear_unimap(vc, ui);
console_unlock();
return ret;
}

int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
{
int err = 0, err1, i;
struct uni_pagedir *p, *q;

console_lock();

/* Save original vc_unipagdir_loc in case we allocate a new one */
p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
if (p->readonly) return -EIO;
if (p->readonly) {
console_unlock();
return -EIO;
}

if (!ct) return 0;
if (!ct) {
console_unlock();
return 0;
}

if (p->refcount > 1) {
int j, k;
u16 **p1, *p2, l;

err1 = con_clear_unimap(vc, NULL);
if (err1) return err1;
err1 = con_do_clear_unimap(vc, NULL);
if (err1) {
console_unlock();
return err1;
}

/*
* Since refcount was > 1, con_clear_unimap() allocated a
Expand Down Expand Up @@ -558,7 +592,8 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
*vc->vc_uni_pagedir_loc = (unsigned long)p;
con_release_unimap(q);
kfree(q);
return err1;
console_unlock();
return err1;
}
}
} else {
Expand Down Expand Up @@ -592,21 +627,30 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
/*
* Merge with fontmaps of any other virtual consoles.
*/
if (con_unify_unimap(vc, p))
if (con_unify_unimap(vc, p)) {
console_unlock();
return err;
}

for (i = 0; i <= 3; i++)
set_inverse_transl(vc, p, i); /* Update inverse translations */
set_inverse_trans_unicode(vc, p);


console_unlock();
return err;
}

/* Loads the unimap for the hardware font, as defined in uni_hash.tbl.
The representation used was the most compact I could come up
with. This routine is executed at sys_setup time, and when the
PIO_FONTRESET ioctl is called. */

/**
* con_set_default_unimap - set default unicode map
* @vc: the console we are updating
*
* Loads the unimap for the hardware font, as defined in uni_hash.tbl.
* The representation used was the most compact I could come up
* with. This routine is executed at video setup, and when the
* PIO_FONTRESET ioctl is called.
*
* The caller must hold the console lock
*/
int con_set_default_unimap(struct vc_data *vc)
{
int i, j, err = 0, err1;
Expand All @@ -617,6 +661,7 @@ int con_set_default_unimap(struct vc_data *vc)
p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
if (p == dflt)
return 0;

dflt->refcount++;
*vc->vc_uni_pagedir_loc = (unsigned long)dflt;
if (p && !--p->refcount) {
Expand All @@ -628,8 +673,9 @@ int con_set_default_unimap(struct vc_data *vc)

/* The default font is always 256 characters */

err = con_clear_unimap(vc, NULL);
if (err) return err;
err = con_do_clear_unimap(vc, NULL);
if (err)
return err;

p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
q = dfont_unitable;
Expand All @@ -654,6 +700,13 @@ int con_set_default_unimap(struct vc_data *vc)
}
EXPORT_SYMBOL(con_set_default_unimap);

/**
* con_copy_unimap - copy unimap between two vts
* @dst_vc: target
* @src_vt: source
*
* The caller must hold the console lock when invoking this method
*/
int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
{
struct uni_pagedir *q;
Expand All @@ -668,13 +721,23 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc)
*dst_vc->vc_uni_pagedir_loc = (long)q;
return 0;
}
EXPORT_SYMBOL(con_copy_unimap);

/**
* con_get_unimap - get the unicode map
* @vc: the console to read from
*
* Read the console unicode data for this console. Called from the ioctl
* handlers.
*/
int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list)
{
int i, j, k, ect;
u16 **p1, *p2;
struct uni_pagedir *p;

console_lock();

ect = 0;
if (*vc->vc_uni_pagedir_loc) {
p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;
Expand All @@ -694,22 +757,19 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni
}
}
__put_user(ect, uct);
console_unlock();
return ((ect <= ct) ? 0 : -ENOMEM);
}

void con_protect_unimap(struct vc_data *vc, int rdonly)
{
struct uni_pagedir *p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc;

if (p)
p->readonly = rdonly;
}

/*
* Always use USER_MAP. These functions are used by the keyboard,
* which shouldn't be affected by G0/G1 switching, etc.
* If the user map still contains default values, i.e. the
* direct-to-font mapping, then assume user is using Latin1.
*
* FIXME: at some point we need to decide if we want to lock the table
* update element itself via the keyboard_event_lock for consistency with the
* keyboard driver as well as the consoles
*/
/* may be called during an interrupt */
u32 conv_8bit_to_uni(unsigned char c)
Expand Down Expand Up @@ -777,4 +837,3 @@ console_map_init(void)
con_set_default_unimap(vc_cons[i].d);
}

EXPORT_SYMBOL(con_copy_unimap);
25 changes: 5 additions & 20 deletions drivers/tty/vt/vt_ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,9 @@ int vt_ioctl(struct tty_struct *tty,
ret = con_font_op(vc_cons[fg_console].d, &op);
if (ret)
break;
console_lock();
con_set_default_unimap(vc_cons[fg_console].d);
console_unlock();
break;
}
#endif
Expand All @@ -934,33 +936,23 @@ int vt_ioctl(struct tty_struct *tty,
case PIO_SCRNMAP:
if (!perm)
ret = -EPERM;
else {
tty_lock();
else
ret = con_set_trans_old(up);
tty_unlock();
}
break;

case GIO_SCRNMAP:
tty_lock();
ret = con_get_trans_old(up);
tty_unlock();
break;

case PIO_UNISCRNMAP:
if (!perm)
ret = -EPERM;
else {
tty_lock();
else
ret = con_set_trans_new(up);
tty_unlock();
}
break;

case GIO_UNISCRNMAP:
tty_lock();
ret = con_get_trans_new(up);
tty_unlock();
break;

case PIO_UNIMAPCLR:
Expand All @@ -970,19 +962,14 @@ int vt_ioctl(struct tty_struct *tty,
ret = copy_from_user(&ui, up, sizeof(struct unimapinit));
if (ret)
ret = -EFAULT;
else {
tty_lock();
else
con_clear_unimap(vc, &ui);
tty_unlock();
}
break;
}

case PIO_UNIMAP:
case GIO_UNIMAP:
tty_lock();
ret = do_unimap_ioctl(cmd, up, perm, vc);
tty_unlock();
break;

case VT_LOCKSWITCH:
Expand Down Expand Up @@ -1196,9 +1183,7 @@ long vt_compat_ioctl(struct tty_struct *tty,

case PIO_UNIMAP:
case GIO_UNIMAP:
tty_lock();
ret = compat_unimap_ioctl(cmd, up, perm, vc);
tty_unlock();
break;

/*
Expand Down
1 change: 0 additions & 1 deletion include/linux/vt_kern.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list);
int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list);
int con_set_default_unimap(struct vc_data *vc);
void con_free_unimap(struct vc_data *vc);
void con_protect_unimap(struct vc_data *vc, int rdonly);
int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc);

#define vc_translate(vc, c) ((vc)->vc_translate[(c) | \
Expand Down

0 comments on commit 5d1a33f

Please sign in to comment.