From f971d812eab00ec7bd791517a99668b08b5c9411 Mon Sep 17 00:00:00 2001 From: Michael Young Date: Aug 23 2017 21:33:48 +0000 Subject: full fix for XSA-226, replacing workaround drop conflict of xendomain and libvirtd as can cause problems (#1398590) add-to-physmap error paths fail to release lock on ARM [XSA-235] (#1484476) Qemu: audio: host memory leakage via capture buffer [CVE-2017-8309] (#1446521) Qemu: input: host memory leakage via keyboard events [CVE-2017-8379] (#1446561) --- diff --git a/droplibvirtconflict.patch b/droplibvirtconflict.patch new file mode 100644 index 0000000..c96dddb --- /dev/null +++ b/droplibvirtconflict.patch @@ -0,0 +1,10 @@ +--- xen-4.9.0/tools/hotplug/Linux/systemd/xendomains.service.in.orig 2017-08-22 21:53:54.706306958 +0100 ++++ xen-4.9.0/tools/hotplug/Linux/systemd/xendomains.service.in 2017-08-22 22:04:55.864754640 +0100 +@@ -5,7 +5,6 @@ + After=network-online.target + After=remote-fs.target + ConditionPathExists=/proc/xen/capabilities +-Conflicts=libvirtd.service + + [Service] + Type=oneshot diff --git a/qemu.git-3268a845f41253fb55852a8429c32b50f36f349a.patch b/qemu.git-3268a845f41253fb55852a8429c32b50f36f349a.patch new file mode 100644 index 0000000..f3129dd --- /dev/null +++ b/qemu.git-3268a845f41253fb55852a8429c32b50f36f349a.patch @@ -0,0 +1,38 @@ +From 3268a845f41253fb55852a8429c32b50f36f349a Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Fri, 28 Apr 2017 09:56:12 +0200 +Subject: [PATCH] audio: release capture buffers + +AUD_add_capture() allocates two buffers which are never released. +Add the missing calls to AUD_del_capture(). + +Impact: Allows vnc clients to exhaust host memory by repeatedly +starting and stopping audio capture. + +Fixes: CVE-2017-8309 +Cc: P J P +Cc: Huawei PSIRT +Reported-by: "Jiangxin (hunter, SCC)" +Signed-off-by: Gerd Hoffmann +Reviewed-by: Prasad J Pandit +Message-id: 20170428075612.9997-1-kraxel@redhat.com +--- + audio/audio.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/audio/audio.c b/audio/audio.c +index c8898d8..beafed2 100644 +--- a/audio/audio.c ++++ b/audio/audio.c +@@ -2028,6 +2028,8 @@ void AUD_del_capture (CaptureVoiceOut *cap, void *cb_opaque) + sw = sw1; + } + QLIST_REMOVE (cap, entries); ++ g_free (cap->hw.mix_buf); ++ g_free (cap->buf); + g_free (cap); + } + return; +-- +1.8.3.1 + diff --git a/qemu.git-fa18f36a461984eae50ab957e47ec78dae3c14fc.patch b/qemu.git-fa18f36a461984eae50ab957e47ec78dae3c14fc.patch new file mode 100644 index 0000000..8574b3e --- /dev/null +++ b/qemu.git-fa18f36a461984eae50ab957e47ec78dae3c14fc.patch @@ -0,0 +1,90 @@ +From fa18f36a461984eae50ab957e47ec78dae3c14fc Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Fri, 28 Apr 2017 10:42:37 +0200 +Subject: [PATCH] input: limit kbd queue depth + +Apply a limit to the number of items we accept into the keyboard queue. + +Impact: Without this limit vnc clients can exhaust host memory by +sending keyboard events faster than qemu feeds them to the guest. + +Fixes: CVE-2017-8379 +Cc: P J P +Cc: Huawei PSIRT +Reported-by: jiangxin1@huawei.com +Signed-off-by: Gerd Hoffmann +Message-id: 20170428084237.23960-1-kraxel@redhat.com +--- + ui/input.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/ui/input.c b/ui/input.c +index ed88cda..fb1f404 100644 +--- a/ui/input.c ++++ b/ui/input.c +@@ -41,6 +41,8 @@ static QTAILQ_HEAD(QemuInputEventQueueHead, QemuInputEventQueue) kbd_queue = + QTAILQ_HEAD_INITIALIZER(kbd_queue); + static QEMUTimer *kbd_timer; + static uint32_t kbd_default_delay_ms = 10; ++static uint32_t queue_count; ++static uint32_t queue_limit = 1024; + + QemuInputHandlerState *qemu_input_handler_register(DeviceState *dev, + QemuInputHandler *handler) +@@ -268,6 +270,7 @@ static void qemu_input_queue_process(void *opaque) + break; + } + QTAILQ_REMOVE(queue, item, node); ++ queue_count--; + g_free(item); + } + } +@@ -282,6 +285,7 @@ static void qemu_input_queue_delay(struct QemuInputEventQueueHead *queue, + item->delay_ms = delay_ms; + item->timer = timer; + QTAILQ_INSERT_TAIL(queue, item, node); ++ queue_count++; + + if (start_timer) { + timer_mod(item->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +@@ -298,6 +302,7 @@ static void qemu_input_queue_event(struct QemuInputEventQueueHead *queue, + item->src = src; + item->evt = evt; + QTAILQ_INSERT_TAIL(queue, item, node); ++ queue_count++; + } + + static void qemu_input_queue_sync(struct QemuInputEventQueueHead *queue) +@@ -306,6 +311,7 @@ static void qemu_input_queue_sync(struct QemuInputEventQueueHead *queue) + + item->type = QEMU_INPUT_QUEUE_SYNC; + QTAILQ_INSERT_TAIL(queue, item, node); ++ queue_count++; + } + + void qemu_input_event_send_impl(QemuConsole *src, InputEvent *evt) +@@ -381,7 +387,7 @@ void qemu_input_event_send_key(QemuConsole *src, KeyValue *key, bool down) + qemu_input_event_send(src, evt); + qemu_input_event_sync(); + qapi_free_InputEvent(evt); +- } else { ++ } else if (queue_count < queue_limit) { + qemu_input_queue_event(&kbd_queue, src, evt); + qemu_input_queue_sync(&kbd_queue); + } +@@ -409,8 +415,10 @@ void qemu_input_event_send_key_delay(uint32_t delay_ms) + kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, qemu_input_queue_process, + &kbd_queue); + } +- qemu_input_queue_delay(&kbd_queue, kbd_timer, +- delay_ms ? delay_ms : kbd_default_delay_ms); ++ if (queue_count < queue_limit) { ++ qemu_input_queue_delay(&kbd_queue, kbd_timer, ++ delay_ms ? delay_ms : kbd_default_delay_ms); ++ } + } + + InputEvent *qemu_input_event_new_btn(InputButton btn, bool down) +-- +1.8.3.1 + diff --git a/qemu.trad.CVE-2017-8309.patch b/qemu.trad.CVE-2017-8309.patch new file mode 100644 index 0000000..10b5b05 --- /dev/null +++ b/qemu.trad.CVE-2017-8309.patch @@ -0,0 +1,38 @@ +From 3268a845f41253fb55852a8429c32b50f36f349a Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Fri, 28 Apr 2017 09:56:12 +0200 +Subject: [PATCH] audio: release capture buffers + +AUD_add_capture() allocates two buffers which are never released. +Add the missing calls to AUD_del_capture(). + +Impact: Allows vnc clients to exhaust host memory by repeatedly +starting and stopping audio capture. + +Fixes: CVE-2017-8309 +Cc: P J P +Cc: Huawei PSIRT +Reported-by: "Jiangxin (hunter, SCC)" +Signed-off-by: Gerd Hoffmann +Reviewed-by: Prasad J Pandit +Message-id: 20170428075612.9997-1-kraxel@redhat.com +--- + audio/audio.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/audio/audio.c b/audio/audio.c +index c8898d8..beafed2 100644 +--- a/audio/audio.c ++++ b/audio/audio.c +@@ -2028,6 +2028,8 @@ void AUD_del_capture (CaptureVoiceOut *cap, void *cb_opaque) + sw = sw1; + } + LIST_REMOVE (cap, entries); ++ qemu_free (cap->hw.mix_buf); ++ qemu_free (cap->buf); + qemu_free (cap); + } + return; +-- +1.8.3.1 + diff --git a/xen.spec b/xen.spec index 90b0097..a4dbd14 100644 --- a/xen.spec +++ b/xen.spec @@ -50,7 +50,7 @@ Summary: Xen is a virtual machine monitor Name: xen Version: 4.8.1 -Release: 6%{?dist} +Release: 7%{?dist} Group: Development/Libraries License: GPLv2+ and LGPLv2+ and BSD URL: http://xen.org/ @@ -144,10 +144,16 @@ Patch75: xsa225.patch Patch76: qemu.git-8409dc884a201bf74b30a9d232b6bbdd00cb7e2b.patch Patch77: qemu.git-215902d7b6fb50c6fc216fc74f770858278ed904.patch Patch78: qemu.trad.CVE-2017-7718.patch -Patch79: xsa226.patch Patch80: xsa227.patch Patch81: xsa228-4.8.patch Patch82: xsa230.patch +Patch83: xsa226.0001-gnttab-dont-use-possibly-unbounded-tail-calls.patch +Patch84: xsa226.0002-gnttab-fix-transitive-grant-handling.patch +Patch85: droplibvirtconflict.patch +Patch86: xsa235-4.9.patch +Patch87: qemu.git-3268a845f41253fb55852a8429c32b50f36f349a.patch +Patch88: qemu.trad.CVE-2017-8309.patch +Patch89: qemu.git-fa18f36a461984eae50ab957e47ec78dae3c14fc.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -366,10 +372,13 @@ manage Xen virtual machines. %patch73 -p1 %patch74 -p1 %patch75 -p1 -%patch79 -p1 %patch80 -p1 %patch81 -p1 %patch82 -p1 +%patch83 -p1 +%patch84 -p1 +%patch85 -p1 +%patch86 -p1 # qemu-xen-traditional patches pushd tools/qemu-xen-traditional @@ -380,6 +389,7 @@ pushd tools/qemu-xen-traditional %patch49 -p1 %patch55 -p1 %patch78 -p1 +%patch88 -p1 popd # qemu-xen patches @@ -408,6 +418,8 @@ pushd tools/qemu-xen %patch60 -p1 %patch76 -p1 %patch77 -p1 +%patch87 -p1 +%patch89 -p1 popd # stubdom sources @@ -910,6 +922,15 @@ rm -rf %{buildroot} %endif %changelog +* Wed Aug 23 2017 Michael Young - 4.8.1-7 +- full fix for XSA-226, replacing workaround +- drop conflict of xendomain and libvirtd as can cause problems (#1398590) +- add-to-physmap error paths fail to release lock on ARM [XSA-235] (#1484476) +- Qemu: audio: host memory leakage via capture buffer [CVE-2017-8309] + (#1446521) +- Qemu: input: host memory leakage via keyboard events [CVE-2017-8379] + (#1446561) + * Tue Aug 15 2017 Michael Young - 4.8.1-6 - Qemu: serial: host memory leakage 16550A UART emulation [CVE-2017-5579] (#1416162) diff --git a/xsa226.0001-gnttab-dont-use-possibly-unbounded-tail-calls.patch b/xsa226.0001-gnttab-dont-use-possibly-unbounded-tail-calls.patch new file mode 100644 index 0000000..7711d3f --- /dev/null +++ b/xsa226.0001-gnttab-dont-use-possibly-unbounded-tail-calls.patch @@ -0,0 +1,134 @@ +From: Jan Beulich +Subject: gnttab: don't use possibly unbounded tail calls + +There is no guarantee that the compiler would actually translate them +to branches instead of calls, so only ones with a known recursion limit +are okay: +- __release_grant_for_copy() can call itself only once, as + __acquire_grant_for_copy() won't permit use of multi-level transitive + grants, +- __acquire_grant_for_copy() is fine to call itself with the last + argument false, as that prevents further recursion, +- __acquire_grant_for_copy() must not call itself to recover from an + observed change to the active entry's pin count + +This is part of CVE-2017-12135 / XSA-226. + +Signed-off-by: Jan Beulich + +--- a/xen/common/grant_table.c ++++ b/xen/common/grant_table.c +@@ -2103,8 +2103,10 @@ __release_grant_for_copy( + + if ( td != rd ) + { +- /* Recursive calls, but they're tail calls, so it's +- okay. */ ++ /* ++ * Recursive calls, but they're bounded (acquire permits only a single ++ * level of transitivity), so it's okay. ++ */ + if ( released_write ) + __release_grant_for_copy(td, trans_gref, 0); + else if ( released_read ) +@@ -2255,10 +2257,11 @@ __acquire_grant_for_copy( + return rc; + } + +- /* We dropped the lock, so we have to check that nobody +- else tried to pin (or, for that matter, unpin) the +- reference in *this* domain. If they did, just give up +- and try again. */ ++ /* ++ * We dropped the lock, so we have to check that nobody else tried ++ * to pin (or, for that matter, unpin) the reference in *this* ++ * domain. If they did, just give up and tell the caller to retry. ++ */ + if ( act->pin != old_pin ) + { + __fixup_status_for_copy_pin(act, status); +@@ -2266,9 +2269,8 @@ __acquire_grant_for_copy( + active_entry_release(act); + grant_read_unlock(rgt); + put_page(*page); +- return __acquire_grant_for_copy(rd, gref, ldom, readonly, +- frame, page, page_off, length, +- allow_transitive); ++ *page = NULL; ++ return ERESTART; + } + + /* The actual remote remote grant may or may not be a +@@ -2574,7 +2576,7 @@ static int gnttab_copy_one(const struct + { + gnttab_copy_release_buf(src); + rc = gnttab_copy_claim_buf(op, &op->source, src, GNTCOPY_source_gref); +- if ( rc < 0 ) ++ if ( rc ) + goto out; + } + +@@ -2584,7 +2586,7 @@ static int gnttab_copy_one(const struct + { + gnttab_copy_release_buf(dest); + rc = gnttab_copy_claim_buf(op, &op->dest, dest, GNTCOPY_dest_gref); +- if ( rc < 0 ) ++ if ( rc ) + goto out; + } + +@@ -2593,6 +2595,14 @@ static int gnttab_copy_one(const struct + return rc; + } + ++/* ++ * gnttab_copy(), other than the various other helpers of ++ * do_grant_table_op(), returns (besides possible error indicators) ++ * "count - i" rather than "i" to ensure that even if no progress ++ * was made at all (perhaps due to gnttab_copy_one() returning a ++ * positive value) a non-zero value is being handed back (zero needs ++ * to be avoided, as that means "success, all done"). ++ */ + static long gnttab_copy( + XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count) + { +@@ -2606,7 +2616,7 @@ static long gnttab_copy( + { + if ( i && hypercall_preempt_check() ) + { +- rc = i; ++ rc = count - i; + break; + } + +@@ -2616,13 +2626,20 @@ static long gnttab_copy( + break; + } + +- op.status = gnttab_copy_one(&op, &dest, &src); +- if ( op.status != GNTST_okay ) ++ rc = gnttab_copy_one(&op, &dest, &src); ++ if ( rc > 0 ) ++ { ++ rc = count - i; ++ break; ++ } ++ if ( rc != GNTST_okay ) + { + gnttab_copy_release_buf(&src); + gnttab_copy_release_buf(&dest); + } + ++ op.status = rc; ++ rc = 0; + if ( unlikely(__copy_field_to_guest(uop, &op, status)) ) + { + rc = -EFAULT; +@@ -3160,6 +3177,7 @@ do_grant_table_op( + rc = gnttab_copy(copy, count); + if ( rc > 0 ) + { ++ rc = count - rc; + guest_handle_add_offset(copy, rc); + uop = guest_handle_cast(copy, void); + } diff --git a/xsa226.0002-gnttab-fix-transitive-grant-handling.patch b/xsa226.0002-gnttab-fix-transitive-grant-handling.patch new file mode 100644 index 0000000..2cf93bd --- /dev/null +++ b/xsa226.0002-gnttab-fix-transitive-grant-handling.patch @@ -0,0 +1,280 @@ +From: Jan Beulich +Subject: gnttab: fix transitive grant handling + +Processing of transitive grants must not use the fast path, or else +reference counting breaks due to the skipped recursive call to +__acquire_grant_for_copy() (its __release_grant_for_copy() +counterpart occurs independent of original pin count). Furthermore +after re-acquiring temporarily dropped locks we need to verify no grant +properties changed if the original pin count was non-zero; checking +just the pin counts is sufficient only for well-behaved guests. As a +result, __release_grant_for_copy() needs to mirror that new behavior. + +Furthermore a __release_grant_for_copy() invocation was missing on the +retry path of __acquire_grant_for_copy(), and gnttab_set_version() also +needs to bail out upon encountering a transitive grant. + +This is part of CVE-2017-12135 / XSA-226. + +Reported-by: Andrew Cooper +Signed-off-by: Jan Beulich +Reviewed-by: Andrew Cooper + +--- a/xen/common/grant_table.c ++++ b/xen/common/grant_table.c +@@ -2050,13 +2050,8 @@ __release_grant_for_copy( + unsigned long r_frame; + uint16_t *status; + grant_ref_t trans_gref; +- int released_read; +- int released_write; + struct domain *td; + +- released_read = 0; +- released_write = 0; +- + grant_read_lock(rgt); + + act = active_entry_acquire(rgt, gref); +@@ -2086,17 +2081,11 @@ __release_grant_for_copy( + + act->pin -= GNTPIN_hstw_inc; + if ( !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) ) +- { +- released_write = 1; + gnttab_clear_flag(_GTF_writing, status); +- } + } + + if ( !act->pin ) +- { + gnttab_clear_flag(_GTF_reading, status); +- released_read = 1; +- } + + active_entry_release(act); + grant_read_unlock(rgt); +@@ -2104,13 +2093,10 @@ __release_grant_for_copy( + if ( td != rd ) + { + /* +- * Recursive calls, but they're bounded (acquire permits only a single ++ * Recursive call, but it is bounded (acquire permits only a single + * level of transitivity), so it's okay. + */ +- if ( released_write ) +- __release_grant_for_copy(td, trans_gref, 0); +- else if ( released_read ) +- __release_grant_for_copy(td, trans_gref, 1); ++ __release_grant_for_copy(td, trans_gref, readonly); + + rcu_unlock_domain(td); + } +@@ -2184,8 +2170,108 @@ __acquire_grant_for_copy( + act->domid, ldom, act->pin); + + old_pin = act->pin; +- if ( !act->pin || +- (!readonly && !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) ) ++ if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive ) ++ { ++ if ( (!old_pin || (!readonly && ++ !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)))) && ++ (rc = _set_status_v2(ldom, readonly, 0, shah, act, ++ status)) != GNTST_okay ) ++ goto unlock_out; ++ ++ if ( !allow_transitive ) ++ PIN_FAIL(unlock_out_clear, GNTST_general_error, ++ "transitive grant when transitivity not allowed\n"); ++ ++ trans_domid = sha2->transitive.trans_domid; ++ trans_gref = sha2->transitive.gref; ++ barrier(); /* Stop the compiler from re-loading ++ trans_domid from shared memory */ ++ if ( trans_domid == rd->domain_id ) ++ PIN_FAIL(unlock_out_clear, GNTST_general_error, ++ "transitive grants cannot be self-referential\n"); ++ ++ /* ++ * We allow the trans_domid == ldom case, which corresponds to a ++ * grant being issued by one domain, sent to another one, and then ++ * transitively granted back to the original domain. Allowing it ++ * is easy, and means that you don't need to go out of your way to ++ * avoid it in the guest. ++ */ ++ ++ /* We need to leave the rrd locked during the grant copy. */ ++ td = rcu_lock_domain_by_id(trans_domid); ++ if ( td == NULL ) ++ PIN_FAIL(unlock_out_clear, GNTST_general_error, ++ "transitive grant referenced bad domain %d\n", ++ trans_domid); ++ ++ /* ++ * __acquire_grant_for_copy() could take the lock on the ++ * remote table (if rd == td), so we have to drop the lock ++ * here and reacquire. ++ */ ++ active_entry_release(act); ++ grant_read_unlock(rgt); ++ ++ rc = __acquire_grant_for_copy(td, trans_gref, rd->domain_id, ++ readonly, &grant_frame, page, ++ &trans_page_off, &trans_length, 0); ++ ++ grant_read_lock(rgt); ++ act = active_entry_acquire(rgt, gref); ++ ++ if ( rc != GNTST_okay ) ++ { ++ __fixup_status_for_copy_pin(act, status); ++ rcu_unlock_domain(td); ++ active_entry_release(act); ++ grant_read_unlock(rgt); ++ return rc; ++ } ++ ++ /* ++ * We dropped the lock, so we have to check that the grant didn't ++ * change, and that nobody else tried to pin/unpin it. If anything ++ * changed, just give up and tell the caller to retry. ++ */ ++ if ( rgt->gt_version != 2 || ++ act->pin != old_pin || ++ (old_pin && (act->domid != ldom || act->frame != grant_frame || ++ act->start != trans_page_off || ++ act->length != trans_length || ++ act->trans_domain != td || ++ act->trans_gref != trans_gref || ++ !act->is_sub_page)) ) ++ { ++ __release_grant_for_copy(td, trans_gref, readonly); ++ __fixup_status_for_copy_pin(act, status); ++ rcu_unlock_domain(td); ++ active_entry_release(act); ++ grant_read_unlock(rgt); ++ put_page(*page); ++ *page = NULL; ++ return ERESTART; ++ } ++ ++ if ( !old_pin ) ++ { ++ act->domid = ldom; ++ act->start = trans_page_off; ++ act->length = trans_length; ++ act->trans_domain = td; ++ act->trans_gref = trans_gref; ++ act->frame = grant_frame; ++ act->gfn = -1ul; ++ /* ++ * The actual remote remote grant may or may not be a sub-page, ++ * but we always treat it as one because that blocks mappings of ++ * transitive grants. ++ */ ++ act->is_sub_page = 1; ++ } ++ } ++ else if ( !old_pin || ++ (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) ) + { + if ( (rc = _set_status(rgt->gt_version, ldom, + readonly, 0, shah, act, +@@ -2206,79 +2292,6 @@ __acquire_grant_for_copy( + trans_page_off = 0; + trans_length = PAGE_SIZE; + } +- else if ( (shah->flags & GTF_type_mask) == GTF_transitive ) +- { +- if ( !allow_transitive ) +- PIN_FAIL(unlock_out_clear, GNTST_general_error, +- "transitive grant when transitivity not allowed\n"); +- +- trans_domid = sha2->transitive.trans_domid; +- trans_gref = sha2->transitive.gref; +- barrier(); /* Stop the compiler from re-loading +- trans_domid from shared memory */ +- if ( trans_domid == rd->domain_id ) +- PIN_FAIL(unlock_out_clear, GNTST_general_error, +- "transitive grants cannot be self-referential\n"); +- +- /* We allow the trans_domid == ldom case, which +- corresponds to a grant being issued by one domain, sent +- to another one, and then transitively granted back to +- the original domain. Allowing it is easy, and means +- that you don't need to go out of your way to avoid it +- in the guest. */ +- +- /* We need to leave the rrd locked during the grant copy */ +- td = rcu_lock_domain_by_id(trans_domid); +- if ( td == NULL ) +- PIN_FAIL(unlock_out_clear, GNTST_general_error, +- "transitive grant referenced bad domain %d\n", +- trans_domid); +- +- /* +- * __acquire_grant_for_copy() could take the lock on the +- * remote table (if rd == td), so we have to drop the lock +- * here and reacquire +- */ +- active_entry_release(act); +- grant_read_unlock(rgt); +- +- rc = __acquire_grant_for_copy(td, trans_gref, rd->domain_id, +- readonly, &grant_frame, page, +- &trans_page_off, &trans_length, 0); +- +- grant_read_lock(rgt); +- act = active_entry_acquire(rgt, gref); +- +- if ( rc != GNTST_okay ) { +- __fixup_status_for_copy_pin(act, status); +- rcu_unlock_domain(td); +- active_entry_release(act); +- grant_read_unlock(rgt); +- return rc; +- } +- +- /* +- * We dropped the lock, so we have to check that nobody else tried +- * to pin (or, for that matter, unpin) the reference in *this* +- * domain. If they did, just give up and tell the caller to retry. +- */ +- if ( act->pin != old_pin ) +- { +- __fixup_status_for_copy_pin(act, status); +- rcu_unlock_domain(td); +- active_entry_release(act); +- grant_read_unlock(rgt); +- put_page(*page); +- *page = NULL; +- return ERESTART; +- } +- +- /* The actual remote remote grant may or may not be a +- sub-page, but we always treat it as one because that +- blocks mappings of transitive grants. */ +- is_sub_page = 1; +- act->gfn = -1ul; +- } + else if ( !(sha2->hdr.flags & GTF_sub_page) ) + { + rc = __get_paged_frame(sha2->full_page.frame, &grant_frame, page, readonly, rd); +@@ -2710,10 +2723,13 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA + case 2: + for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ ) + { +- if ( ((shared_entry_v2(gt, i).hdr.flags & GTF_type_mask) == +- GTF_permit_access) && +- (shared_entry_v2(gt, i).full_page.frame >> 32) ) ++ switch ( shared_entry_v2(gt, i).hdr.flags & GTF_type_mask ) + { ++ case GTF_permit_access: ++ if ( !(shared_entry_v2(gt, i).full_page.frame >> 32) ) ++ break; ++ /* fall through */ ++ case GTF_transitive: + gdprintk(XENLOG_WARNING, + "tried to change grant table version to 1 with non-representable entries\n"); + res = -ERANGE; diff --git a/xsa226.patch b/xsa226.patch deleted file mode 100644 index 48fae12..0000000 --- a/xsa226.patch +++ /dev/null @@ -1,133 +0,0 @@ -From: Andrew Cooper -Subject: grant_table: Default to v1, and disallow transitive grants - -The reference counting and locking discipline for transitive grants is broken. -Their use is therefore declared out of security support. - -This is XSA-226. - -Transitive grants are expected to be unconditionally available with grant -table v2. Hiding transitive grants alone is an ABI breakage for the guest. -Modern versions of Linux and the Windows PV drivers use grant table v1, but -older versions did use v2. - -In principle, disabling gnttab v2 entirely is the safer way to cause guests to -avoid using transitive grants. However, some older guests which defaulted to -using gnttab v2 don't tolerate falling back from v2 to v1 over migrate. - -This patch introduces a new command line option to control grant table -behaviour. One suboption allows a choice of the maximum grant table version -Xen will allow the guest to use, and defaults to v2. A different suboption -independently controls whether transitive grants can be used. - -The default case is: - - gnttab=max_ver:2 - -To disable gnttab v2 entirely, use: - - gnttab=max_ver:1 - -To allow gnttab v2 and transitive grants, use: - - gnttab=max_ver:2,transitive - -Reported-by: Jan Beulich -Signed-off-by: Andrew Cooper -diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown -index 4002eab..af079b4 100644 ---- a/docs/misc/xen-command-line.markdown -+++ b/docs/misc/xen-command-line.markdown -@@ -868,6 +868,22 @@ Controls EPT related features. - - Specify which console gdbstub should use. See **console**. - -+### gnttab -+> `= List of [ max_ver:, transitive ]` -+ -+> Default: `gnttab=max_ver:2,no-transitive` -+ -+Control various aspects of the grant table behaviour available to guests. -+ -+* `max_ver` Select the maximum grant table version to offer to guests. Valid -+version are 1 and 2. -+* `transitive` Permit or disallow the use of transitive grants. Note that the -+use of grant table v2 without transitive grants is an ABI breakage from the -+guests point of view. -+ -+*Warning:* -+Due to XSA-226, the use of transitive grants is outside of security support. -+ - ### gnttab\_max\_frames - > `= ` - -diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c -index ae34547..87131f8 100644 ---- a/xen/common/grant_table.c -+++ b/xen/common/grant_table.c -@@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", max_nr_grant_frames); - unsigned int __read_mostly max_grant_frames; - integer_param("gnttab_max_frames", max_grant_frames); - -+static unsigned int __read_mostly opt_gnttab_max_version = 2; -+static bool __read_mostly opt_transitive_grants; -+ -+static void __init parse_gnttab(char *s) -+{ -+ char *ss; -+ -+ do { -+ ss = strchr(s, ','); -+ if ( ss ) -+ *ss = '\0'; -+ -+ if ( !strncmp(s, "max_ver:", 8) ) -+ { -+ long ver = simple_strtol(s + 8, NULL, 10); -+ -+ if ( ver >= 1 && ver <= 2 ) -+ opt_gnttab_max_version = ver; -+ } -+ else -+ { -+ bool val = !!strncmp(s, "no-", 3); -+ -+ if ( !val ) -+ s += 3; -+ -+ if ( !strcmp(s, "transitive") ) -+ opt_transitive_grants = val; -+ } -+ -+ s = ss + 1; -+ } while ( ss ); -+} -+ -+custom_param("gnttab", parse_gnttab); -+ - /* The maximum number of grant mappings is defined as a multiplier of the - * maximum number of grant table entries. This defines the multiplier used. - * Pretty arbitrary. [POLICY] -@@ -2191,6 +2227,10 @@ __acquire_grant_for_copy( - } - else if ( (shah->flags & GTF_type_mask) == GTF_transitive ) - { -+ if ( !opt_transitive_grants ) -+ PIN_FAIL(unlock_out_clear, GNTST_general_error, -+ "transitive grant disallowed by policy\n"); -+ - if ( !allow_transitive ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, - "transitive grant when transitivity not allowed\n"); -@@ -3159,7 +3199,10 @@ do_grant_table_op( - } - case GNTTABOP_set_version: - { -- rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t)); -+ if ( opt_gnttab_max_version == 1 ) -+ rc = -ENOSYS; /* Behave as before set_version was introduced. */ -+ else -+ rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t)); - break; - } - case GNTTABOP_get_status_frames: diff --git a/xsa235-4.9.patch b/xsa235-4.9.patch new file mode 100644 index 0000000..25dd650 --- /dev/null +++ b/xsa235-4.9.patch @@ -0,0 +1,49 @@ +From: Jan Beulich +Subject: arm/mm: release grant lock on xenmem_add_to_physmap_one() error paths + +Commit 55021ff9ab ("xen/arm: add_to_physmap_one: Avoid to map mfn 0 if +an error occurs") introduced error paths not releasing the grant table +lock. Replace them by a suitable check after the lock was dropped. + +This is XSA-235. + +Reported-by: Wei Liu +Signed-off-by: Jan Beulich +Reviewed-by: Julien Grall + +--- a/xen/arch/arm/mm.c ++++ b/xen/arch/arm/mm.c +@@ -1164,7 +1164,7 @@ int xenmem_add_to_physmap_one( + if ( idx < nr_status_frames(d->grant_table) ) + mfn = virt_to_mfn(d->grant_table->status[idx]); + else +- return -EINVAL; ++ mfn = mfn_x(INVALID_MFN); + } + else + { +@@ -1175,14 +1175,21 @@ int xenmem_add_to_physmap_one( + if ( idx < nr_grant_frames(d->grant_table) ) + mfn = virt_to_mfn(d->grant_table->shared_raw[idx]); + else +- return -EINVAL; ++ mfn = mfn_x(INVALID_MFN); + } + +- d->arch.grant_table_gfn[idx] = gfn; ++ if ( mfn != mfn_x(INVALID_MFN) ) ++ { ++ d->arch.grant_table_gfn[idx] = gfn; + +- t = p2m_ram_rw; ++ t = p2m_ram_rw; ++ } + + grant_write_unlock(d->grant_table); ++ ++ if ( mfn == mfn_x(INVALID_MFN) ) ++ return -EINVAL; ++ + break; + case XENMAPSPACE_shared_info: + if ( idx != 0 )