From 079c9534a96da9a85a2a2f9715851050fbfbf749 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Tue, 28 Feb 2012 14:49:23 +0000 Subject: vt:tackle kbd_table Keyboard struct lifetime is easy, but the locking is not and is completely ignored by the existing code. Tackle this one head on - Make the kbd_table private so we can run down all direct users - Hoick the relevant ioctl handlers into the keyboard layer - Lock them with the keyboard lock so they don't change mid keypress - Add helpers for things like console stop/start so we isolate the poking around properly - Tweak the braille console so it still builds There are a couple of FIXME locking cases left for ioctls that are so hideous they should be addressed in a later patch. After this patch the kbd_table is private and all the keyboard jiggery pokery is in one place. This update fixes speakup and also a memory leak in the original. Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/selection.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/tty/vt/selection.c') diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 7a0a12ae545..738e45a3513 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -30,6 +30,7 @@ extern void poke_blanked_console(void); +/* FIXME: all this needs locking */ /* Variables for selection control. */ /* Use a dynamic buffer, instead of static (Dec 1994) */ struct vc_data *sel_cons; /* must not be deallocated */ @@ -138,7 +139,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t char *bp, *obp; int i, ps, pe, multiplier; u16 c; - struct kbd_struct *kbd = kbd_table + fg_console; + int mode; poke_blanked_console(); @@ -182,7 +183,11 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t clear_selection(); sel_cons = vc_cons[fg_console].d; } - use_unicode = kbd && kbd->kbdmode == VC_UNICODE; + mode = vt_do_kdgkbmode(fg_console); + if (mode == K_UNICODE) + use_unicode = 1; + else + use_unicode = 0; switch (sel_mode) { -- cgit v1.2.3-70-g09d2 From 20f62579dccc84428554b914e24a312a6554f841 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 2 Mar 2012 14:59:37 +0000 Subject: vt: push down tioclinux cases Some of this ventures into selection which is still a complete lost cause. We are not making it any worse. It's completely busted anyway. Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/selection.c | 12 ++++++------ drivers/tty/vt/vt.c | 12 ++++++++++++ drivers/tty/vt/vt_ioctl.c | 2 -- 3 files changed, 18 insertions(+), 8 deletions(-) (limited to 'drivers/tty/vt/selection.c') diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 738e45a3513..2a509167092 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -75,7 +75,7 @@ clear_selection(void) { /* * User settable table: what characters are to be considered alphabetic? - * 256 bits + * 256 bits. FIXME: Needs a locking model. */ static u32 inwordLut[8]={ 0x00000000, /* control chars */ @@ -307,7 +307,8 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t * queue of the tty associated with the current console. * Invoked by ioctl(). * - * Locking: always called with BTM from vt_ioctl + * Locking: called without locks. Calls the ldisc wrongly with + * unsafe methods, */ int paste_selection(struct tty_struct *tty) { @@ -322,13 +323,12 @@ int paste_selection(struct tty_struct *tty) poke_blanked_console(); console_unlock(); + /* FIXME: wtf is this supposed to achieve ? */ ld = tty_ldisc_ref(tty); - if (!ld) { - tty_unlock(); + if (!ld) ld = tty_ldisc_ref_wait(tty); - tty_lock(); - } + /* FIXME: this is completely unsafe */ add_wait_queue(&vc->paste_wait, &wait); while (sel_buffer && sel_buffer_lth > pasted) { set_current_state(TASK_INTERRUPTIBLE); diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index ce2b8234362..ab7385e526a 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -2637,11 +2637,15 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) ret = __put_user(data, p); break; case TIOCL_GETMOUSEREPORTING: + console_lock(); /* May be overkill */ data = mouse_reporting(); + console_unlock(); ret = __put_user(data, p); break; case TIOCL_SETVESABLANK: + console_lock(); ret = set_vesa_blanking(p); + console_unlock(); break; case TIOCL_GETKMSGREDIRECT: data = vt_get_kmsg_redirect(); @@ -2658,13 +2662,21 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) } break; case TIOCL_GETFGCONSOLE: + /* No locking needed as this is a transiently + correct return anyway if the caller hasn't + disabled switching */ ret = fg_console; break; case TIOCL_SCROLLCONSOLE: if (get_user(lines, (s32 __user *)(p+4))) { ret = -EFAULT; } else { + /* Need the console lock here. Note that lots + of other calls need fixing before the lock + is actually useful ! */ + console_lock(); scrollfront(vc_cons[fg_console].d, lines); + console_unlock(); ret = 0; } break; diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c index e05094d7634..c6720be8d21 100644 --- a/drivers/tty/vt/vt_ioctl.c +++ b/drivers/tty/vt/vt_ioctl.c @@ -298,9 +298,7 @@ int vt_ioctl(struct tty_struct *tty, switch (cmd) { case TIOCLINUX: - tty_lock(); ret = tioclinux(tty, arg); - tty_unlock(); break; case KIOCSOUND: if (!perm) -- cgit v1.2.3-70-g09d2 From 5289475d1375017ab4288b276383e9684280876d Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 2 Mar 2012 15:00:02 +0000 Subject: vt: tackle the main part of the selection logic We leave the existing paste mess alone and just fix up the vt side of things. Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/selection.c | 39 +++++++++++++++++++++++++++++++-------- drivers/tty/vt/vt.c | 2 ++ 2 files changed, 33 insertions(+), 8 deletions(-) (limited to 'drivers/tty/vt/selection.c') diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 2a509167092..8e9b4be97a2 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -62,10 +62,14 @@ sel_pos(int n) use_unicode); } -/* remove the current selection highlight, if any, - from the console holding the selection. */ -void -clear_selection(void) { +/** + * clear_selection - remove current selection + * + * Remove the current selection highlight, if any from the console + * holding the selection. The caller must hold the console lock. + */ +void clear_selection(void) +{ highlight_pointer(-1); /* hide the pointer */ if (sel_start != -1) { highlight(sel_start, sel_end); @@ -75,7 +79,7 @@ clear_selection(void) { /* * User settable table: what characters are to be considered alphabetic? - * 256 bits. FIXME: Needs a locking model. + * 256 bits. Locked by the console lock. */ static u32 inwordLut[8]={ 0x00000000, /* control chars */ @@ -92,10 +96,20 @@ static inline int inword(const u16 c) { return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1); } -/* set inwordLut contents. Invoked by ioctl(). */ +/** + * set loadlut - load the LUT table + * @p: user table + * + * Load the LUT table from user space. The caller must hold the console + * lock. Make a temporary copy so a partial update doesn't make a mess. + */ int sel_loadlut(char __user *p) { - return copy_from_user(inwordLut, (u32 __user *)(p+4), 32) ? -EFAULT : 0; + u32 tmplut[8]; + if (copy_from_user(tmplut, (u32 __user *)(p+4), 32)) + return -EFAULT; + memcpy(inwordLut, tmplut, 32); + return 0; } /* does screen address p correspond to character at LH/RH edge of screen? */ @@ -131,7 +145,16 @@ static int store_utf8(u16 c, char *p) } } -/* set the current selection. Invoked by ioctl() or by kernel code. */ +/** + * set_selection - set the current selection. + * @sel: user selection info + * @tty: the console tty + * + * Invoked by the ioctl handle for the vt layer. + * + * The entire selection process is managed under the console_lock. It's + * a lot under the lock but its hardly a performance path + */ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *tty) { struct vc_data *vc = vc_cons[fg_console].d; diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index ab7385e526a..e5abceacc2d 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -2623,7 +2623,9 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) console_unlock(); break; case TIOCL_SELLOADLUT: + console_lock(); ret = sel_loadlut(p); + console_unlock(); break; case TIOCL_GETSHIFTSTATE: -- cgit v1.2.3-70-g09d2