aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* update Xen version to 4.1.6-rc24.1.6-rc2Jan Beulich2013-08-272-2/+2
|
* Correct X2-APIC HVM emulationJuergen Gross2013-08-221-0/+1
| | | | | | | | | | commit 6859874b61d5ddaf5289e72ed2b2157739b72ca5 ("x86/HVM: fix x2APIC APIC_ID read emulation") introduced an error for the hvm emulation of x2apic. Any try to write to APIC_ICR MSR will result in a GP fault. Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> master commit: 69962e19ed432570f6cdcfdb5f6f22d6e3c54e6c master date: 2013-08-22 11:24:00 +0200
* xen: Add stdbool.h workaround for BSD.Tim Deegan2013-08-202-2/+15
| | | | | | | | | | | | | | | | | | On *BSD, stdbool.h lives in /usr/include, but we don't want to have that on the search path in case we pick up any headers from the build host's C libraries. Copy the equivalent hack already in place for stdarg.h: on all supported compilers the contents of stdbool.h are trivial, so just supply the things we need in a xen/stdbool.h header. Signed-off-by: Tim Deegan <tim@xen.org> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Keir Fraser <keir@xen.org> Tested-by: Patrick Welche <prlw1@cam.ac.uk> master commit: 7b9685ca4ed2fd723600ce66eb20a6d0c115b6cb master date: 2013-08-15 22:00:45 +0100
* x86/time: fix check for negative time in __update_vcpu_system_time()Tim Deegan2013-08-201-1/+2
| | | | | | | | Clang points out that u64 stime variable is always >= 0. Signed-off-by: Tim Deegan <tim@xen.org> master commit: ab7f9a793c78dfea81c037b34b0dd2db7070d8f8 master date: 2013-08-15 13:17:10 +0200
* x86/MTRR: fix range check in mtrr_add_page()Jan Beulich2013-08-201-1/+1
| | | | | | | | | | | Extracted from Yinghai Lu's Linux commit d5c78673 ("x86: Fix /proc/mtrr with base/size more than 44bits"). Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org> master commit: f67af6d5803b6a015e30cb490a94f9547cb0437c master date: 2013-08-14 11:20:26 +0200
* VT-d: protect against bogus information coming from BIOSJan Beulich2013-08-202-0/+6
| | | | | | | | | | | | | | | Add checks similar to those done by Linux: The DRHD address must not be all zeros or all ones (Linux only checks for zero), and capabilities as well as extended capabilities must not be all ones. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Ben Guthro <benjamin.guthro@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-by: Ben Guthro <benjamin.guthro@citrix.com> Acked by: Yang Zhang <yang.z.zhang@intel.com> Acked-by: Xiantao Zhang <xiantao.zhang@intel.com> master commit: e8e8b030ecf916fea19639f0b6a446c1c9dbe174 master date: 2013-08-14 11:18:24 +0200
* libelf: Fix typo in header guard macroPatrick Welche2013-08-201-2/+2
| | | | | | | | | | s/__LIBELF_PRIVATE_H_/__LIBELF_PRIVATE_H__/ Signed-off-by: Patrick Welche <prlw1@cam.ac.uk> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> master commit: 0aec8823501f8ee058c1ba673d2ac3e0f3f2e8db master date: 2013-08-08 12:47:38 +0100
* x86: explicit suffix in inline assembler (for clang).Tim Deegan2013-08-161-1/+1
| | | | | | | | | | | | | | This fixes the clang build, and has no effect on gcc's output. Signed-off-by: Tim Deegan <tim@xen.org> Committed-by: Jan Beulich <jbeulich@suse.com> master commit: 59a28b5f045331641cbf0c1fc8d5d67afe328939 master date: 2013-02-14 14:20:06 +0100 Note that this isn't just a build fix - if the "delta" input ends up in memory, gas would default to 32-bit operand size (and should really warn about the ambiguity).
* x86/time: Update wallclock in shared info when altering domain time offsetAndrew Cooper2013-08-081-0/+1
| | | | | | | | | | | | domain_set_time_offset() udpates d->time_offset_seconds, but does not correct the wallclock in the shared info, meaning that it is incorrect until the next XENPF_settime hypercall from dom0 which resynchronises the wallclock for all domains. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org> master commit: 915a59f25c5eddd86bc2cae6389d0ed2ab87e69e master date: 2013-07-18 09:16:15 +0200
* x86/cpuidle: Change logging for unknown APIC IDsAndrew Cooper2013-08-082-2/+5
| | | | | | | | | | | | | | | | | Dom0 uses this hypercall to pass ACPI information to Xen. It is not very uncommon for more cpus to be listed in the ACPI tables than are present on the system, particularly on systems with a common BIOS for a 2 and 4 socket server varients. As Dom0 does not control the number of entries in the ACPI tables, and is required to pass everything it finds to Xen, change the logging. There is now an single unconditional warning for the first unknown ID, and further warnings if "cpuinfo" is requested by the user on the command line. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> master commit: 85047d9e4f4afeb73bca1e98f705a2f4f1d51c03 master date: 2013-07-17 08:45:20 +0200
* x86/mm: Ensure useful progress in alloc_l2_table()Andrew Cooper2013-08-081-1/+2
| | | | | | | | | | | | | | | | While debugging the issue which turned out to be XSA-58, a printk in this loop showed that it was quite easy to never make useful progress, because of consistently failing the preemption check. One single l2 entry is a reasonable amount of work to do, even if an action is pending, and also assures forwards progress across repeat continuations. Tweak the continuation criteria to fail on the first iteration of the loop. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org> master commit: d3a55d7d9bb518efe08143d050deff9f4ee80ec1 master date: 2013-07-04 10:33:18 +0200
* update Xen version to 4.1.6-rc14.1.6-rc1Ian Jackson2013-07-191-1/+1
|
* tools/debugger/kdd: Remove dependencies files during make cleanDaniel Kiper2013-07-171-1/+1
| | | | | | | | Remove dependencies files during make clean. Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> (cherry picked from commit 38bdfb9197b93262248ff489eed336d80db52b54)
* tools/libfsimage: Fix clean and distclean make targetsDaniel Kiper2013-07-172-3/+3
| | | | | | | | | | | | | | | | | If there is a single colon for a given target and the target is redefined in another place (e.g. in included file) then make executes only new target and displays following warning: Makefile:35: warning: overriding commands for target `clean' tools/libfsimage/common/../../../tools/libfsimage/Rules.mk:25: warning: ignoring old commands for target `clean' To cope with that issue define all required targets as double-colon rules. Additionally, remove some redundant stuff. Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> (cherry picked from commit 667d8a84b244d02e9c6a2d02d6a02fc90c2efb4e)
* QEMU_TAG updateIan Jackson2013-07-171-3/+3
|
* pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)Marcel J.E. Mol2013-07-171-0/+2
| | | | | | | | | | | | | | | | Booting a fedora 19 domU failed because a it could not properly parse the grub.cfg file. This was cased by set default="${next_entry}" This statement actually is within an 'if' statement, so maybe it would be better to skip code within if/fi blocks... But this patch seems to work fine. Signed-off-by: Marcel Mol <marcel@mesa.nl> Acked-by: Ian Campbell <ian.campbell@citix.com> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> (cherry picked from commit d513814db6af2b298b8776d7ffc5fb1261e176f4)
* iommu/amd: Workaround for erratum 787Suravee Suthikulpanit2013-07-111-0/+8
| | | | | | | | | | | | | | | | | | | | | | | The IOMMU interrupt handling in bottom half must clear the PPR log interrupt and event log interrupt bits to re-enable the interrupt. This is done by writing 1 to the memory mapped register to clear the bit. Due to hardware bug, if the driver tries to clear this bit while the IOMMU hardware also setting this bit, the conflict will result with the bit being set. If the interrupt handling code does not make sure to clear this bit, subsequent changes in the event/PPR logs will no longer generating interrupts, and would result if buffer overflow. After clearing the bits, the driver must read back the register to verify. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Adjust to apply on top of heavily modified patch 1. Adjust flow to get away with a single readl() in each instance of the status register checks. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tim Deegan <tim@xen.org> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> master commit: 9eabb0735400e2b6059dfa3f0b47a426f61f570a master date: 2013-07-02 08:50:41 +0200
* iommu/amd: Fix logic for clearing the IOMMU interrupt bitsSuravee Suthikulpanit2013-07-112-25/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The IOMMU interrupt bits in the IOMMU status registers are "read-only, and write-1-to-clear (RW1C). Therefore, the existing logic which reads the register, set the bit, and then writing back the values could accidentally clear certain bits if it has been set. The correct logic would just be writing only the value which only set the interrupt bits, and leave the rest to zeros. This patch also, clean up #define masks as Jan has suggested. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> With iommu_interrupt_handler() properly having got switched its readl() from status to control register, the subsequent writel() needed to be switched too (and the RW1C comment there was bogus). Some of the cleanup went too far - undone. Further, with iommu_interrupt_handler() now actually disabling the interrupt sources, they also need to get re-enabled by the tasklet once it finished processing the respective log. This also implies re-running the tasklet so that log entries added between reading the log and re- enabling the interrupt will get handled in a timely manner. Finally, guest write emulation to the status register needs to be done with the RW1C (and RO for all other bits) semantics in mind too. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tim Deegan <tim@xen.org> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> master commit: 2823a0c7dfc979db316787e1dd42a8845e5825c0 master date: 2013-07-02 08:49:43 +0200
* x86: don't pass negative time to gtime_to_gtsc() (try 2)Jan Beulich2013-07-111-9/+6
| | | | | | | | | | | | | | | | | This mostly reverts commit eb60be3d ("x86: don't pass negative time to gtime_to_gtsc()") and instead corrects __update_vcpu_system_time()'s handling of this_cpu(cpu_time).stime_local_stamp dating back before the start of a HVM guest (which would otherwise lead to a negative value getting passed to gtime_to_gtsc(), causing scale_delta() to produce meaningless output). Flushing the value to zero was wrong, and printing a message for something that can validly happen wasn't very useful either. Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> master commit: 5ad914bc867c5a6a4957869c89918f4e1f9dd9c4 master date: 2013-07-02 08:48:03 +0200
* AMD/intremap: Prevent use of per-device vector maps until irq logic is fixedAndrew Cooper2013-07-111-2/+17
| | | | | | | | | | | | | | | | | | | | | | | XSA-36 changed the default vector map mode from global to per-device. This is because a global vector map does not prevent one PCI device from impersonating another and launching a DoS on the system. However, the per-device vector map logic is broken for devices with multiple MSI-X vectors, which can either result in a failed ASSERT() or misprogramming of a guests interrupt remapping tables. The core problem is not trivial to fix. In an effort to get AMD systems back to a non-regressed state, introduce a new type of vector map called per-device-global. This uses per-device vector maps in the IOMMU, but uses a single used_vector map for the core IRQ logic. This patch is intended to be removed as soon as the per-device logic is fixed correctly. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> master commit: f0fe8227624d5c02715ed086867d12cd24f6ff47 master date: 2013-06-27 14:01:18 +0200
* libelf: fix printing of pointersJan Beulich2013-07-111-3/+3
| | | | | | | | | Printing them as decimal number, the more with 0x prefix, is confusing and presumably relatively useless to most of us. Signed-off-by: Jan Beulich <jbeulich@suse.com> master commit: 59912eb06fda88af6c5ec16a2a382619d3829a7b master date: 2013-06-26 14:43:52 +0100
* x86/hvm: fix HVMOP_inject_trap return value on successTim Deegan2013-07-111-0/+1
| | | | | | | | | Reported-by: Antony Saba <Antony.Saba@mandiant.com> Signed-off-by: Tim Deegan <tim@xen.org> Acked-by: Keir Fraser <keir@xen.org> Acked-by: George Dunlap <george.dunlap@eu.citrix.com> master commit: a12d15d8c1d512a4ed6498b39f9058f69a1c1f6c master date: 2013-06-21 17:01:50 +0200
* x86/HVM: fix initialization of wallclock time for PVHVM on migrationRoger Pau Monné2013-07-111-19/+14
| | | | | | | | | | | | | | | | | Call update_domain_wallclock_time on hvm_latch_shinfo_size even if the bitness of the guest has already been set, this fixes the problem with the wallclock not being set for PVHVM guests on resume from migration. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Clean up the resulting code and retain the (slightly adjusted) original comment. Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> master commit: f8e8fd56bd7d5675e8331b4ec74bae76c9dbf24e master date: 2013-06-12 10:05:39 +0200
* x86/HVM: fix x2APIC APIC_ID read emulationZhenguo Wang2013-07-111-3/+20
| | | | | | | | | | | | | | | | | | | | APIC and x2APIC have different format for APIC_ID register. Need translation. Signed-off-by: Zhenguo Wang <wangzhenguo@huawei.com> Signed-off-by: Xiaowei Yang <xiaowei.yang@huawei.com> Convert code to use switch(), fixing coding style issue at once, and use GET_xAPIC_ID() on the value read instead of VLAPIC_ID() (reading the field again). In the course of this also properly reject both read and writes on the non-existing MSR corresponding to APIC_ICR2. Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> master commit: 6859874b61d5ddaf5289e72ed2b2157739b72ca5 master date: 2013-06-11 09:45:55 +0200
* libxl: suppress device assignment to HVM guest when there is no IOMMUJan Beulich2013-07-091-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | This in effect copies similar logic from xend: While there's no way to check whether a device is assigned to a particular guest, XEN_DOMCTL_test_assign_device at least allows checking whether an IOMMU is there and whether a device has been assign to _some_ guest. For the time being, this should be enough to cover for the missing error checking/recovery in other parts of libxl's device assignment paths. There remains a (functionality-, but not security-related) race in that the iommu should be set up earlier, but this is too risky a change for this stage of the 4.3 release. This is a security issue, XSA-61. Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> master commit: 826eb17271d3c647516d9944c47b0779afedea25 master date: 2013-07-01 15:20:28 +0100 master commit: 826eb17271d3c647516d9944c47b0779afedea25 master date: 2013-07-01 15:20:28 +0100
* xsm/flask: Fix XSM support for HVMOP_track_dirty_vramAurelien Chartier2013-07-041-3/+3
| | | | | | | | | | | | | The XSM check for HVMOP_track_dirty_vram is done with a call to xsm_hvm_param, therefore the switch handling that case should be located in flask_hvm_param and not in flask_hvmcontext. This was fixed upstream by the two following patches : * 875756ca34fabc7243c4a682ffd7008710a907e2 (add case in flask_hvm_param) * 652f94327383c5517b709f0a3e4b970216b3d375 (remove case from flask_hvmcontext) Signed-off-by: Aurelien Chartier <aurelien.chartier@citrix.com> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
* libxl: Restrict permissions on PV console device xenstore nodesIan Jackson2013-06-276-43/+83
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Matthew Daley has observed that the PV console protocol places sensitive host state into a guest writeable xenstore locations, this includes: - The pty used to communicate between the console backend daemon and its client, allowing the guest administrator to read and write arbitrary host files. - The output file, allowing the guest administrator to write arbitrary host files or to target arbitrary qemu chardevs which include sockets, udp, ptr, pipes etc (see -chardev in qemu(1) for a more complete list). - The maximum buffer size, allowing the guest administrator to consume more resources than the host administrator has configured. - The backend to use (qemu vs xenconsoled), potentially allowing the guest administrator to confuse host software. So we arrange to make the sensitive keys in the xenstore frontend directory read only for the guest. This is safe since the xenstore permissions model, unlike POSIX directory permissions, does not allow the guest to remove and recreate a node if it has write access to the containing directory. There are a few associated wrinkles: - The primary PV console is "special". It's xenstore node is not under the usual /devices/ subtree and it does not use the customary xenstore state machine protocol. Unfortunately its directory is used for other things, including the vnc-port node, which we do not want the guest to be able to write to. Rather than trying to track down all the possible secondary uses of this directory just make it r/o to the guest. All newly created subdirectories inherit these permissions and so are now safe by default. - The other serial consoles do use the customary xenstore state machine and therefore need write access to at least the "protocol" and "state" nodes, however they may also want to use arbitrary "feature-foo" nodes (although I'm not aware of any) and therefore we cannot simply lock down the entire frontend directory. Instead we add support to libxl__device_generic_add for frontend keys which are explicitly read only and use that to lock down the sensitive keys. - Minios' console frontend wants to write the "type" node, which it has no business doing since this is a host/toolstack level decision. This fails now that the node has become read only to the PV guest. Since the toolstack already writes this node just remove the attempt to set it. This is CVE-2013-2211 / XSA-57 Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Conflicts (4.2 backport): tools/libxl/libxl.c (no vtpm, free front_ro on error in libxl__device_console_add) Conflicts (4.1 backport): extras/mini-os/console/xenbus.c tools/libxl/libxl.c tools/libxl/libxl_device.c tools/libxl/libxl_internal.h tools/libxl/libxl_pci.c tools/libxl/libxl_xshelp.c - minios code was in xencons_ring.c - many places need &gc not just gc - libxl__xs_writev path is not const - varios minor context fixups
* x86: fix emuirq regression from XSA-21 fixJan Beulich2013-06-271-0/+2
| | | | | | | | The XSA-21 patch broke the assumption of "ret" being zero prior to the IRQ_UNBOUND check. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
* x86: fix page refcount handling in page table pin error pathJan Beulich2013-06-263-8/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | In the original patch 7 of the series addressing XSA-45 I mistakenly took the addition of the call to get_page_light() in alloc_page_type() to cover two decrements that would happen: One for the PGT_partial bit that is getting set along with the call, and the other for the page reference the caller hold (and would be dropping on its error path). But of course the additional page reference is tied to the PGT_partial bit, and hence any caller of a function that may leave ->arch.old_guest_table non-NULL for error cleanup purposes has to make sure a respective page reference gets retained. Similar issues were then also spotted elsewhere: In effect all callers of get_page_type_preemptible() need to deal with errors in similar ways. To make sure error handling can work this way without leaking page references, a respective assertion gets added to that function. This is CVE-2013-1432 / XSA-58. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Tim Deegan <tim@xen.org> master commit: 9b167bd2f394f821ae3252d74a15704a4bf91f6d master date: 2013-06-26 15:32:58 +0200
* x86/vtsc: update vcpu_time in hvm_set_guest_timeRoger Pau Monné2013-06-171-1/+13
| | | | | | | | | | | | | When using a vtsc, hvm_set_guest_time changes hvm_vcpu.stime_offset, which is used in the vcpu time structure to calculate the tsc_timestamp, so after updating stime_offset we need to propagate the change to vcpu_time in order for the guest to get the right time if using the PV clock. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com> master commit: 32c864a35ece2c24a336d183869a546798a4b241 master date: 2013-06-05 10:03:08 +0200
* x86/MCE: disable if MCE banks are not presentAravindh Puthiyaparambil2013-06-171-1/+8
| | | | | | | | | | | | | | | | | | | When booting Xen on VMware ESX 5.1 and Workstation 9, you hit a GPF during MCE initialization. The culprit is line 631 in set_poll_bankmask(): bitmap_copy(mb->bank_map, mca_allbanks->bank_map, nr_mce_banks); What is happening is that in mca_cap_init(), nr_mce_banks is being set to 0. This causes the allocation of bank_map to be set to ZERO_BLOCK_PTR which is the return value for zero-size allocation by xzalloc_array()/_xmalloc(). This results in the bitmap_copy() to fail disastrously. The following patch fixes this issue. Signed-off-by: Aravindh Puthiyaparambil <aravindp@cisco.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christoph Egger <chegger@amazon.de> master commit: 5cffb77c4072fa5b46700a2dbb3e46c5a54eba6d master date: 2013-06-03 15:42:46 +0200
* x86: fix ordering of operations in destroy_irq()Jan Beulich2013-06-171-23/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The fix for XSA-36, switching the default of vector map management to be per-device, exposed more readily a problem with the cleanup of these vector maps: dynamic_irq_cleanup() clearing desc->arch.used_vectors keeps the subsequently invoked clear_irq_vector() from clearing the bits for both the in-use and a possibly still outstanding old vector. Fix this by folding dynamic_irq_cleanup() into destroy_irq(), which was its only caller, deferring the clearing of the vector map pointer until after clear_irq_vector(). Once at it, also defer resetting of desc->handler until after the loop around smp_mb() checking for IRQ_INPROGRESS to be clear, fixing a (mostly theoretical) issue with the intercation with do_IRQ(): If we don't defer the pointer reset, do_IRQ() could, for non-guest IRQs, call ->ack() and ->end() with different ->handler pointers, potentially leading to an IRQ remaining un-acked. The issue is mostly theoretical because non-guest IRQs are subject to destroy_irq() only on (boot time) error paths. As to the changed locking: Invoking clear_irq_vector() with desc->lock held is okay because vector_lock already nests inside desc->lock (proven by set_desc_affinity(), which takes vector_lock and gets called from various desc->handler->ack implementations, getting invoked with desc->lock held). Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com> master commit: 43427e65ccfea0c6dc0232f358287e6cc616b507 master date: 2013-05-31 08:35:22 +0200
* libxc: check blob size before proceeding in xc_dom_check_gzipMatthew Daley2013-06-141-0/+5
| | | | | | This is part of the fix to a security issue, XSA-55. Signed-off-by: Matthew Daley <mattjd@gmail.com>
* libxc: range checks in xc_dom_p2m_host and _guestIan Jackson2013-06-141-0/+4
| | | | | | | | | | | | | | | | These functions take guest pfns and look them up in the p2m. They did no range checking. However, some callers, notably xc_dom_boot.c:setup_hypercall_page want to pass untrusted guest-supplied value(s). It is most convenient to detect this here and return INVALID_MFN. This is part of the fix to a security issue, XSA-55. Changes from Xen 4.2 version of this patch: * 4.2 lacks dom->rambase_pfn, so don't add/subtract/check it. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libxc: check return values from mallocIan Jackson2013-06-1410-3/+56
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A sufficiently malformed input to libxc (such as a malformed input ELF or other guest-controlled data) might cause one of libxc's malloc() to fail. In this case we need to make sure we don't dereference or do pointer arithmetic on the result. Search for all occurrences of \b(m|c|re)alloc in libxc, and all functions which call them, and add appropriate error checking where missing. This includes the functions xc_dom_malloc*, which now print a message when they fail so that callers don't have to do so. The function xc_cpuid_to_str wasn't provided with a sane return value and has a pretty strange API, which now becomes a little stranger. There are no in-tree callers. Changes in the Xen 4.2 version of this series: * No need to fix code relating to ARM. * No need to fix code relating to superpage support. * Additionally fix `dom->p2m_host = xc_dom_malloc...' in xc_dom_ia64.c. Changes in the Xen 4.1 version of this series: * An additional check is needed in xc_flask.c:xc_flask_access. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_rangeIan Jackson2013-06-148-0/+113
| | | | | | | | | | | | | | | | | | | | | The return values from xc_dom_*_to_ptr and xc_map_foreign_range are sometimes dereferenced, or subjected to pointer arithmetic, without checking whether the relevant function failed and returned NULL. Add an appropriate error check at every call site. Changes in the 4.2 backport of this series: * Fix tools/libxc/xc_dom_x86.c:setup_pgtables_x86_32. * Fix tools/libxc/xc_dom_ia64.c:start_info_ia64. * Fix tools/libxc/ia64/xc_ia64_dom_fwloader.c:xc_dom_load_fw_kernel. Conflicts in the 4.1 backport of this series: * xc_dom_load_elf_kernel has less error handling in 4.1. * the VM generation ID code is not in 4.1. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libxc: Add range checking to xc_dom_binloaderIan Jackson2013-06-141-2/+13
| | | | | | | | | | | | | | | | | | | | | | | | | This is a simple binary image loader with its own metadata format. However, it is too careless with image-supplied values. Add the following checks: * That the image is bigger than the metadata table; otherwise the pointer arithmetic to calculate the metadata table location may yield undefined and dangerous values. * When clamping the end of the region to search, that we do not calculate pointers beyond the end of the image. The C specification does not permit this and compilers are becoming ever more determined to miscompile code when they can "prove" various falsehoods based on assertions from the C spec. * That the supplied image is big enough for the text we are allegedly copying from it. Otherwise we might have a read overrun and copy the results (perhaps a lot of secret data) into the guest. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: abolish obsolete macrosIan Jackson2013-06-146-65/+42
| | | | | | | | | | | | | | | | | Abolish ELF_PTRVAL_[CONST_]{CHAR,VOID}; change uses to elf_ptrval. Abolish ELF_HANDLE_DECL_NONCONST; change uses to ELF_HANDLE_DECL. Abolish ELF_OBSOLETE_VOIDP_CAST; simply remove all uses. No functional change. (Verified by diffing assembler output.) This is part of the fix to a security issue, XSA-55. Conflicts in the 4.1 backport: * elf_load_image is not in 4.1. * elf_note_numeric_array is not in 4.1. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
* libelf: check loops for running awayIan Jackson2013-06-145-29/+135
| | | | | | | | | | | | | | | | | | | | | | | | | | Ensure that libelf does not have any loops which can run away indefinitely even if the input is bogus. (Grepped for \bfor, \bwhile and \bgoto in libelf and xc_dom_*loader*.c.) Changes needed: * elf_note_next uses the note's unchecked alleged length, which might wrap round. If it does, return ELF_MAX_PTRVAL (0xfff..fff) instead, which will be beyond the end of the section and so terminate the caller's loop. * In various loops over section and program headers, check that the calculated header pointer is still within the image, and quit the loop if it isn't. We have not changed loops which might, in principle, iterate over the whole image - even if they might do so one byte at a time with a nontrivial access check function in the middle. This is part of the fix to a security issue, XSA-55. Conflicts in Xen 4.1 version of the series: * Trivial conflict due to elf_note_numeric_array not existing. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libxc: Introduce xc_bitops.hIan Jackson2013-06-141-0/+63
| | | | | | | | | | | | | | | | | Copy the one file tools/libxc/xc_bitops.h from xen.git#aa1355f9. We will need this for the next patch, which calls for a bitmap in libxc. xc_bitops.h was introduced to unify various existing sets of bitmap operations. In this patch we backport only the introduction, not the replacement of the other instances. So we introduce another instance Sorry :-/. This is part of the fix to a security issue, XSA-55. This patch is unique to the Xen 4.1 version of the XSA-55 series. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: use only unsigned integersIan Jackson2013-06-149-80/+98
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed integers have undesirable undefined behaviours on overflow. Malicious compilers can turn apparently-correct code into code with security vulnerabilities etc. So use only unsigned integers. Exceptions are booleans (which we have already changed) and error codes. We _do_ change all the chars which aren't fixed constants from our own text segment, but not the char*s. This is because it is safe to access an arbitrary byte through a char*, but not necessarily safe to convert an arbitrary value to a char. As a consequence we need to compile libelf with -Wno-pointer-sign. It is OK to change all the signed integers to unsigned because all the inequalities in libelf are in contexts where we don't "expect" negative numbers. In libelf-dominfo.c:elf_xen_parse we rename a variable "rc" to "more_notes" as it actually contains a note count derived from the input image. The "error" return value from elf_xen_parse_notes is changed from -1 to ~0U. grepping shows only one occurrence of "PRId" or "%d" or "%ld" in libelf and xc_dom_elfloader.c (a "%d" which becomes "%u"). This is part of the fix to a security issue, XSA-55. Conflicts in 4.1 series: * xc_dom_load_elf_kernel has no rc variable to change. * elf_load_image doesn't exist. For those concerned about unintentional functional changes, the following rune produces a version of the patch which is much smaller and eliminates only non-functional changes: GIT_EXTERNAL_DIFF=.../unsigned-differ git-diff <before>..<after> where <before> and <after> are git refs for the code before and after this patch, and unsigned-differ is this shell script: #!/bin/bash set -e seddery () { perl -pe 's/\b(?:elf_errorstatus|elf_negerrnoval)\b/int/g' } path="$1" in="$2" out="$5" set +e diff -pu --label "$path~" <(seddery <"$in") --label "$path" <(seddery <"$out") rc=$? set -e if [ $rc = 1 ]; then rc=0; fi exit $rc Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: use C99 bool for booleansIan Jackson2013-06-146-21/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We want to remove uses of "int" because signed integers have undesirable undefined behaviours on overflow. Malicious compilers can turn apparently-correct code into code with security vulnerabilities etc. In this patch we change all the booleans in libelf to C99 bool, from <stdbool.h>. For the one visible libelf boolean in libxc's public interface we retain the use of int to avoid changing the ABI; libxc converts it to a bool for consumption by libelf. It is OK to change all values only ever used as booleans to _Bool (bool) because conversion from any scalar type to a _Bool works the same as the boolean test in if() or ?: and is always defined (C99 6.3.1.2). But we do need to check that all these variables really are only ever used that way. (It is theoretically possible that the old code truncated some 64-bit values to 32-bit ints which might become zero depending on the value, which would mean a behavioural change in this patch, but it seems implausible that treating 0x????????00000000 as false could have been intended.) This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: Make all callers call elf_check_brokenIan Jackson2013-06-144-8/+45
| | | | | | | | | | | | | | | | | | | | | | | This arranges that if the new pointer reference error checking tripped, we actually get a message about it. In this patch these messages do not change the actual return values from the various functions: so pointer reference errors do not prevent loading. This is for fear that some existing kernels might cause the code to make these wild references, which would then break, which is not a good thing in a security patch. In xen/arch/x86/domain_build.c we have to introduce an "out" label and change all of the "return rc" beyond the relevant point into "goto out". This is part of the fix to a security issue, XSA-55. Differences in 4.1 backport: * No xen/arch/arm. * There was less error handling in xen/arch/x86/domain_build.c so less need to change it. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: Check pointer references in elf_is_elfbinaryIan Jackson2013-06-144-6/+11
| | | | | | | | | | | | | | | | | | | elf_is_elfbinary didn't take a length parameter and could potentially access out of range when provided with a very short image. We only need to check the size is enough for the actual dereference in elf_is_elfbinary; callers are just using it to check the magic number and do their own checks (usually via the new elf_ptrval system) before dereferencing other parts of the header. This is part of the fix to a security issue, XSA-55. Conflicts in 4.1 backport: * xen/arch/x86/bzimage.c in 4.1 doesn't use elf_is_elfbinary. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
* libelf: check all pointer accessesIan Jackson2013-06-148-84/+303
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We change the ELF_PTRVAL and ELF_HANDLE types and associated macros: * PTRVAL becomes a uintptr_t, for which we provide a typedef elf_ptrval. This means no arithmetic done on it can overflow so the compiler cannot do any malicious invalid pointer arithmetic "optimisations". It also means that any places where we dereference one of these pointers without using the appropriate macros or functions become a compilation error. So we can be sure that we won't miss any memory accesses. All the PTRVAL variables were previously void* or char*, so the actual address calculations are unchanged. * ELF_HANDLE becomes a union, one half of which keeps the pointer value and the other half of which is just there to record the type. The new type is not a pointer type so there can be no address calculations on it whose meaning would change. Every assignment or access has to go through one of our macros. * The distinction between const and non-const pointers and char*s and void*s in libelf goes away. This was not important (and anyway libelf tended to cast away const in various places). * The fields elf->image and elf->dest are renamed. That proves that we haven't missed any unchecked uses of these actual pointer values. * The caller may fill in elf->caller_xdest_base and _size to specify another range of memory which is safe for libelf to access, besides the input and output images. * When accesses fail due to being out of range, we mark the elf "broken". This will be checked and used for diagnostics in a following patch. We do not check for write accesses to the input image. This is because libelf actually does this in a number of places. So we simply permit that. * Each caller of libelf which used to set dest now sets dest_base and dest_size. * In xc_dom_load_elf_symtab we provide a new actual-pointer value hdr_ptr which we get from mapping the guest's kernel area and use (checking carefully) as the caller_xdest area. * The STAR(h) macro in libelf-dominfo.c now uses elf_access_unsigned. * elf-init uses the new elf_uval_3264 accessor to access the 32-bit fields, rather than an unchecked field access (ie, unchecked pointer access). * elf_uval has been reworked to use elf_uval_3264. Both of these macros are essentially new in this patch (although they are derived from the old elf_uval) and need careful review. * ELF_ADVANCE_DEST is now safe in the sense that you can use it to chop parts off the front of the dest area but if you chop more than is available, the dest area is simply set to be empty, preventing future accesses. * We introduce some #defines for memcpy, memset, memmove and strcpy: - We provide elf_memcpy_safe and elf_memset_safe which take PTRVALs and do checking on the supplied pointers. - Users inside libelf must all be changed to either elf_mem*_unchecked (which are just like mem*), or elf_mem*_safe (which take PTRVALs) and are checked. Any unchanged call sites become compilation errors. * We do _not_ at this time fix elf_access_unsigned so that it doesn't make unaligned accesses. We hope that unaligned accesses are OK on every supported architecture. But it does check the supplied pointer for validity. This is part of the fix to a security issue, XSA-55. Additional change in 4.1 backport: * ELF_PRPTRVAL needs to be defined oddly on 4.1 and earlier because Xen's headers provide no definitions of uintptr_t or PRIuPTR. Conflicts: * Callers of elf_load_binary don't check its return value in 4.1. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
* libelf: check nul-terminated strings properlyIan Jackson2013-06-144-11/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is not safe to simply take pointers into the ELF and use them as C pointers. They might not be properly nul-terminated (and the pointers might be wild). So we are going to introduce a new function elf_strval for safely getting strings. This will check that the addresses are in range and that there is a proper nul-terminated string. Of course it might discover that there isn't. In that case, it will be made to fail. This means that elf_note_name might fail, too. For the benefit of call sites which are just going to pass the value to a printf-like function, we provide elf_strfmt which returns "(invalid)" on failure rather than NULL. In this patch we introduce dummy definitions of these functions. We introduce calls to elf_strval and elf_strfmt everywhere, and update all the call sites with appropriate error checking. There is not yet any semantic change, since before this patch all the places where we introduce elf_strval dereferenced the value anyway, so it mustn't have been NULL. In future patches, when elf_strval is made able return NULL, when it does so it will mark the elf "broken" so that an appropriate diagnostic can be printed. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
* tools/xcutils/readnotes: adjust print_l1_mfn_valid_noteIan Jackson2013-06-141-5/+6
| | | | | | | | | | | | | | | | Use the new PTRVAL macros and elf_access_unsigned in print_l1_mfn_valid_note. No functional change unless the input is wrong, or we are reading a file for a different endianness. Separated out from the previous patch because this change does produce a difference in the generated code. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
* libelf: introduce macros for memory access and pointer handlingIan Jackson2013-06-146-153/+310
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We introduce a collection of macros which abstract away all the pointer arithmetic and dereferences used for accessing the input ELF and the output area(s). We use the new macros everywhere. For now, these macros are semantically identical to the code they replace, so this patch has no functional change. elf_is_elfbinary is an exception: since it doesn't take an elf*, we need to handle it differently. In a future patch we will change it to take, and check, a length parameter. For now we just mark it with a fixme. Nontrivial differences in the 4.1 backport: * We need to provide our own elf_uintptr_t since Xen doesn't. * We see some additional differences in our verification diff. * The "function-filter" needs to massage additional symbol names. Conflicts: * In xc_dom_load_elf_symtab the old code used *(Elf64_Word*)(&shdr->e64.sh_name) and the new Elf32_Word but in fact the type in the struct has changed too so the new code using elf_store_field is still correct. * loadelfimage, elf_load_image etc. don't exist and are done directly with memcpy/memset; patch adjusted appropriately. * elf_note_numeric_array doesn't exist in 4.1. That this patch has no functional change can be verified as follows: 0. Copy the scripts "comparison-generate" and "function-filter" out of this commit message. 1. Check out the tree before this patch. 2. Run the script ../comparison-generate .... ../before 3. Check out the tree after this patch. 4. Run the script ../comparison-generate .... ../after 5. diff --exclude=\*.[soi] -ruN before/ after/ |less Expect these differences: * stubdom/zlib-x86_64/ztest*.s2 The filename of this test file apparently contains the pid. * stubdom/grub/kexec.s2: Large differences following ".section .debug_info" (which the 4.1 build system erroneously fails to suppress). * tools/libxc/xc_domain_restore.s2 (64-bit build): One trivial code gen difference with no semantic import. * xen/common/version.s2 The xen build timestamp appears in two diff hunks. Verification that this is all that's needed: In a completely built xen.git, find * -name .*.d -type f | xargs grep -l libelf\.h Expect results in: xen/arch/x86: Checked above. tools/libxc: Checked above. tools/xcutils/readnotes: Checked above. tools/xenstore: Checked above. xen/common/libelf: This is the build for the hypervisor; checked in B above. stubdom: We have one stubdom which reads ELFs using our libelf, pvgrub, which is checked above. I have not done this verification for ARM. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> -8<- comparison-generate -8<- #!/bin/bash # usage: # cd xen.git # .../comparison-generate OUR-CONFIG BUILD-RUNE-PREFIX ../before|../after # eg: # .../comparison-generate ~/work/.config 'schroot -pc64 --' ../before set -ex test $# = 3 || need-exactly-three-arguments our_config=$1 build_rune_prefix=$2 result_dir=$3 git clean -x -d -f cp "$our_config" . cat <<END >>.config debug_symbols=n CFLAGS += -save-temps END perl -i~ -pe 's/ -g / -g0 / if m/^CFLAGS/' xen/Rules.mk if [ -f ./configure ]; then $build_rune_prefix ./configure fi $build_rune_prefix make -C xen $build_rune_prefix make -C tools/include $build_rune_prefix make -C stubdom grub $build_rune_prefix make -C tools/libxc $build_rune_prefix make -C tools/xenstore $build_rune_prefix make -C tools/xcutils rm -rf "$result_dir" mkdir "$result_dir" set +x for f in `find xen tools stubdom -name \*.[soi]`; do mkdir -p "$result_dir"/`dirname $f` cp $f "$result_dir"/${f} case $f in *.s) ../function-filter <$f >"$result_dir"/${f}2 ;; esac done echo ok. -8<- -8<- function-filter -8<- #!/usr/bin/perl -w # function-filter # script for massaging gcc-generated labels to be consistent use strict; our @lines; my $sedderybody = "sub seddery () {\n"; while (<>) { push @lines, $_; if (m/^(__FUNCTION__|__func__|_ctx|note_desc|types|last_order|memflags|mutex|d\d_cpu_last|write_count|wall_last|__PRETTY_FUNCTION__)\.(\d+)\:/ || m/^\s+\.local\s+(_ctx|write_count|d\d_cpu_last|wall_last|mutex)\.(\d+)\s*$/) { $sedderybody .= " s/\\b$1\\.$2\\b/__XSA55MANGLED__$1.$./g;\n"; } } $sedderybody .= "}\n1;\n"; eval $sedderybody or die $@; foreach (@lines) { seddery(); print or die $!; } -8<-
* libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialisedIan Jackson2013-06-141-1/+1
| | | | | | | | | | | | | | | | | | | xc_dom_load_elf_symtab (with load==0) calls elf_round_up, but it mistakenly used the uninitialised variable "syms" when calculating dom->bsd_symtab_start. This should be a reference to "elf". This change might have the effect of rounding the value differently. Previously if the uninitialised value (a single byte on the stack) was ELFCLASS64 (ie, 2), the alignment would be to 8 bytes, otherwise to 4. However, the value is calculated from dom->kernel_seg.vend so this could only make a difference if that value wasn't already aligned to 8 bytes. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
* libelf: abolish elf_sval and elf_access_signedIan Jackson2013-06-142-39/+0
| | | | | | | | | These are not used anywhere. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>