Xen Security Advisory 313 v3 (CVE-2020-11740,CVE-2020-11741) - multiple xenoprof issues

Xen Security Advisory 313 v3 (CVE-2020-11740,CVE-2020-11741) - multiple xenoprof issues

[Xen.org security team]

[-- Attachment #1: Type: text/plain, Size: 4329 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

    Xen Security Advisory CVE-2020-11740,CVE-2020-11741 / XSA-313
                              version 3

                       multiple xenoprof issues

UPDATES IN VERSION 3
====================

Public release.

ISSUE DESCRIPTION
=================

Unprivileged guests can request to map xenoprof buffers, even if
profiling has not been enabled for those guests.  These buffers were
not scrubbed.  This is CVE-2020-11740.

Furthermore, for guests for which "active" profiling was enabled by
the administrator, the xenoprof code uses the standard Xen shared ring
structure.  Unfortunately, this code did not treat the guest as a
potential adversary: it trusts the guest not to modify buffer size
information or modify head / tail pointers in unexpected ways.  This is
CVE-2020-11741.

IMPACT
======

A malicious guest may be able to access sensitive information
pertaining to other guests.  Guests with "active profiling" enabled
can crash the host (DoS).  Privilege escalation cannot be ruled out.

VULNERABLE SYSTEMS
==================

Only x86 PV guests can leverage the vulnerabilities.  Arm guests and
x86 HVM and PVH guests cannot leverage the vulnerabilities.

All Xen versions back to at least 3.2 are vulnerable.

Any x86 PV guest can leverage the information leak.  Only x86 PV guests
whose host administrator has explicitly enabled "active profiling" for an
untrusted guest can exploit the DoS / potential privilege escalation.

Only builds of Xen with the Xenoprof functionality enabled at build
time are vulnerable.  The option to disable the functionality at build
time was been introduced in Xen 4.7.

MITIGATION
==========

Never making any untrusted guests "active" will avoid all but the info
leak part of the vulnerabilities.  There's no known mitigation for the
information leak (lack of scrubbing).

CREDITS
=======

This issue was discovered by Ilja Van Sprundel of IOActive.

RESOLUTION
==========

Applying the attached set of patches resolves these issues.

The first patch fixes the information leak issue, and should be
applied to all x86 systems running untrusted PV guests.

The second patch fixes the "active profiling" issue.  Systems which do
not enable active profiling can safely skip patch 2.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa313-?.patch         xen-unstable, Xen 4.9.x - 4.13.x

$ sha256sum xsa313*
63a11c5470a6c24f19d3a8a45042306256e7422d6556e3d76badaa515deb76d6  xsa313.meta
f186ad88b492b730aeae3bd01083dd6c13813ce08bcd4ffc608d7af500633a62  xsa313-1.patch
9fbcb5f11e5029e7d371ddb3520443c2780f240edc3d24436872935e34a85c37  xsa313-2.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAl6VpdkMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZYZcH/0UHo2zmXGMDvZn1EF20ccKXNoZjvAE5TxSr/A/M
qkeASj4IMKlrPOrvs7aQSp97vECTz71Fxz2z7wpGwgIdiOYcRVg/t3b/+E1QSx5N
T7xYxxD9ULOLBQyPjYnXYwDC9+9yy+PZuWt3oPeXHrdtLI/5VY/gCzU+k+7bDABh
uljJ5KqxeQ5W8DOCR+XscQSZ9wiSkyh8MANjuJJ7uhtVDBo+ul94lrInJYEaBVpI
At5cU53B5nVGQ3RkNyWKjSW3VbL1TLgTdWAJNQOo+Z0OZJiKm6xQ6OYph2L4C4j4
e5A5c8UZAXLxVFWIMuiRW2GekOQEkGXtu+uJP00GuXm3+cQ=
=1C0J
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa313.meta --]
[-- Type: application/octet-stream, Size: 1552 bytes --]

{
  "XSA": 313,
  "SupportedVersions": [
    "master",
    "4.13",
    "4.12",
    "4.11",
    "4.10",
    "4.9"
  ],
  "Trees": [
    "xen"
  ],
  "Recipes": {
    "4.10": {
      "Recipes": {
        "xen": {
          "StableRef": "49a5d6e92317a7d9acbf0bdbd25b2809dfd84260",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    },
    "4.11": {
      "Recipes": {
        "xen": {
          "StableRef": "6bc54c0696c0f6f639598363d284c7188a9e20ae",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    },
    "4.12": {
      "Recipes": {
        "xen": {
          "StableRef": "824bdb432fc8831ee4684e45361a78faee4548ed",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    },
    "4.13": {
      "Recipes": {
        "xen": {
          "StableRef": "d3f3e447676667ef30b48708d359c8f8b13a9a03",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    },
    "4.9": {
      "Recipes": {
        "xen": {
          "StableRef": "cf2e9cc0ba0432f05cdca36dcd46be5fdfd7ca0c",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    },
    "master": {
      "Recipes": {
        "xen": {
          "StableRef": "e19b4b3b55f84e0cfcc02fe5d66965969a81c965",
          "Prereqs": [],
          "Patches": [
            "xsa313-?.patch"
          ]
        }
      }
    }
  }
}

[-- Attachment #3: xsa313-1.patch --]
[-- Type: application/octet-stream, Size: 871 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: xenoprof: clear buffer intended to be shared with guests

alloc_xenheap_pages() making use of MEMF_no_scrub is fine for Xen
internally used allocations, but buffers allocated to be shared with
(unpriviliged) guests need to be zapped of their prior content.

This is part of XSA-313.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>

--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -253,6 +253,9 @@ static int alloc_xenoprof_struct(
         return -ENOMEM;
     }
 
+    for ( i = 0; i < npages; ++i )
+        clear_page(d->xenoprof->rawbuf + i * PAGE_SIZE);
+
     d->xenoprof->npages = npages;
     d->xenoprof->nbuf = nvcpu;
     d->xenoprof->bufsize = bufsize;

[-- Attachment #4: xsa313-2.patch --]
[-- Type: application/octet-stream, Size: 5178 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: xenoprof: limit consumption of shared buffer data

Since a shared buffer can be written to by the guest, we may only read
the head and tail pointers from there (all other fields should only ever
be written to). Furthermore, for any particular operation the two values
must be read exactly once, with both checks and consumption happening
with the thus read values. (The backtrace related xenoprof_buf_space()
use in xenoprof_log_event() is an exception: The values used there get
re-checked by every subsequent xenoprof_add_sample().)

Since that code needed touching, also fix the double increment of the
lost samples count in case the backtrace related xenoprof_add_sample()
invocation in xenoprof_log_event() fails.

Where code is being touched anyway, add const as appropriate, but take
the opportunity to entirely drop the now unused domain parameter of
xenoprof_buf_space().

This is part of XSA-313.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>

--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -479,25 +479,22 @@ static int add_passive_list(XEN_GUEST_HA
 
 
 /* Get space in the buffer */
-static int xenoprof_buf_space(struct domain *d, xenoprof_buf_t * buf, int size)
+static int xenoprof_buf_space(int head, int tail, int size)
 {
-    int head, tail;
-
-    head = xenoprof_buf(d, buf, event_head);
-    tail = xenoprof_buf(d, buf, event_tail);
-
     return ((tail > head) ? 0 : size) + tail - head - 1;
 }
 
 /* Check for space and add a sample. Return 1 if successful, 0 otherwise. */
-static int xenoprof_add_sample(struct domain *d, xenoprof_buf_t *buf,
+static int xenoprof_add_sample(const struct domain *d,
+                               const struct xenoprof_vcpu *v,
                                uint64_t eip, int mode, int event)
 {
+    xenoprof_buf_t *buf = v->buffer;
     int head, tail, size;
 
     head = xenoprof_buf(d, buf, event_head);
     tail = xenoprof_buf(d, buf, event_tail);
-    size = xenoprof_buf(d, buf, event_size);
+    size = v->event_size;
     
     /* make sure indexes in shared buffer are sane */
     if ( (head < 0) || (head >= size) || (tail < 0) || (tail >= size) )
@@ -506,7 +503,7 @@ static int xenoprof_add_sample(struct do
         return 0;
     }
 
-    if ( xenoprof_buf_space(d, buf, size) > 0 )
+    if ( xenoprof_buf_space(head, tail, size) > 0 )
     {
         xenoprof_buf(d, buf, event_log[head].eip) = eip;
         xenoprof_buf(d, buf, event_log[head].mode) = mode;
@@ -530,7 +527,6 @@ static int xenoprof_add_sample(struct do
 int xenoprof_add_trace(struct vcpu *vcpu, uint64_t pc, int mode)
 {
     struct domain *d = vcpu->domain;
-    xenoprof_buf_t *buf = d->xenoprof->vcpu[vcpu->vcpu_id].buffer;
 
     /* Do not accidentally write an escape code due to a broken frame. */
     if ( pc == XENOPROF_ESCAPE_CODE )
@@ -539,7 +535,8 @@ int xenoprof_add_trace(struct vcpu *vcpu
         return 0;
     }
 
-    return xenoprof_add_sample(d, buf, pc, mode, 0);
+    return xenoprof_add_sample(d, &d->xenoprof->vcpu[vcpu->vcpu_id],
+                               pc, mode, 0);
 }
 
 void xenoprof_log_event(struct vcpu *vcpu, const struct cpu_user_regs *regs,
@@ -570,17 +567,22 @@ void xenoprof_log_event(struct vcpu *vcp
     /* Provide backtrace if requested. */
     if ( backtrace_depth > 0 )
     {
-        if ( (xenoprof_buf_space(d, buf, v->event_size) < 2) ||
-             !xenoprof_add_sample(d, buf, XENOPROF_ESCAPE_CODE, mode, 
-                                  XENOPROF_TRACE_BEGIN) )
+        if ( xenoprof_buf_space(xenoprof_buf(d, buf, event_head),
+                                xenoprof_buf(d, buf, event_tail),
+                                v->event_size) < 2 )
         {
             xenoprof_buf(d, buf, lost_samples)++;
             lost_samples++;
             return;
         }
+
+        /* xenoprof_add_sample() will increment lost_samples on failure */
+        if ( !xenoprof_add_sample(d, v, XENOPROF_ESCAPE_CODE, mode,
+                                  XENOPROF_TRACE_BEGIN) )
+            return;
     }
 
-    if ( xenoprof_add_sample(d, buf, pc, mode, event) )
+    if ( xenoprof_add_sample(d, v, pc, mode, event) )
     {
         if ( is_active(vcpu->domain) )
             active_samples++;
--- a/xen/include/xen/xenoprof.h
+++ b/xen/include/xen/xenoprof.h
@@ -61,12 +61,12 @@ struct xenoprof {
 
 #ifndef CONFIG_COMPAT
 #define XENOPROF_COMPAT(x) 0
-#define xenoprof_buf(d, b, field) ((b)->field)
+#define xenoprof_buf(d, b, field) ACCESS_ONCE((b)->field)
 #else
 #define XENOPROF_COMPAT(x) ((x)->is_compat)
-#define xenoprof_buf(d, b, field) (*(!(d)->xenoprof->is_compat ? \
-                                       &(b)->native.field : \
-                                       &(b)->compat.field))
+#define xenoprof_buf(d, b, field) ACCESS_ONCE(*(!(d)->xenoprof->is_compat \
+                                                ? &(b)->native.field \
+                                                : &(b)->compat.field))
 #endif
 
 struct domain;