Messages in this thread Patch in this message | | | Date | Sun, 19 Apr 1998 20:31:46 +0200 | From | Pavel Machek <> | Subject | Re: VT switching (Was: SECURITY: Kill process when on console) |
| |
Hi!
> > Well, I'll happy to fix kernel side. X side is problem for me. > > At the end of the X-server initialisation routine, the X-server has: > a) chosen and opened a free VT or > opened the VT specified on the command line, > b) put the VT into VT_PROCESS mode, > c) arranged for a signal hadler to be called on VT changes, and > d) activated the VT. > > Judging by the current code in the X-server, a user-level program to > do just this ought to be about 50 lines of C, including error > handling. If your interface allow this to be done safely, I will bolt > it into the X-server and send a patch to the XFree86 maintainers.
Ook, I have some experimental code for you. It's kernel patch. (Sorry, I'm not pushing it into kernel until it proves usable).
It is not-much-tested.
Pavel
--- clean/include/linux/kd.h Sun Dec 14 23:34:58 1997 +++ linux/include/linux/kd.h Sun Apr 12 22:43:18 1998 @@ -2,7 +2,11 @@ #define _LINUX_KD_H #include <linux/types.h> -/* 0x4B is 'K', to avoid collision with termios and vt */ +/* + * 0x4B is 'K', to avoid collision with termios and vt + * + * FIXME: Should use _IO() macros. + */ #define GIO_FONT 0x4B60 /* gets font in expanded form */ #define PIO_FONT 0x4B61 /* use font in expanded form */ @@ -40,6 +44,7 @@ #define KDDISABIO 0x4B37 /* disable i/o to video board */ #define KDSETMODE 0x4B3A /* set text/graphics mode */ +#define KDSETMODEX 0x4B72 /* set text/graphics mode of any console */ #define KD_TEXT 0x00 #define KD_GRAPHICS 0x01 #define KD_TEXT0 0x02 /* obsolete */ @@ -134,6 +139,6 @@ /* note: 0x4B00-0x4B4E all have had a value at some time; don't reuse for the time being */ -/* note: 0x4B60-0x4B6D, 0x4B70, 0x4B71 used above */ +/* note: 0x4B60-0x4B6D, 0x4B70-0x4B72 used above */ #endif /* _LINUX_KD_H */ --- clean/include/linux/vt.h Sun Mar 24 11:09:37 1996 +++ linux/include/linux/vt.h Sun Apr 12 22:43:42 1998 @@ -1,7 +1,11 @@ #ifndef _LINUX_VT_H #define _LINUX_VT_H -/* 0x56 is 'V', to avoid collision with termios and kd */ +/* + * 0x56 is 'V', to avoid collision with termios and kd + * + * FIXME: should use _IO() macros. + */ #define VT_OPENQRY 0x5600 /* find available vt */ @@ -50,5 +54,6 @@ #define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */ #define VT_LOCKSWITCH 0x560B /* disallow vt switching */ #define VT_UNLOCKSWITCH 0x560C /* allow vt switching */ +#define VT_OPENALLOC 0x560D /* find available vt and alloc it */ #endif /* _LINUX_VT_H */ --- clean/drivers/char/vt.c Mon Dec 1 22:36:05 1997 +++ linux/drivers/char/vt.c Sun Apr 12 22:37:20 1998 @@ -1,12 +1,15 @@ + /* * linux/drivers/char/vt.c * * Copyright (C) 1992 obz under the linux copyright + * [Who is obz? Can't find references, anywhere...] * * Dynamic diacritical handling - aeb@cwi.nl - Dec 1993 * Dynamic keymap and string allocation - aeb@cwi.nl - May 1994 * Restrict VT switching via ioctl() - grif@cs.ucr.edu - Dec 1995 * Some code moved for less code duplication - Andi Kleen - Mar 1997 + * Attempt to fix races in userland code - pavel@ucw.cz - Apr 1998 */ #include <linux/config.h> @@ -541,7 +537,19 @@ (cmd == KDENABIO)) ? -ENXIO : 0; #endif + case KDSETMODEX: + if (!suser()) + return -EPERM; + console = arg >> 16; + arg &= 0xffff; + + if (!vc_cons_allocated(console)) /* well possible */ + return -ENOIOCTLCMD; + case KDSETMODE: + /* FIXME: how is userland expected to use this? This + * means that console has to be current in order to + * set its parameters. Lose, lose. <pavel@ucw.cz> */ /* * currently, setting the mode from KD_TEXT to KD_GRAPHICS * doesn't do a whole lot. i'm not sure if it should do any @@ -591,6 +599,9 @@ return -EPERM; switch(arg) { case K_RAW: + /* We do not want userland code to know + about hardware details, do we? */ + printk( KERN_WARNING "Warning: Deprecated keyboard RAW mode set by %s (%d)\n", current->comm, current->pid ); kbd->kbdmode = VC_RAW; break; case K_MEDIUMRAW: @@ -598,7 +609,7 @@ break; case K_XLATE: kbd->kbdmode = VC_XLATE; - compute_shiftstate(); + compute_shiftstate(); /* should be no longer neccessary */ break; case K_UNICODE: kbd->kbdmode = VC_UNICODE; @@ -746,6 +759,7 @@ /* the frsig is ignored, so we set it to 0 */ vt_cons[console]->vt_mode.frsig = 0; vt_cons[console]->vt_pid = current->pid; + vt_cons[console]->vt_uid = current->euid; /* no switch is required -- saw@shade.msu.ru */ vt_cons[console]->vt_newvt = -1; return 0; @@ -778,6 +792,10 @@ /* * Returns the first available (non-opened) console. + * + * Hmm. And how do you expect userland to use this syscall? + * There's race here by definition: two processes do OPENQRY, + * both get same terminal, both try to use it. <pavel@ucw.cz> */ case VT_OPENQRY: for (i = 0; i < MAX_NR_CONSOLES; ++i) @@ -787,6 +805,19 @@ goto setint; /* + * This should obsolete VT_OPENQRY: as it allocates console, it + * avoids race. <pavel@ucw.cz> + */ + case VT_OPENALLOC: + for (i = 0; i < MAX_NR_CONSOLES; ++i) + if (! VT_IS_IN_USE(i)) + break; + ucval = i < MAX_NR_CONSOLES ? (i+1) : -1; + if (ucval != -1) + ucval = vc_allocate(ucval) ? -1 : ucval; + goto setint; + + /* * ioctl(fd, VT_ACTIVATE, num) will cause us to switch to vt # num, * with num >= 1 (switches to vt 0, our console, are not allowed, just * to preserve sanity). @@ -1119,7 +1150,7 @@ */ void complete_change_console(unsigned int new_console) { - unsigned char old_vc_mode; + unsigned char old_vc_mode, bad = 0; if ((new_console == fg_console) || (vt_dont_switch)) return; @@ -1142,12 +1173,28 @@ */ if (vt_cons[new_console]->vt_mode.mode == VT_PROCESS) { + { + /* Security: Code below allowed user on console to + * send arbitrary signal after PID wrap-around. This + * is probably not really safe on SMP, but I do not + * think that PID's are going to wrap-around in that + * small window. (If they do, we have *much* worse + * problems, anyway.) <pavel@ucw.cz> + */ + + struct task_struct *p; + + read_lock(&tasklist_lock); + p = find_task_by_pid(vt_cons[new_console]->vt_pid); + bad = !p || (p->euid != vt_cons[new_console]->vt_uid); + read_unlock(&tasklist_lock); + } /* * Send the signal as privileged - kill_proc() will * tell us if the process has gone or something else - * is awry - */ - if (kill_proc(vt_cons[new_console]->vt_pid, + * is awry */ + if (!bad && + kill_proc(vt_cons[new_console]->vt_pid, vt_cons[new_console]->vt_mode.acqsig, 1) != 0) {
-- I'm really pavel@atrey.karlin.mff.cuni.cz. Pavel Look at http://atrey.karlin.mff.cuni.cz/~pavel/ ;-).
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu
| |