On Fri, Sep 30, 2005 at 12:27:04PM -0700, Linus Torvalds wrote: > > > On Fri, 30 Sep 2005, Chris Wright wrote: > > > > Sorry, I missed the thread up to this, but this looks fundamentally > > broken. The kill_proc_info_as_uid() idea is not sufficient because more > > than uid/euid are needed for permission check. There's capabilities and > > security labels. > > Not for this particular USB use, there isn't. Since you can only send a > signal to yourself anyway, the uid/euid check is just testing that you're > still who you were.
please find the patch below. It compiles, but I didn't yet have the
time to verify it makes the bug disappear and the async urb delivery is
still working.
I'll try to report whether it's working tomorrow.
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -30,6 +30,8 @@
* Revision history
* 22.12.1999 0.1 Initial release (split from proc_usb.c)
* 04.01.2000 0.2 Turned into its own filesystem
+ * 30.09.2005 0.3 Fix user-triggerable oops in async URB delivery
+ * (CAN-2005-3055)
*/
/*****************************************************************************/
@@ -58,7 +60,8 @@ static struct class *usb_device_class;
struct async {
struct list_head asynclist;
struct dev_state *ps;
- struct task_struct *task;
+ pid_t pid;
+ pid_t uid, euid;
unsigned int signr;
unsigned int ifnum;
void __user *userbuffer;
@@ -290,7 +293,8 @@ static void async_completed(struct urb *
sinfo.si_errno = as->urb->status;
sinfo.si_code = SI_ASYNCIO;
sinfo.si_addr = as->userurb;
- send_sig_info(as->signr, &sinfo, as->task);
+ kill_proc_info_as_uid(as->signr, &sinfo, as->pid, as->uid,
+ as->euid);
}
wake_up(&ps->wait);
}
@@ -988,7 +992,9 @@ static int proc_do_submiturb(struct dev_
as->userbuffer = NULL;
as->signr = uurb->signr;
as->ifnum = ifnum;
- as->task = current;
+ as->pid = current->pid;
+ as->uid = current->uid;
+ as->euid = current->euid;
if (!(uurb->endpoint & USB_DIR_IN)) {
if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
as->urb->transfer_buffer_length)) {
free_async(as);
diff --git a/include/linux/sched.h b/include/linux/sched.h
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1018,6 +1018,7 @@ extern int force_sig_info(int, struct si
extern int __kill_pg_info(int sig, struct siginfo *info, pid_t pgrp);
extern int kill_pg_info(int, struct siginfo *, pid_t);
extern int kill_proc_info(int, struct siginfo *, pid_t);
+extern int kill_proc_info_as_uid(int, struct siginfo *, pid_t, pid_t, pid_t);
extern void do_notify_parent(struct task_struct *, int);
extern void force_sig(int, struct task_struct *);
extern void force_sig_specific(int, struct task_struct *);
diff --git a/kernel/signal.c b/kernel/signal.c
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -655,7 +655,8 @@ static int rm_from_queue(unsigned long m
* Bad permissions for sending the signal
*/
static int check_kill_permission(int sig, struct siginfo *info,
- struct task_struct *t)
+ struct task_struct *t,
+ pid_t uid, pid_t euid)
{
int error = -EINVAL;
if (!valid_signal(sig))
@@ -665,8 +666,8 @@ static int check_kill_permission(int sig
(unsigned long)info != 2 && SI_FROMUSER(info)))
&& ((sig != SIGCONT) ||
(current->signal->session != t->signal->session))
- && (current->euid ^ t->suid) && (current->euid ^ t->uid)
- && (current->uid ^ t->suid) && (current->uid ^ t->uid)
+ && (euid ^ t->suid) && (euid ^ t->uid)
+ && (uid ^ t->suid) && (uid ^ t->uid)
&& !capable(CAP_KILL))
return error;
@@ -1132,7 +1133,24 @@ int group_send_sig_info(int sig, struct
unsigned long flags;
int ret;
- ret = check_kill_permission(sig, info, p);
+ ret = check_kill_permission(sig, info, p, current->uid, current->euid);
+ if (!ret && sig && p->sighand) {
+ spin_lock_irqsave(&p->sighand->siglock, flags);
+ ret = __group_send_sig_info(sig, info, p);
+ spin_unlock_irqrestore(&p->sighand->siglock, flags);
+ }
+
+ return ret;
+}
+
+static int group_send_sig_info_as_uid(int sig, struct siginfo *info,
+ struct task_struct *p,
+ pid_t uid, pid_t euid)
+{
+ unsigned long flags;
+ int ret;
+
+ ret = check_kill_permission(sig, info, p, current->uid, current->euid);
if (!ret && sig && p->sighand) {
spin_lock_irqsave(&p->sighand->siglock, flags);
ret = __group_send_sig_info(sig, info, p);
@@ -1192,6 +1210,22 @@ kill_proc_info(int sig, struct siginfo *
return error;
}
+/* like kill_proc_info(), but doesn't use uid/euid of "current" */
+int
+kill_proc_info_as_uid(int sig, struct siginfo *info, pid_t pid,
+ pid_t uid, pid_t euid)
+{
+ int error;
+ struct task_struct *p;
+
+ read_lock(&tasklist_lock);
+ p = find_task_by_pid(pid);
+ error = -ESRCH;
+ if (p)
+ error = group_send_sig_info_as_uid(sig, info, p, uid, euid);
+ read_unlock(&tasklist_lock);
+ return error;
+}
/*
* kill_something_info() interprets pid in interesting ways just like kill(2).
@@ -2290,7 +2324,8 @@ asmlinkage long sys_tgkill(int tgid, int
p = find_task_by_pid(pid);
error = -ESRCH;
if (p && (p->tgid == tgid)) {
- error = check_kill_permission(sig, &info, p);
+ error = check_kill_permission(sig, &info, p, current->uid,
+ current->euid);
/*
* The null signal is a permissions and process existence
* probe. No signal is actually delivered.
@@ -2330,7 +2365,8 @@ sys_tkill(int pid, int sig)
p = find_task_by_pid(pid);
error = -ESRCH;
if (p) {
- error = check_kill_permission(sig, &info, p);
+ error = check_kill_permission(sig, &info, p, current->uid,
+ current->euid);
/*
* The null signal is a permissions and process existence
* probe. No signal is actually delivered.
--
- Harald Welte <[EMAIL PROTECTED]> http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
pgpzqVID5nYav.pgp
Description: PGP signature
