Author: royger
Date: Thu May 24 10:18:31 2018
New Revision: 334144
URL: https://svnweb.freebsd.org/changeset/base/334144

Log:
  dev/xenstore: prevent transaction hijacking
  
  The user-space xenstore device is currently lacking a check to make
  sure that the caller is only using transaction ids currently assigned
  to it. This allows users of the xenstore device to hijack transactions
  not started by them, although the scope is limited to transactions
  started by the same domain.
  
  Tested by:      Nathan Friess <nathan.fri...@gmail.com>
  Sponsored by:   Citrix Systems R&D

Modified:
  head/sys/dev/xen/xenstore/xenstore_dev.c

Modified: head/sys/dev/xen/xenstore/xenstore_dev.c
==============================================================================
--- head/sys/dev/xen/xenstore/xenstore_dev.c    Thu May 24 10:18:14 2018        
(r334143)
+++ head/sys/dev/xen/xenstore/xenstore_dev.c    Thu May 24 10:18:31 2018        
(r334144)
@@ -214,6 +214,18 @@ xs_dev_watch_cb(struct xs_watch *watch, const char **v
        free(payload, M_XENSTORE);
 }
 
+static struct xs_dev_transaction *
+xs_dev_find_transaction(struct xs_dev_data *u, uint32_t tx_id)
+{
+       struct xs_dev_transaction *trans;
+
+       LIST_FOREACH(trans, &u->transactions, list)
+               if (trans->handle.id == tx_id)
+                       return (trans);
+
+       return (NULL);
+}
+
 static int 
 xs_dev_read(struct cdev *dev, struct uio *uio, int ioflag)
 {
@@ -281,6 +293,12 @@ xs_dev_write(struct cdev *dev, struct uio *uio, int io
        case XS_MKDIR:
        case XS_RM:
        case XS_SET_PERMS:
+               /* Check that this transaction id is not hijacked. */
+               if (u->u.msg.tx_id != 0 &&
+                   xs_dev_find_transaction(u, u->u.msg.tx_id) == NULL) {
+                       error = EINVAL;
+                       break;
+               }
                error = xs_dev_request_and_reply(&u->u.msg, &reply);
                if (!error) {
                        if (u->u.msg.type == XS_TRANSACTION_START) {
@@ -289,12 +307,10 @@ xs_dev_write(struct cdev *dev, struct uio *uio, int io
                                trans->handle.id = strtoul(reply, NULL, 0);
                                LIST_INSERT_HEAD(&u->transactions, trans, list);
                        } else if (u->u.msg.type == XS_TRANSACTION_END) {
-                               LIST_FOREACH(trans, &u->transactions, list)
-                                       if (trans->handle.id == u->u.msg.tx_id)
-                                               break;
-#if 0 /* XXX does this mean the list is empty? */
-                               BUG_ON(&trans->list == &u->transactions);
-#endif
+                               trans = xs_dev_find_transaction(u,
+                                   u->u.msg.tx_id);
+                               KASSERT(trans != NULL,
+                                   ("Unable to find transaction"));
                                LIST_REMOVE(trans, list);
                                free(trans, M_XENSTORE);
                        }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to