Re: [PATCH] more SAK stuff
>More interestingly, it changes the operation of SAK in two ways: >(a) It does less, namely will not kill processes with uid 0. I think this is bad for security. (I assume you meant euid 0, not ruid 0. Using the real uid for access control decisions is a very odd thing to do.) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
The SUID bit (was Re: [PATCH] more SAK stuff)
On Thursday 05 July 2001 21:45, Albert D. Cahalan wrote: > Oh, cry me a river. You can set the RUID, EUID, SUID, and FUID > in that same parent process or after you fork(). Okay, I'll bite. The file user ID is fine, the effective user ID is what the suid bit sets to root of course, the saved user id is irrelevant to this (haven't encountered something that actually cares about it yet, and yes I have been checking source code when I bump into a problem). But the actual uid (real user ID) ain't root, and an euid of root doesn't let me change the uid itself to root, or at least I haven't figured out how. (And haven't really tried: there are some things that might conceivably care whether you really are root or not, but the samba change password command isn't one of them. I have a password protected cgi accessed via ssl which allows the manipulation of a limited subset of samba users, and the samba tool will happily let me change anybody's password as suid root. But to add a user, the script has to append an entry to the file manually and then change the password from "racecondition" (which it is) to whatever the user's password should be. I could patch and ship nonstandard samba binaries, but that makes automatic upgrades problematic. (And samba, being a net accessable server, REALLY needs to be kept up to date.)) Do you have a code example of how a program with euid root can change its actual uid (which several programs check when they should be checking euid, versions of dhcpcd before I complained about it case in point)? Some of it's misguided "policy", assuming that the suid bit is on the executable itself instead of its parent process. A check and an error "Thou shalt not set this suid root" is fairly common on things that can be securely run from a daemon running AS root. So apparently, the obvious way to fix it is to relax the security restrictions even MORE, which is silly. > Since you didn't set all the UID values, I have to wonder what > else you forgot to do. Maybe you shouldn't be messing with > setuid programming. Ah, the BSD attitude. If you don't already know it, you should die rather than try to learn it. Anybody who isn't perfect should leave us alone, we LIKE our user base small. :) Following this logic, nobody should use Linux because the kernel has repeatedly shipped with holes allowing people to hack root, gaping big holes like the insmod `;rm -rf /` thing last year. Apparently we should all be using an early 90's version of netware or some kind of embedded system audited for stack overflows and burned in ROM... Rob (Reference dilbert: "Here's a quarter kid, go buy yourself a real computer." That's a nice way to recruit new users to help politically support decss or convince video card manufacturers to release source code to their 3d drivers, winmodems, funky encryption in USB audio, slipping registration stuff in the ATA spec...) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
The SUID bit (was Re: [PATCH] more SAK stuff)
On Thursday 05 July 2001 21:45, Albert D. Cahalan wrote: Oh, cry me a river. You can set the RUID, EUID, SUID, and FUID in that same parent process or after you fork(). Okay, I'll bite. The file user ID is fine, the effective user ID is what the suid bit sets to root of course, the saved user id is irrelevant to this (haven't encountered something that actually cares about it yet, and yes I have been checking source code when I bump into a problem). But the actual uid (real user ID) ain't root, and an euid of root doesn't let me change the uid itself to root, or at least I haven't figured out how. (And haven't really tried: there are some things that might conceivably care whether you really are root or not, but the samba change password command isn't one of them. I have a password protected cgi accessed via ssl which allows the manipulation of a limited subset of samba users, and the samba tool will happily let me change anybody's password as suid root. But to add a user, the script has to append an entry to the file manually and then change the password from racecondition (which it is) to whatever the user's password should be. I could patch and ship nonstandard samba binaries, but that makes automatic upgrades problematic. (And samba, being a net accessable server, REALLY needs to be kept up to date.)) Do you have a code example of how a program with euid root can change its actual uid (which several programs check when they should be checking euid, versions of dhcpcd before I complained about it case in point)? Some of it's misguided policy, assuming that the suid bit is on the executable itself instead of its parent process. A check and an error Thou shalt not set this suid root is fairly common on things that can be securely run from a daemon running AS root. So apparently, the obvious way to fix it is to relax the security restrictions even MORE, which is silly. Since you didn't set all the UID values, I have to wonder what else you forgot to do. Maybe you shouldn't be messing with setuid programming. Ah, the BSD attitude. If you don't already know it, you should die rather than try to learn it. Anybody who isn't perfect should leave us alone, we LIKE our user base small. :) Following this logic, nobody should use Linux because the kernel has repeatedly shipped with holes allowing people to hack root, gaping big holes like the insmod `;rm -rf /` thing last year. Apparently we should all be using an early 90's version of netware or some kind of embedded system audited for stack overflows and burned in ROM... Rob (Reference dilbert: Here's a quarter kid, go buy yourself a real computer. That's a nice way to recruit new users to help politically support decss or convince video card manufacturers to release source code to their 3d drivers, winmodems, funky encryption in USB audio, slipping registration stuff in the ATA spec...) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
More interestingly, it changes the operation of SAK in two ways: (a) It does less, namely will not kill processes with uid 0. I think this is bad for security. (I assume you meant euid 0, not ruid 0. Using the real uid for access control decisions is a very odd thing to do.) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
Rob Landley writes: > Off the top of my head, fun things you can't do suid root: ... > ps (What the...? Worked in Red Hat 7, but not in suse 7.1. > Huh? "suid-to apache ps ax" works fine, though...) The ps command used to require setuid root. People would set the bit by habit. > I keep bumping into more of these all the time. Often it's fun > little warnings "you shouldn't have the suid bit on this > executable", which is frustrating 'cause I haven't GOT the suid bit > on that executable, it inherited it from its parent process, which > DOES explicitly set the $PATH and blank most of the environment > variables and other fun stuff...) Oh, cry me a river. You can set the RUID, EUID, SUID, and FUID in that same parent process or after you fork(). Since you didn't set all the UID values, I have to wonder what else you forgot to do. Maybe you shouldn't be messing with setuid programming. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
On Monday 02 July 2001 15:10, Hua Zhong wrote: > -> From Alan Cox <[EMAIL PROTECTED]> : > > > (a) It does less, namely will not kill processes with uid 0. > > > Ted, any objections? > > > > That breaks the security guarantee. Suppose I use a setuid app to confuse > > you into doing something ? > > a setuid app only changes euid, doesn't it? Yup. And you'd be amazed how many fun little user mode things were either never tested with the suid bit or obstinately refuse to run for no good reason. (Okay, I made something like a sudo script. It's in a directory that non-root users can't access and I'm being as careful as I know how to be, but I've got a cgi that needs root access to query/set system and network configuration.) Off the top of my head, fun things you can't do suid root: The samba adduser command. (But I CAN edit the smb.passwd file directly, which got me around this.) su without password (understandable, implementation detail. It's always suid, being run by somebody other than root is how it knows when it NEEDS to ask for a password. But when I want to DROP root privelidges... Wound up making "suid-to" to do it.) ps (What the...? Worked in Red Hat 7, but not in suse 7.1. Huh? "suid-to apache ps ax" works fine, though...) dhcpcd (I patched it and yelled at the maintainer of this months ago, should be fixed now. But a clear case of checking uid when he meant euid, which is outright PERVASIVE...). I keep bumping into more of these all the time. Often it's fun little warnings "you shouldn't have the suid bit on this executable", which is frustrating 'cause I haven't GOT the suid bit on that executable, it inherited it from its parent process, which DOES explicitly set the $PATH and blank most of the environment variables and other fun stuff...) By the way, anybody who knows why samba goes postal if you change the hostname of the box while it's running, please explain it to me. It's happy once HUPed, then again it execs itself. (Not nmbd. smbd. Why does it CARE? And sshd has the most amazing timeouts if it can't do a reverse dns lookup on the incoming IP, even if I tell it not to log!) Apache has a similar problem, and HUP-ing it interrupts in-progress transfers, which could be very large files, 'cause it execs itself. I made that happy by telling it its host name was a dot notation IP address, although that does mean that logging into a password protected web page using the host name forces you to log in twice (again when it switches you to http://1.2.3.4/blah...) Fun, isn't it? :) Alan's right. We DO need a rant tag. Rob - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
Rob Landley writes: Off the top of my head, fun things you can't do suid root: ... ps (What the...? Worked in Red Hat 7, but not in suse 7.1. Huh? suid-to apache ps ax works fine, though...) The ps command used to require setuid root. People would set the bit by habit. I keep bumping into more of these all the time. Often it's fun little warnings you shouldn't have the suid bit on this executable, which is frustrating 'cause I haven't GOT the suid bit on that executable, it inherited it from its parent process, which DOES explicitly set the $PATH and blank most of the environment variables and other fun stuff...) Oh, cry me a river. You can set the RUID, EUID, SUID, and FUID in that same parent process or after you fork(). Since you didn't set all the UID values, I have to wonder what else you forgot to do. Maybe you shouldn't be messing with setuid programming. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
On Monday 02 July 2001 15:10, Hua Zhong wrote: - From Alan Cox [EMAIL PROTECTED] : (a) It does less, namely will not kill processes with uid 0. Ted, any objections? That breaks the security guarantee. Suppose I use a setuid app to confuse you into doing something ? a setuid app only changes euid, doesn't it? Yup. And you'd be amazed how many fun little user mode things were either never tested with the suid bit or obstinately refuse to run for no good reason. (Okay, I made something like a sudo script. It's in a directory that non-root users can't access and I'm being as careful as I know how to be, but I've got a cgi that needs root access to query/set system and network configuration.) Off the top of my head, fun things you can't do suid root: The samba adduser command. (But I CAN edit the smb.passwd file directly, which got me around this.) su without password (understandable, implementation detail. It's always suid, being run by somebody other than root is how it knows when it NEEDS to ask for a password. But when I want to DROP root privelidges... Wound up making suid-to to do it.) ps (What the...? Worked in Red Hat 7, but not in suse 7.1. Huh? suid-to apache ps ax works fine, though...) dhcpcd (I patched it and yelled at the maintainer of this months ago, should be fixed now. But a clear case of checking uid when he meant euid, which is outright PERVASIVE...). I keep bumping into more of these all the time. Often it's fun little warnings you shouldn't have the suid bit on this executable, which is frustrating 'cause I haven't GOT the suid bit on that executable, it inherited it from its parent process, which DOES explicitly set the $PATH and blank most of the environment variables and other fun stuff...) By the way, anybody who knows why samba goes postal if you change the hostname of the box while it's running, please explain it to me. It's happy once HUPed, then again it execs itself. (Not nmbd. smbd. Why does it CARE? And sshd has the most amazing timeouts if it can't do a reverse dns lookup on the incoming IP, even if I tell it not to log!) Apache has a similar problem, and HUP-ing it interrupts in-progress transfers, which could be very large files, 'cause it execs itself. I made that happy by telling it its host name was a dot notation IP address, although that does mean that logging into a password protected web page using the host name forces you to log in twice (again when it switches you to http://1.2.3.4/blah...) Fun, isn't it? :) Alan's right. We DO need a rant tag. Rob - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
-> From Alan Cox <[EMAIL PROTECTED]> : > > (a) It does less, namely will not kill processes with uid 0. > > Ted, any objections? > > That breaks the security guarantee. Suppose I use a setuid app to confuse > you into doing something ? a setuid app only changes euid, doesn't it? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
On Mon, Jul 02, 2001 at 02:16:36PM +0200, [EMAIL PROTECTED] wrote: > (a) It does less, namely will not kill processes with uid 0. > Ted, any objections? What if you have a process running wild as uid 0 (i.e. X server gone bad) that you need to die *right now*? -- "Don't dwell on reality; it will only keep you from greatness." -- Randall McBride, Jr. ** Evil Genius Bryon Roche, Kain <[EMAIL PROTECTED]> PGP signature
Re: [PATCH] more SAK stuff
>> (a) It does less, namely will not kill processes with uid 0.
>> Ted, any objections?
Alan:
> That breaks the security guarantee. Suppose I use a setuid app to confuse
> you into doing something ?
On second thoughts I agree. Here is the patch without test for p->uid.
Andries
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c
./linux/drivers/char/keyboard.c
--- ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c Mon Oct 16 21:58:51 2000
+++ ./linux/drivers/char/keyboard.c Mon Jul 2 13:28:09 2001
@@ -506,6 +506,8 @@
* them properly.
*/
+ if (!tty && ttytab && ttytab[0] && ttytab[0]->driver_data)
+ tty = ttytab[0];
do_SAK(tty);
reset_vc(fg_console);
#if 0
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c
./linux/drivers/char/tty_io.c
--- ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c Sun Jul 1 15:19:26 2001
+++ ./linux/drivers/char/tty_io.c Mon Jul 2 14:53:52 2001
@@ -1818,20 +1818,29 @@
*
* Nasty bug: do_SAK is being called in interrupt context. This can
* deadlock. We punt it up to process context. AKPM - 16Mar2001
+ *
+ * Treat all VTs as a single tty for the purposes of SAK. A process with an
+ * open fd for one VT can do interesting things to all. aeb, 2001-07-02
*/
-static void __do_SAK(void *arg)
+#ifdef CONFIG_VT
+static inline int tty_is_vt(struct tty_struct *tty)
{
-#ifdef TTY_SOFT_SAK
- tty_hangup(tty);
+ return tty ? (tty->driver.type == TTY_DRIVER_TYPE_CONSOLE) : 0;
+}
#else
- struct tty_struct *tty = arg;
+static inline int tty_is_vt(struct tty_struct *tty)
+{
+ return 0;
+}
+#endif
+
+static inline void tty_hard_SAK(struct tty_struct *tty)
+{
struct task_struct *p;
int session;
- int i;
- struct file *filp;
-
- if (!tty)
- return;
+ int i;
+ struct file *filp;
+
session = tty->session;
if (tty->ldisc.flush_buffer)
tty->ldisc.flush_buffer(tty);
@@ -1839,7 +1848,9 @@
tty->driver.flush_buffer(tty);
read_lock(_lock);
for_each_task(p) {
+ /* all VTs are considered a single tty here */
if ((p->tty == tty) ||
+ (tty_is_vt(tty) && tty_is_vt(p->tty)) ||
((session > 0) && (p->session == session))) {
send_sig(SIGKILL, p, 1);
continue;
@@ -1850,7 +1861,9 @@
for (i=0; i < p->files->max_fds; i++) {
filp = fcheck_files(p->files, i);
if (filp && (filp->f_op == _fops) &&
- (filp->private_data == tty)) {
+ (filp->private_data == tty ||
+(tty_is_vt(tty) &&
+ tty_is_vt(filp->private_data {
send_sig(SIGKILL, p, 1);
break;
}
@@ -1860,6 +1873,17 @@
task_unlock(p);
}
read_unlock(_lock);
+}
+
+static void __do_SAK(void *arg)
+{
+ struct tty_struct *tty = arg;
+ if (!tty) /* impossible */
+ return;
+#ifdef TTY_SOFT_SAK
+ tty_hangup(tty);
+#else
+ tty_hard_SAK(tty);
#endif
}
@@ -1872,6 +1896,8 @@
*/
void do_SAK(struct tty_struct *tty)
{
+ if (!tty)
+ return;
PREPARE_TQUEUE(>SAK_tq, __do_SAK, tty);
schedule_task(>SAK_tq);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
>> (a) It does less, namely will not kill processes with uid 0. >> Ted, any objections? Alan: > That breaks the security guarantee. Suppose I use a setuid app to confuse > you into doing something ? You confuse me? Unlikely :-) Indeed, discussion is possible. I think my version is more secure and more useful, but if you disagree, delete the lines /* do not kill root processes */ if (p->uid == 0) continue; Andries - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
> (a) It does less, namely will not kill processes with uid 0. > Ted, any objections? That breaks the security guarantee. Suppose I use a setuid app to confuse you into doing something ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] more SAK stuff
Dear Linus, Alan, Ted, Andrew, all:
(i) Andrew - why don't you add yourself to the CREDITS file?
(then I'll find your email address at the first instead of the second attempt)
(ii) Yesterday I complained about the fact that pressing SAK twice
crashes the kernel (because the close from the first time will set
tty->driver_data = 0;
and then on the next press kbd has tty==0 and do_SAK() kills the system).
There is more bad stuff in this 2.4.3 patch:
-void do_SAK( struct tty_struct *tty)
+static void __do_SAK(void *arg)
{
#ifdef TTY_SOFT_SAK
tty_hangup(tty);
#else
+ struct tty_struct *tty = arg;
Clearly, if TTY_SOFT_SAK is defined this will not compile
(or, worse, will pick up some global variable tty).
The patch below has yesterdays fix of do_SAK(), and fixes this
compilation problem. I invented a separate inline routine here -
I do not like very long stretches of code inside #ifdef.
More interestingly, it changes the operation of SAK in two ways:
(a) It does less, namely will not kill processes with uid 0.
Ted, any objections?
For example, when syslog has several output streams, and one is
to /dev/tty10, then a SAK typed at /dev/tty10 should not kill syslog,
that is bad for security.
(b) It does more, namely will for the purposes of SAK consider all
VTs equivalent, so that a SAK typed on /dev/tty1 also kills processes
that have an open file descriptor on /dev/tty2.
That is good for security, since many keyboard or console ioctls just
require an open fd for some VT, and this process on tty2 can for example
change the keymap on tty1.
One of the motivations of this patch was that SAK should be able
to kill a "while [ 1 ]; do chvt 21; done", that is the reason
for the keyboard.c fragment.
Ted, please complain if anything is wrong with the way
filp->private_data is used.
Andries
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c
./linux/drivers/char/keyboard.c
--- ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c Mon Oct 16 21:58:51 2000
+++ ./linux/drivers/char/keyboard.c Mon Jul 2 13:28:09 2001
@@ -506,6 +506,8 @@
* them properly.
*/
+ if (!tty && ttytab && ttytab[0] && ttytab[0]->driver_data)
+ tty = ttytab[0];
do_SAK(tty);
reset_vc(fg_console);
#if 0
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c
./linux/drivers/char/tty_io.c
--- ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c Sun Jul 1 15:19:26 2001
+++ ./linux/drivers/char/tty_io.c Mon Jul 2 13:27:19 2001
@@ -1818,20 +1818,29 @@
*
* Nasty bug: do_SAK is being called in interrupt context. This can
* deadlock. We punt it up to process context. AKPM - 16Mar2001
+ *
+ * Treat all VTs as a single tty for the purposes of SAK. A process with an
+ * open fd for one VT can do interesting things to all. aeb, 2001-07-02
*/
-static void __do_SAK(void *arg)
+#ifdef CONFIG_VT
+static inline int tty_is_vt(struct tty_struct *tty)
{
-#ifdef TTY_SOFT_SAK
- tty_hangup(tty);
+ return tty ? (tty->driver.type == TTY_DRIVER_TYPE_CONSOLE) : 0;
+}
#else
- struct tty_struct *tty = arg;
+static inline int tty_is_vt(struct tty_struct *tty)
+{
+ return 0;
+}
+#endif
+
+static inline void tty_hard_SAK(struct tty_struct *tty)
+{
struct task_struct *p;
int session;
- int i;
- struct file *filp;
-
- if (!tty)
- return;
+ int i;
+ struct file *filp;
+
session = tty->session;
if (tty->ldisc.flush_buffer)
tty->ldisc.flush_buffer(tty);
@@ -1839,7 +1848,12 @@
tty->driver.flush_buffer(tty);
read_lock(_lock);
for_each_task(p) {
+ /* do not kill root processes */
+ if (p->uid == 0)
+ continue;
+ /* all VTs are considered a single tty here */
if ((p->tty == tty) ||
+ (tty_is_vt(tty) && tty_is_vt(p->tty)) ||
((session > 0) && (p->session == session))) {
send_sig(SIGKILL, p, 1);
continue;
@@ -1850,7 +1864,9 @@
for (i=0; i < p->files->max_fds; i++) {
filp = fcheck_files(p->files, i);
if (filp && (filp->f_op == _fops) &&
- (filp->private_data == tty)) {
+ (filp->private_data == tty ||
+(tty_is_vt(tty) &&
+ tty_is_vt(filp->private_data {
send_sig(SIGKILL, p, 1);
break;
}
@@ -1860,6 +1876,17 @@
task_unlock(p);
}
read_unlock(_lock);
+}
+
+static void __do_SAK(void *arg)
+{
+ struct tty_struct *tty = arg;
+
Re: [PATCH] more SAK stuff
On Mon, Jul 02, 2001 at 02:16:36PM +0200, [EMAIL PROTECTED] wrote: (a) It does less, namely will not kill processes with uid 0. Ted, any objections? What if you have a process running wild as uid 0 (i.e. X server gone bad) that you need to die *right now*? -- Don't dwell on reality; it will only keep you from greatness. -- Randall McBride, Jr. ** Evil Genius Bryon Roche, Kain [EMAIL PROTECTED] PGP signature
Re: [PATCH] more SAK stuff
- From Alan Cox [EMAIL PROTECTED] : (a) It does less, namely will not kill processes with uid 0. Ted, any objections? That breaks the security guarantee. Suppose I use a setuid app to confuse you into doing something ? a setuid app only changes euid, doesn't it? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] more SAK stuff
Dear Linus, Alan, Ted, Andrew, all:
(i) Andrew - why don't you add yourself to the CREDITS file?
(then I'll find your email address at the first instead of the second attempt)
(ii) Yesterday I complained about the fact that pressing SAK twice
crashes the kernel (because the close from the first time will set
tty-driver_data = 0;
and then on the next press kbd has tty==0 and do_SAK() kills the system).
There is more bad stuff in this 2.4.3 patch:
-void do_SAK( struct tty_struct *tty)
+static void __do_SAK(void *arg)
{
#ifdef TTY_SOFT_SAK
tty_hangup(tty);
#else
+ struct tty_struct *tty = arg;
Clearly, if TTY_SOFT_SAK is defined this will not compile
(or, worse, will pick up some global variable tty).
The patch below has yesterdays fix of do_SAK(), and fixes this
compilation problem. I invented a separate inline routine here -
I do not like very long stretches of code inside #ifdef.
More interestingly, it changes the operation of SAK in two ways:
(a) It does less, namely will not kill processes with uid 0.
Ted, any objections?
For example, when syslog has several output streams, and one is
to /dev/tty10, then a SAK typed at /dev/tty10 should not kill syslog,
that is bad for security.
(b) It does more, namely will for the purposes of SAK consider all
VTs equivalent, so that a SAK typed on /dev/tty1 also kills processes
that have an open file descriptor on /dev/tty2.
That is good for security, since many keyboard or console ioctls just
require an open fd for some VT, and this process on tty2 can for example
change the keymap on tty1.
One of the motivations of this patch was that SAK should be able
to kill a while [ 1 ]; do chvt 21; done, that is the reason
for the keyboard.c fragment.
Ted, please complain if anything is wrong with the way
filp-private_data is used.
Andries
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c
./linux/drivers/char/keyboard.c
--- ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c Mon Oct 16 21:58:51 2000
+++ ./linux/drivers/char/keyboard.c Mon Jul 2 13:28:09 2001
@@ -506,6 +506,8 @@
* them properly.
*/
+ if (!tty ttytab ttytab[0] ttytab[0]-driver_data)
+ tty = ttytab[0];
do_SAK(tty);
reset_vc(fg_console);
#if 0
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c
./linux/drivers/char/tty_io.c
--- ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c Sun Jul 1 15:19:26 2001
+++ ./linux/drivers/char/tty_io.c Mon Jul 2 13:27:19 2001
@@ -1818,20 +1818,29 @@
*
* Nasty bug: do_SAK is being called in interrupt context. This can
* deadlock. We punt it up to process context. AKPM - 16Mar2001
+ *
+ * Treat all VTs as a single tty for the purposes of SAK. A process with an
+ * open fd for one VT can do interesting things to all. aeb, 2001-07-02
*/
-static void __do_SAK(void *arg)
+#ifdef CONFIG_VT
+static inline int tty_is_vt(struct tty_struct *tty)
{
-#ifdef TTY_SOFT_SAK
- tty_hangup(tty);
+ return tty ? (tty-driver.type == TTY_DRIVER_TYPE_CONSOLE) : 0;
+}
#else
- struct tty_struct *tty = arg;
+static inline int tty_is_vt(struct tty_struct *tty)
+{
+ return 0;
+}
+#endif
+
+static inline void tty_hard_SAK(struct tty_struct *tty)
+{
struct task_struct *p;
int session;
- int i;
- struct file *filp;
-
- if (!tty)
- return;
+ int i;
+ struct file *filp;
+
session = tty-session;
if (tty-ldisc.flush_buffer)
tty-ldisc.flush_buffer(tty);
@@ -1839,7 +1848,12 @@
tty-driver.flush_buffer(tty);
read_lock(tasklist_lock);
for_each_task(p) {
+ /* do not kill root processes */
+ if (p-uid == 0)
+ continue;
+ /* all VTs are considered a single tty here */
if ((p-tty == tty) ||
+ (tty_is_vt(tty) tty_is_vt(p-tty)) ||
((session 0) (p-session == session))) {
send_sig(SIGKILL, p, 1);
continue;
@@ -1850,7 +1864,9 @@
for (i=0; i p-files-max_fds; i++) {
filp = fcheck_files(p-files, i);
if (filp (filp-f_op == tty_fops)
- (filp-private_data == tty)) {
+ (filp-private_data == tty ||
+(tty_is_vt(tty)
+ tty_is_vt(filp-private_data {
send_sig(SIGKILL, p, 1);
break;
}
@@ -1860,6 +1876,17 @@
task_unlock(p);
}
read_unlock(tasklist_lock);
+}
+
+static void __do_SAK(void *arg)
+{
+ struct tty_struct *tty = arg;
+ if (!tty)
Re: [PATCH] more SAK stuff
(a) It does less, namely will not kill processes with uid 0. Ted, any objections? That breaks the security guarantee. Suppose I use a setuid app to confuse you into doing something ? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
(a) It does less, namely will not kill processes with uid 0. Ted, any objections? Alan: That breaks the security guarantee. Suppose I use a setuid app to confuse you into doing something ? You confuse me? Unlikely :-) Indeed, discussion is possible. I think my version is more secure and more useful, but if you disagree, delete the lines /* do not kill root processes */ if (p-uid == 0) continue; Andries - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] more SAK stuff
(a) It does less, namely will not kill processes with uid 0.
Ted, any objections?
Alan:
That breaks the security guarantee. Suppose I use a setuid app to confuse
you into doing something ?
On second thoughts I agree. Here is the patch without test for p-uid.
Andries
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c
./linux/drivers/char/keyboard.c
--- ../linux-2.4.6-pre8/linux/drivers/char/keyboard.c Mon Oct 16 21:58:51 2000
+++ ./linux/drivers/char/keyboard.c Mon Jul 2 13:28:09 2001
@@ -506,6 +506,8 @@
* them properly.
*/
+ if (!tty ttytab ttytab[0] ttytab[0]-driver_data)
+ tty = ttytab[0];
do_SAK(tty);
reset_vc(fg_console);
#if 0
diff -u --recursive --new-file ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c
./linux/drivers/char/tty_io.c
--- ../linux-2.4.6-pre8/linux/drivers/char/tty_io.c Sun Jul 1 15:19:26 2001
+++ ./linux/drivers/char/tty_io.c Mon Jul 2 14:53:52 2001
@@ -1818,20 +1818,29 @@
*
* Nasty bug: do_SAK is being called in interrupt context. This can
* deadlock. We punt it up to process context. AKPM - 16Mar2001
+ *
+ * Treat all VTs as a single tty for the purposes of SAK. A process with an
+ * open fd for one VT can do interesting things to all. aeb, 2001-07-02
*/
-static void __do_SAK(void *arg)
+#ifdef CONFIG_VT
+static inline int tty_is_vt(struct tty_struct *tty)
{
-#ifdef TTY_SOFT_SAK
- tty_hangup(tty);
+ return tty ? (tty-driver.type == TTY_DRIVER_TYPE_CONSOLE) : 0;
+}
#else
- struct tty_struct *tty = arg;
+static inline int tty_is_vt(struct tty_struct *tty)
+{
+ return 0;
+}
+#endif
+
+static inline void tty_hard_SAK(struct tty_struct *tty)
+{
struct task_struct *p;
int session;
- int i;
- struct file *filp;
-
- if (!tty)
- return;
+ int i;
+ struct file *filp;
+
session = tty-session;
if (tty-ldisc.flush_buffer)
tty-ldisc.flush_buffer(tty);
@@ -1839,7 +1848,9 @@
tty-driver.flush_buffer(tty);
read_lock(tasklist_lock);
for_each_task(p) {
+ /* all VTs are considered a single tty here */
if ((p-tty == tty) ||
+ (tty_is_vt(tty) tty_is_vt(p-tty)) ||
((session 0) (p-session == session))) {
send_sig(SIGKILL, p, 1);
continue;
@@ -1850,7 +1861,9 @@
for (i=0; i p-files-max_fds; i++) {
filp = fcheck_files(p-files, i);
if (filp (filp-f_op == tty_fops)
- (filp-private_data == tty)) {
+ (filp-private_data == tty ||
+(tty_is_vt(tty)
+ tty_is_vt(filp-private_data {
send_sig(SIGKILL, p, 1);
break;
}
@@ -1860,6 +1873,17 @@
task_unlock(p);
}
read_unlock(tasklist_lock);
+}
+
+static void __do_SAK(void *arg)
+{
+ struct tty_struct *tty = arg;
+ if (!tty) /* impossible */
+ return;
+#ifdef TTY_SOFT_SAK
+ tty_hangup(tty);
+#else
+ tty_hard_SAK(tty);
#endif
}
@@ -1872,6 +1896,8 @@
*/
void do_SAK(struct tty_struct *tty)
{
+ if (!tty)
+ return;
PREPARE_TQUEUE(tty-SAK_tq, __do_SAK, tty);
schedule_task(tty-SAK_tq);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
