On Thu, Oct 13, 2005 at 12:50:03AM +0200, Moritz Muehlenhoff wrote:
> Hi,
> on linux-kernel there were some iterations of patch review before an
> acceptable patch could be cooked up. Here's the final one from Harald
> Welte. Meanwhile Linus already commited an older version version of
> his patch, for which Harald wanted to push an additional patch, so
> I guess it's the best to directly include this fix.

Sorry this is taking a while to get in.
I had do do a bit of backporting (attached for reference).
And I am begining to suspect that it is an ABI breaker.

Ohh, and I think I separately broke the 2.6.8 build, so I am looking
into that.


-- 
Horms
--- from-0001/drivers/usb/core/devio.c
+++ to-0003/drivers/usb/core/devio.c    2005-10-14 16:50:10.000000000 +0900
@@ -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)
  */
 
 /*****************************************************************************/
@@ -53,7 +55,8 @@
 struct async {
        struct list_head asynclist;
        struct dev_state *ps;
-       struct task_struct *task;
+       pid_t pid;
+       uid_t uid, euid;
        unsigned int signr;
        unsigned int ifnum;
        void __user *userbuffer;
@@ -270,7 +273,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);
 }
@@ -495,9 +499,11 @@ static int usbdev_open(struct inode *ino
        INIT_LIST_HEAD(&ps->async_pending);
        INIT_LIST_HEAD(&ps->async_completed);
        init_waitqueue_head(&ps->wait);
-       ps->discsignr = 0;
-       ps->disctask = current;
-       ps->disccontext = NULL;
+       ps->disc.signr = 0;
+       ps->disc.pid = current->pid;
+       ps->disc.uid = current->uid;
+       ps->disc.euid = current->euid;
+       ps->disc.context = NULL;
        ps->ifclaimed = 0;
        wmb();
        list_add_tail(&ps->list, &dev->filelist);
@@ -940,7 +946,9 @@ static int proc_submiturb(struct dev_sta
                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);
@@ -1059,8 +1067,8 @@ static int proc_disconnectsignal(struct 
                return -EFAULT;
        if (ds.signr != 0 && (ds.signr < SIGRTMIN || ds.signr > SIGRTMAX))
                return -EINVAL;
-       ps->discsignr = ds.signr;
-       ps->disccontext = ds.context;
+       ps->disc.signr = ds.signr;
+       ps->disc.context = ds.context;
        return 0;
 }
 
--- from-0001/drivers/usb/core/inode.c
+++ to-0003/drivers/usb/core/inode.c    2005-10-14 16:46:34.000000000 +0900
@@ -815,12 +815,14 @@ void usbfs_remove_device(struct usb_devi
        while (!list_empty(&dev->filelist)) {
                ds = list_entry(dev->filelist.next, struct dev_state, list);
                list_del_init(&ds->list);
-               if (ds->discsignr) {
+               if (ds->disc.signr) {
                        sinfo.si_signo = SIGPIPE;
                        sinfo.si_errno = EPIPE;
                        sinfo.si_code = SI_ASYNCIO;
-                       sinfo.si_addr = ds->disccontext;
-                       send_sig_info(ds->discsignr, &sinfo, ds->disctask);
+                       sinfo.si_addr = ds->disc.context;
+                       kill_proc_info_as_uid(ds->disc.signr, &sinfo,
+                                             ds->disc.pid, ds->disc.uid,
+                                             ds->disc.euid);
                }
        }
        usbfs_update_special();
--- from-0001/include/linux/sched.h
+++ to-0003/include/linux/sched.h       2005-10-14 16:58:21.000000000 +0900
@@ -794,6 +794,7 @@ extern int __kill_pg_info(int sig, struc
 extern int kill_pg_info(int, struct siginfo *, pid_t);
 extern int kill_sl_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, uid_t, uid_t);
 extern void notify_parent(struct task_struct *, int);
 extern void do_notify_parent(struct task_struct *, int);
 extern void force_sig(int, struct task_struct *);
--- from-0001/include/linux/usbdevice_fs.h
+++ to-0003/include/linux/usbdevice_fs.h        2005-10-14 16:53:51.000000000 
+0900
@@ -160,9 +160,12 @@ struct dev_state {
        struct list_head async_pending;
        struct list_head async_completed;
        wait_queue_head_t wait;     /* wake up if a request completed */
-       unsigned int discsignr;
-       struct task_struct *disctask;
-       void __user *disccontext;
+       struct {
+               unsigned int signr;
+               pid_t pid;
+               uid_t uid, euid;
+               void __user *context;
+       } disc;
        unsigned long ifclaimed;
 };
 
--- from-0001/kernel/signal.c
+++ to-0003/kernel/signal.c     2005-10-14 16:57:03.000000000 +0900
@@ -1154,6 +1154,42 @@ kill_proc_info(int sig, struct siginfo *
        return error;
 }
 
+/* This is like kill_proc_info(), but doesn't use uid/euid of "current".
+ * Most likely this function is not what you want, it was added as a special
+ * case for usbdevio */
+int
+kill_proc_info_as_uid(int sig, struct siginfo *info, pid_t pid,
+                     uid_t uid, uid_t euid)
+{
+       int ret = -EINVAL;
+       struct task_struct *p;
+
+       if (!valid_signal(sig))
+               return ret;
+
+       read_lock(&tasklist_lock);
+       p = find_task_by_pid(pid);
+       if (!p) {
+               ret = -ESRCH;
+               goto out_unlock;
+       }
+       if ((!info || ((unsigned long)info != 1 &&
+                       (unsigned long)info != 2 && SI_FROMUSER(info)))
+           && (euid ^ p->suid) && (euid ^ p->uid)
+           && (uid ^ p->suid) && (uid ^ p->uid)) {
+               ret = -EPERM;
+               goto out_unlock;
+       }
+       if (sig && p->sighand) {
+               unsigned long flags;
+               spin_lock_irqsave(&p->sighand->siglock, flags);
+               ret = __group_send_sig_info(sig, info, p);
+               spin_unlock_irqrestore(&p->sighand->siglock, flags);
+       }
+out_unlock:
+       read_unlock(&tasklist_lock);
+       return ret;
+}
 
 /*
  * kill_something_info() interprets pid in interesting ways just like kill(2).
@@ -1892,6 +1928,7 @@ EXPORT_SYMBOL(kill_pg);
 EXPORT_SYMBOL(kill_pg_info);
 EXPORT_SYMBOL(kill_proc);
 EXPORT_SYMBOL(kill_proc_info);
+EXPORT_SYMBOL_GPL(kill_proc_info_as_uid);
 EXPORT_SYMBOL(kill_sl);
 EXPORT_SYMBOL(kill_sl_info);
 EXPORT_SYMBOL(notify_parent);

Reply via email to