[xen stable-4.16] tools/xenstore: let unread watch events time out

[patchbot@xxxxxxx]

commit d08cdf0b19daf948a6b9754e90de9bc304bcd262
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Tue Sep 13 07:35:07 2022 +0200
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 1 14:07:24 2022 +0000

    tools/xenstore: let unread watch events time out
    
    A future modification will limit the number of outstanding requests
    for a domain, where "outstanding" means that the response of the
    request or any resulting watch event hasn't been consumed yet.
    
    In order to avoid a malicious guest being capable to block other guests
    by not reading watch events, add a timeout for watch events. In case a
    watch event hasn't been consumed after this timeout, it is being
    deleted. Set the default timeout to 20 seconds (a random value being
    not too high).
    
    In order to support to specify other timeout values in future, use a
    generic command line option for that purpose:
    
    --timeout|-w watch-event=<seconds>
    
    This is part of XSA-326 / CVE-2022-42311.
    
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
    (cherry picked from commit 5285dcb1a5c01695c11e6397c95d906b5e765c98)
---
 tools/xenstore/xenstored_core.c | 133 +++++++++++++++++++++++++++++++++++++++-
 tools/xenstore/xenstored_core.h |   6 ++
 2 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index bf22438739..45244c021c 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -108,6 +108,8 @@ int quota_max_transaction = 10;
 int quota_nb_perms_per_node = 5;
 int quota_max_path_len = XENSTORE_REL_PATH_MAX;
 
+unsigned int timeout_watch_event_msec = 20000;
+
 void trace(const char *fmt, ...)
 {
        va_list arglist;
@@ -211,19 +213,92 @@ void reopen_log(void)
        }
 }
 
+static uint64_t get_now_msec(void)
+{
+       struct timespec now_ts;
+
+       if (clock_gettime(CLOCK_MONOTONIC, &now_ts))
+               barf_perror("Could not find time (clock_gettime failed)");
+
+       return now_ts.tv_sec * 1000 + now_ts.tv_nsec / 1000000;
+}
+
 static void free_buffered_data(struct buffered_data *out,
                               struct connection *conn)
 {
+       struct buffered_data *req;
+
        list_del(&out->list);
+
+       /*
+        * Update conn->timeout_msec with the next found timeout value in the
+        * queued pending requests.
+        */
+       if (out->timeout_msec) {
+               conn->timeout_msec = 0;
+               list_for_each_entry(req, &conn->out_list, list) {
+                       if (req->timeout_msec) {
+                               conn->timeout_msec = req->timeout_msec;
+                               break;
+                       }
+               }
+       }
+
        talloc_free(out);
 }
 
+static void check_event_timeout(struct connection *conn, uint64_t msecs,
+                               int *ptimeout)
+{
+       uint64_t delta;
+       struct buffered_data *out, *tmp;
+
+       if (!conn->timeout_msec)
+               return;
+
+       delta = conn->timeout_msec - msecs;
+       if (conn->timeout_msec <= msecs) {
+               delta = 0;
+               list_for_each_entry_safe(out, tmp, &conn->out_list, list) {
+                       /*
+                        * Only look at buffers with timeout and no data
+                        * already written to the ring.
+                        */
+                       if (out->timeout_msec && out->inhdr && !out->used) {
+                               if (out->timeout_msec > msecs) {
+                                       conn->timeout_msec = out->timeout_msec;
+                                       delta = conn->timeout_msec - msecs;
+                                       break;
+                               }
+
+                               /*
+                                * Free out without updating conn->timeout_msec,
+                                * as the update is done in this loop already.
+                                */
+                               out->timeout_msec = 0;
+                               trace("watch event path %s for domain %u timed 
out\n",
+                                     out->buffer, conn->id);
+                               free_buffered_data(out, conn);
+                       }
+               }
+               if (!delta) {
+                       conn->timeout_msec = 0;
+                       return;
+               }
+       }
+
+       if (*ptimeout == -1 || *ptimeout > delta)
+               *ptimeout = delta;
+}
+
 void conn_free_buffered_data(struct connection *conn)
 {
        struct buffered_data *out;
 
        while ((out = list_top(&conn->out_list, struct buffered_data, list)))
                free_buffered_data(out, conn);
+
+       conn->timeout_msec = 0;
 }
 
 static bool write_messages(struct connection *conn)
@@ -411,6 +486,7 @@ static void initialize_fds(int *p_sock_pollfd_idx, int 
*ptimeout)
 {
        struct connection *conn;
        struct wrl_timestampt now;
+       uint64_t msecs;
 
        if (fds)
                memset(fds, 0, sizeof(struct pollfd) * current_array_size);
@@ -431,10 +507,12 @@ static void initialize_fds(int *p_sock_pollfd_idx, int 
*ptimeout)
 
        wrl_gettime_now(&now);
        wrl_log_periodic(now);
+       msecs = get_now_msec();
 
        list_for_each_entry(conn, &connections, list) {
                if (conn->domain) {
                        wrl_check_timeout(conn->domain, now, ptimeout);
+                       check_event_timeout(conn, msecs, ptimeout);
                        if (conn_can_read(conn) ||
                            (conn_can_write(conn) &&
                             !list_empty(&conn->out_list)))
@@ -794,6 +872,7 @@ void send_reply(struct connection *conn, enum 
xsd_sockmsg_type type,
                return;
        bdata->inhdr = true;
        bdata->used = 0;
+       bdata->timeout_msec = 0;
 
        if (len <= DEFAULT_BUFFER_SIZE)
                bdata->buffer = bdata->default_buffer;
@@ -845,6 +924,12 @@ void send_event(struct connection *conn, const char *path, 
const char *token)
        bdata->hdr.msg.type = XS_WATCH_EVENT;
        bdata->hdr.msg.len = len;
 
+       if (timeout_watch_event_msec && domain_is_unprivileged(conn)) {
+               bdata->timeout_msec = get_now_msec() + timeout_watch_event_msec;
+               if (!conn->timeout_msec)
+                       conn->timeout_msec = bdata->timeout_msec;
+       }
+
        /* Queue for later transmission. */
        list_add_tail(&bdata->list, &conn->out_list);
 }
@@ -2201,6 +2286,9 @@ static void usage(void)
 "  -t, --transaction <nb>  limit the number of transaction allowed per 
domain,\n"
 "  -A, --perm-nb <nb>      limit the number of permissions per node,\n"
 "  -M, --path-max <chars>  limit the allowed Xenstore node path length,\n"
+"  -w, --timeout <what>=<seconds>   set the timeout in seconds for <what>,\n"
+"                          allowed timeout candidates are:\n"
+"                          watch-event: time a watch-event is kept pending\n"
 "  -R, --no-recovery       to request that no recovery should be attempted 
when\n"
 "                          the store is corrupted (debug only),\n"
 "  -I, --internal-db       store database in memory, not on disk\n"
@@ -2223,6 +2311,7 @@ static struct option options[] = {
        { "transaction", 1, NULL, 't' },
        { "perm-nb", 1, NULL, 'A' },
        { "path-max", 1, NULL, 'M' },
+       { "timeout", 1, NULL, 'w' },
        { "no-recovery", 0, NULL, 'R' },
        { "internal-db", 0, NULL, 'I' },
        { "verbose", 0, NULL, 'V' },
@@ -2236,6 +2325,39 @@ int dom0_domid = 0;
 int dom0_event = 0;
 int priv_domid = 0;
 
+static int get_optval_int(const char *arg)
+{
+       char *end;
+       long val;
+
+       val = strtol(arg, &end, 10);
+       if (!*arg || *end || val < 0 || val > INT_MAX)
+               barf("invalid parameter value \"%s\"\n", arg);
+
+       return val;
+}
+
+static bool what_matches(const char *arg, const char *what)
+{
+       unsigned int what_len = strlen(what);
+
+       return !strncmp(arg, what, what_len) && arg[what_len] == '=';
+}
+
+static void set_timeout(const char *arg)
+{
+       const char *eq = strchr(arg, '=');
+       int val;
+
+       if (!eq)
+               barf("quotas must be specified via <what>=<seconds>\n");
+       val = get_optval_int(eq + 1);
+       if (what_matches(arg, "watch-event"))
+               timeout_watch_event_msec = val * 1000;
+       else
+               barf("unknown timeout \"%s\"\n", arg);
+}
+
 int main(int argc, char *argv[])
 {
        int opt;
@@ -2250,7 +2372,7 @@ int main(int argc, char *argv[])
        orig_argc = argc;
        orig_argv = argv;
 
-       while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:A:M:T:RVW:U", 
options,
+       while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:A:M:T:RVW:w:U", 
options,
                                  NULL)) != -1) {
                switch (opt) {
                case 'D':
@@ -2300,6 +2422,9 @@ int main(int argc, char *argv[])
                        quota_max_path_len = min(XENSTORE_REL_PATH_MAX,
                                                 quota_max_path_len);
                        break;
+               case 'w':
+                       set_timeout(optarg);
+                       break;
                case 'e':
                        dom0_event = strtol(optarg, NULL, 10);
                        break;
@@ -2741,6 +2866,12 @@ static void add_buffered_data(struct buffered_data 
*bdata,
                barf("error restoring buffered data");
 
        memcpy(bdata->buffer, data, len);
+       if (bdata->hdr.msg.type == XS_WATCH_EVENT && timeout_watch_event_msec &&
+           domain_is_unprivileged(conn)) {
+               bdata->timeout_msec = get_now_msec() + timeout_watch_event_msec;
+               if (!conn->timeout_msec)
+                       conn->timeout_msec = bdata->timeout_msec;
+       }
 
        /* Queue for later transmission. */
        list_add_tail(&bdata->list, &conn->out_list);
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index e7ee87825c..8a81fc693f 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -27,6 +27,7 @@
 #include <fcntl.h>
 #include <stdbool.h>
 #include <stdint.h>
+#include <time.h>
 #include <errno.h>
 
 #include "xenstore_lib.h"
@@ -67,6 +68,8 @@ struct buffered_data
                char raw[sizeof(struct xsd_sockmsg)];
        } hdr;
 
+       uint64_t timeout_msec;
+
        /* The actual data. */
        char *buffer;
        char default_buffer[DEFAULT_BUFFER_SIZE];
@@ -118,6 +121,7 @@ struct connection
 
        /* Buffered output data */
        struct list_head out_list;
+       uint64_t timeout_msec;
 
        /* Transaction context for current request (NULL if none). */
        struct transaction *transaction;
@@ -244,6 +248,8 @@ extern int dom0_event;
 extern int priv_domid;
 extern int quota_nb_entry_per_domain;
 
+extern unsigned int timeout_watch_event_msec;
+
 /* Map the kernel's xenstore page. */
 void *xenbus_map(void);
 void unmap_xenbus(void *interface);
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.16