From `man 2 realloc`:
If realloc() fails, the original block is left untouched; it is not freed or moved.
Found using GCC -fanalyzer:
| 184 | backup->entries = realloc(backup->entries,
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | | |
| | | | (91) when ‘realloc’ fails
| | | (92) ‘old_ptes.entries’ leaks here; was allocated at (44)
| | (90) ...to here
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Edwin, with the help of GCC's -fanalyzer, identified that p2m_frame_list_list
gets leaked. What fanalyzer can't see is that the live_p2m_frame_list_list
and live_p2m_frame_list foreign mappings are leaked too.
Rework the logic so the out path is executed unconditionally, which cleans up
all the intermediate allocations/mappings appropriately.
Fixes: bd7a29c3d0 ("tools/libs/ctrl: fix xc_core_arch_map_p2m() to support linear p2m table")
Reported-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Replace PKG_CONFIG variable name with PKG_CONFIG_FILE for the name of
the pkg-config file.
This is preventing a conflict in some build systems where PKG_CONFIG
actually contains the path to the pkg-config executable to use, as the
default assignment in libs.mk is using a weak assignment (?=).
This problem has been found when trying to build the latest version of
Xen tools using buildroot.
Fixes: d400dc5729 ("tools: tweak tools/libs/libs.mk for being able to support libxenctrl")
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Clang-15 (as seen in the FreeBSD 14 tests) complains:
xg_main.c:1248 error: a function declaration without a
prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
xg_init()
^
void
The error message is a bit confusing but appears to new as part of
-Wdeprecated-non-prototype which is part of supporting C2x which formally
removes K&R syntax.
Either way, fix the identified function.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Unfortunatly, --default-symver doesn't work with LLVM's LD, LLD.
Instead, we will generate a temporary version-script.
In order to allow regenerating the script, we'll have a different
filename. In order to check if the content is up-to-date, we'll always
generated it and compare.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes: 98d95437ed ("libs: Fix auto-generation of version-script for unstable libs")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Refer to Intel SDM Rev 70 (Dec 2022), Vol3 17.2.3 "Supervisor Shadow Stack
Token".
Architecturally, an event delivery which starts in CPL<3 and switches shadow
stack will first validate the Supervisor Shadow Stack Token (setting the busy
bit), then pushes CS/LIP/SSP. One example of this is an NMI interrupting Xen.
Some CPUs suffer from an issue called fracturing, whereby a fault/vmexit/etc
between setting the busy bit and completing the event injection renders the
action non-restartable, because when it comes time to restart, the busy bit is
found to be already set.
This is far more easily encountered under virt, yet it is not the fault of the
hypervisor, nor the fault of the guest kernel. The fault lies somewhere
between the architectural specification, and the uarch behaviour.
Intel have allocated CPUID.7[1].ecx[18] CET_SSS to enumerate that supervisor
shadow stacks are safe to use. Because of how Xen lays out its shadow stacks,
fracturing is not expected to be a problem on native.
Detect this case on boot and default to not using shstk if virtualised.
Specifying `cet=shstk` on the command line will override this heuristic and
enable shadow stacks irrespective.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
We don't actually need ecx yet, but adding it in now will reduce the amount to
which leaf 7 is out of order in a featureset.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
flex/bison generate two targets, and when those targets are
prerequisite of other rules they are considered independently by make.
We can have a situation where the .c file is out-of-date but not the
.h, git checkout for example. In this case, if a rule only have the .h
file as prerequiste, make will procced and start to build the object.
In parallel, another target can have the .c file as prerequisite and
make will find out it need re-generating and do so, changing the .h at
the same time. This parallel task breaks the first one.
To avoid this scenario, we put both the header and the source as
prerequisite for all object even if they only need the header.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
libxlu_pci.c doesn't use any "xlu__cfg*()" functions and doesn't use
any of the disk parsing functions "xlu__disk*()", so all those headers
can be removed.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The parameters of hashfn/eqfn should never be modified. So constify
them and propagate the const to the users.
Take the opportunity to solve some coding style issues around the
code modified.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
When loading a Xenstore stubdom the loader doesn't know whether the
lo be loaded kernel is a PVH or a PV one. So it tries to load it as
a PVH one first, and if this fails it is loading it as a PV kernel.
This results in errors being logged in case the stubdom is a PV kernel.
Suppress those errors by setting the minimum logging level to
"critical" while trying to load the kernel as PVH.
In case PVH mode and PV mode loading fails, retry PVH mode loading
without changing the log level in order to get the error messages
logged.
Fixes: f89955449c ("tools/init-xenstore-domain: support xenstore pvh stubdom")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Rework "libacpi.h" include in "libxl_x86_acpi.c" as to be more
selective about the include path and only add "tools/libacpi/". Also
"libxl_dom.c" don't use "libacpi.h" anymore. Use "-iquote" for libacpi
headers.
Get rid of the weird "$(eval stem =" in the middle of a recipe and use
a make automatic variable "$(*F)" instead.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
No need for $(AUTOSRCS), GNU make can generate them as needed when
trying to build them as needed when trying to build the object. Also,
those two AUTOSRCS don't need to be a prerequisite of "all". As for
the "clean" target, those two files are already removed via "_*.c".
We don't need $(AUTOINCS) either:
- As for both _libxl_save_msgs*.h headers, we are adding more
selective dependencies so the headers will still be generated as
needed.
- "clean" rule already delete the _*.h files, so AUTOINCS aren't needed
there.
"libxl_internal_json.h" doesn't seems to have ever existed, so the
dependency is removed.
Rework objects prerequisites, to have them dependents on either
"libxl.h" or "libxl_internal.h". "libxl.h" is not normally included
directly in the source code as "libxl_internal.h" is used instead, but
we have "libxl.h" as prerequisite of "libxl_internal.h", so generated
headers will still be generated as needed.
Make doesn't need "libxl.h" to generate "testidl.c", "libxl.h" is only
needed later when building "testidl.o". This avoid the need to
regenerate "testidl.c" when only "libxl.h" changed. Also use automatic
variables $< and $@.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
When there isn't a version-script for a shared library (like for
unstable libs), we create one based on the current Xen version. But
that version-script became out-of-date as soon as Xen's version
changes and make as no way to regenerate the version-script on
rebuild.
For unstable libs, we only needs the symver to be different from a
previous release of Xen. There's an option "--default-symver" which
allow to use the soname as symver and as the soname have the Xen
release version, it will be different for every release. With
--default-symver we don't need to generate a version-script.
But we also need to know if there's already an existing version script
, for that we introduce $(version-script) to be used to point to the
path of the existing script. (Guessing if a version script exist for a
stable library with for example $(wildcard) won't work as a file will
exist when building the library without this patch.)
We don't need the version-script unless we are making the shared
library so it is removed from the "all" target.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes for Py3:
* class Delayed(): file not defined; also an error for pylint -E. Inherit
object instead for Py2 compatibility. Fix DomainInfo() too.
* Inconsistent use of tabs and spaces for indentation (in one block)
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
All caml_alloc_*() functions can throw exceptions, and longjump out of
context. If this happens, we leak the xch/xce handle.
Reorder the logic to allocate the the Ocaml object first.
Fixes: 8b3c06a3e5 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Fixes: 22d5affdf0 ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The modification to the _H() macro for Ocaml 5 support introduced a subtle
bug. From the manual:
https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code
"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."
Previously, the value was a naked C pointer, so dereferencing it wasn't
"accessing any Ocaml data", but the fix to avoid naked C pointers added a
layer of indirection through an Ocaml Custom object, meaning that the common
pattern of using _H() in a blocking section is unsafe.
In order to fix:
* Drop the _H() macro and replace it with a static inline xch_of_val().
* Opencode the assignment into Data_custom_val() in the constructors.
* Rename "value xch" parameters to "value xch_val" so we can consistently
have "xc_interface *xch" on the stack, and obtain the pointer with the GC
lock still held.
* Drop the _D() macro while at it, because it's just pointless indirection.
Fixes: 8b3c06a3e5 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The intf->{addr,len} references in the xc_map_foreign_range() call are unsafe.
From the manual:
https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code
"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."
More than what the manual says, the intf pointer is (potentially) invalidated
by caml_enter_blocking_section() if another thread happens to perform garbage
collection at just the right (wrong) moment.
Rewrite the logic. There's no need to stash data in the Ocaml object until
the success path at the very end.
Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The patch adding this binding was plain broken, and unreviewed. It modified
the C stub to add a 4th parameter without an equivalent adjustment in the
Ocaml side of the bindings.
In 64bit builds, this causes us to dereference whatever dead value is in %rcx
when trying to interpret the rflags parameter.
This has gone unnoticed because Xapi doesn't use this binding (it has its
own), but unbreak the binding by passing RDM_RELAXED unconditionally for
now (matching the libxl default behaviour).
Fixes: 9b34056cb4 ("tools: extend xc_assign_device() to support rdm reservation policy")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
* Remove local integers when all we're returning is Val_int() of another
variable. The CAMLlocal*() can't be optimised automatically, as it's
registered with the GC.
* Rename "virq_type" to "virq" and "_port" to "port".
* In stub_eventchn_pending(), rename 'port' to 'rc', to be consistent with
all other stubs that return xenevtchn_port_or_error_t.
* In stub_eventchn_unmask(), check for rc == -1 to be consistent with all
other stubs.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The modification to the _H() macro for Ocaml 5 support introduced a subtle
bug. From the manual:
https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code
"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."
Previously, the value was a naked C pointer, so dereferencing it wasn't
"accessing any Ocaml data", but the fix to avoid naked C pointers added a
layer of indirection through an Ocaml Custom object, meaning that the common
pattern of using _H() in a blocking section is unsafe.
In order to fix:
* Drop the _H() macro and replace it with a static inline xce_of_val().
* Opencode the assignment into Data_custom_val() in the two constructors.
* Rename "value xce" parameters to "value xce_val" so we can consistently
have "xenevtchn_handle *xce" on the stack, and obtain the pointer with the
GC lock still held.
Fixes: 22d5affdf0 ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
caml_alloc() takes units of Wsize (word size), not bytes. As a consequence,
we're allocating 4 or 8 times too much memory.
Ocaml has a helper, Wsize_bsize(), but it truncates cases which aren't an
exact multiple. Use a BUILD_BUG_ON() to cover the potential for truncation,
as there's no rounding-up form of the helper.
Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Fixes: d3e649277a ("ocaml: add mmap bindings implementation.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
There is no such thing as an Ocaml function (C stub or otherwise) taking no
parameters. In the absence of any other parameters, unit is still passed.
This doesn't explode with any ABI we care about, but would malfunction for an
ABI environment such as stdcall.
Fixes: c3afd398ba ("ocaml: Add XS bindings.")
Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The configuration file can contain typos or various errors that could prevent
live update from succeeding (e.g. a flag only valid on a different version).
Unknown entries in the config file would be ignored on startup normally,
add a strict --config-test that live-update can use to check that the config file
is valid *for the new binary*.
For compatibility with running old code during live update recognize
--live --help as an equivalent to --config-test.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Using `ocp-indent` for now to just make minimal modifications in
tabs vs spaces and get the right indentation.
This avoids perpetuating a formatting style that is inconsistent with
the rest of Xen, and that makes preparing and submitting patches more
difficult (OCaml indentation tools usually only support spaces, not tabs).
No functional change.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Python < 3.10 by default uses 'int' type for data+size string types
(s#), unless PY_SSIZE_T_CLEAN is defined - in which case it uses
Py_ssize_t. The former behavior was removed in Python 3.10 and now it's
required to define PY_SSIZE_T_CLEAN before including Python.h, and using
Py_ssize_t for the length argument. The PY_SSIZE_T_CLEAN behavior is
supported since Python 2.5.
Adjust bindings accordingly.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
When a domain performs a kexec (soft reset), libxl__build_pre() is
called with the existing domid. Calling libxl__cpuid_legacy() on the
existing domain fails since the cpuid policy has already been set, and
the guest isn't rebuilt and doesn't kexec.
xc: error: Failed to set d1's policy (err leaf 0xffffffff, subleaf 0xffffffff, msr 0xffffffff) (17 = File exists): Internal error
libxl: error: libxl_cpuid.c:494:libxl__cpuid_legacy: Domain 1:Failed to apply CPUID policy: File exists
libxl: error: libxl_create.c:1641:domcreate_rebuild_done: Domain 1:cannot (re-)build domain: -3
libxl: error: libxl_xshelp.c:201:libxl__xs_read_mandatory: xenstore read failed: `/libxl/1/type': No such file or directory
libxl: warning: libxl_dom.c:49:libxl__domain_type: unable to get domain type for domid=1, assuming HVM
During a soft_reset, skip calling libxl__cpuid_legacy() to avoid the
issue. Before commit 34990446ca, the libxl__cpuid_legacy() failure
would have been ignored, so kexec would continue.
Fixes: 34990446ca ("libxl: don't ignore the return value from xc_cpuid_apply_policy")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
I'm observing guest kexec trigger xenstored to abort on a double free.
gdb output:
Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140645614258112) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
at ./nptl/pthread_kill.c:44
at ./nptl/pthread_kill.c:78
at ./nptl/pthread_kill.c:89
at ../sysdeps/posix/raise.c:26
at talloc.c:119
ptr=ptr@entry=0x559fae724290) at talloc.c:232
at xenstored_core.c:2945
(gdb) frame 5
at talloc.c:119
119 TALLOC_ABORT("Bad talloc magic value - double free");
(gdb) frame 7
at xenstored_core.c:2945
2945 talloc_increase_ref_count(conn);
(gdb) p conn
$1 = (struct connection *) 0x559fae724290
Looking at a xenstore trace, we have:
IN 0x559fae71f250 20230120 17:40:53 READ (/local/domain/3/image/device-model-dom
id )
wrl: dom 0 1 msec 10000 credit 1000000 reserve 100 disc
ard
wrl: dom 3 1 msec 10000 credit 1000000 reserve 100 disc
ard
wrl: dom 0 0 msec 10000 credit 1000000 reserve 0 disc
ard
wrl: dom 3 0 msec 10000 credit 1000000 reserve 0 disc
ard
OUT 0x559fae71f250 20230120 17:40:53 ERROR (ENOENT )
wrl: dom 0 1 msec 10000 credit 1000000 reserve 100 disc
ard
wrl: dom 3 1 msec 10000 credit 1000000 reserve 100 disc
ard
IN 0x559fae71f250 20230120 17:40:53 RELEASE (3 )
DESTROY watch 0x559fae73f630
DESTROY watch 0x559fae75ddf0
DESTROY watch 0x559fae75ec30
DESTROY watch 0x559fae75ea60
DESTROY watch 0x559fae732c00
DESTROY watch 0x559fae72cea0
DESTROY watch 0x559fae728fc0
DESTROY watch 0x559fae729570
DESTROY connection 0x559fae724290
orphaned node /local/domain/3/device/suspend/event-channel deleted
orphaned node /local/domain/3/device/vbd/51712 deleted
orphaned node /local/domain/3/device/vkbd/0 deleted
orphaned node /local/domain/3/device/vif/0 deleted
orphaned node /local/domain/3/control/shutdown deleted
orphaned node /local/domain/3/control/feature-poweroff deleted
orphaned node /local/domain/3/control/feature-reboot deleted
orphaned node /local/domain/3/control/feature-suspend deleted
orphaned node /local/domain/3/control/feature-s3 deleted
orphaned node /local/domain/3/control/feature-s4 deleted
orphaned node /local/domain/3/control/sysrq deleted
orphaned node /local/domain/3/data deleted
orphaned node /local/domain/3/drivers deleted
orphaned node /local/domain/3/feature deleted
orphaned node /local/domain/3/attr deleted
orphaned node /local/domain/3/error deleted
orphaned node /local/domain/3/console/backend-id deleted
and no further output.
The trace shows that DESTROY was called for connection 0x559fae724290,
but that is the same pointer (conn) main() was looping through from
connections. So it wasn't actually removed from the connections list?
Reverting commit e8e6e42279 "tools/xenstore: simplify loop handling
connection I/O" fixes the abort/double free. I think the use of
list_for_each_entry_safe is incorrect. list_for_each_entry_safe makes
traversal safe for deleting the current iterator, but RELEASE/do_release
will delete some other entry in the connections list. I think the
observed abort is because list_for_each_entry has next pointing to the
deleted connection, and it is used in the subsequent iteration.
Add a comment explaining the unsuitability of list_for_each_entry_safe.
Also notice that the old code takes a reference on next which would
prevents a use-after-free.
This reverts commit e8e6e42279.
This is XSA-425/CVE-2022-42330.
Fixes: e8e6e42279 ("tools/xenstore: simplify loop handling connection I/O")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
WRMSR Non-Serialising is an optimisation intended for cases where an MSR needs
updating, but architectural serialising properties are not needed.
In is anticipated that this will apply to most if not all MSRs modified on
context switch paths.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Protection Key Supervisor works in a very similar way to Protection Key User,
except that instead of a PKRU register used by the {RD,WR}PKRU instructions,
the supervisor protection settings live in MSR_PKRS and is accessed using
normal {RD,WR}MSR instructions.
PKS has the same problematic interactions with PV guests as PKU (more infact,
given the guest kernel's CPL), so we'll only support this for HVM guests for
now.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Using a tab for separating the command from the options in the output
of "xenstore-control help" results in a rather ugly list.
Use a fixed size for the command instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today check_store() is only testing the correctness of the node tree.
Add verification of the accounting data (number of nodes) and correct
the data if it is wrong.
Do the initial check_store() call only after Xenstore entries of a
live update have been read. This is wanted to make sure the accounting
data is correct after a live update.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Make the xenstored internal trace configurable by adding classes
which can be switched on and off independently from each other.
Define the following classes:
- obj: Creation and deletion of interesting "objects" (watch,
transaction, connection)
- io: incoming requests and outgoing responses
- wrl: write limiting
Per default "obj" and "io" are switched on.
Entries written via trace() will always be printed (if tracing is on
at all).
Add the capability to control the trace settings via the "log"
command and via a new "--log-control" command line option.
Add a missing trace_create() call for creating a transaction.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of using malloc() and friends, let the hashtable implementation
use the talloc framework.
This is more consistent with the rest of xenstored and it allows to
track memory usage via "xenstore-control memreport".
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Letting hashtable_remove() return the value of the removed element is
not used anywhere in Xenstore, and it conflicts with a hashtable
created specifying the HASHTABLE_FREE_VALUE flag.
So just drop returning the value.
This of course requires to free the value if the HASHTABLE_FREE_VALUE
was specified, as otherwise it would be a memory leak.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of returning 0 or 1 let chk_domain_generation() return a
boolean value.
Simplify the only caller by removing the ret variable.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
clang 14 is complaining about a NULL dereference for constructs like:
domain_is_unprivileged(conn) ? conn->in : NULL
as it can't know that domain_is_unprivileged(conn) will return false
if conn is NULL.
Fix that by making domain_is_unprivileged() an inline function (and
related to that domid_is_unprivileged(), too).
In order not having to make struct domain public, use conn->id instead
of conn->domain->domid for the test.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There are some places left where dom0 is associated with domid 0.
Use dom0_domid instead.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Rework the interface and the internals of the per-domain node
accounting:
- rename the functions to domain_nbentry_*() in order to better match
the related counter name
- switch from node pointer to domid as interface, as all nodes have the
owner filled in
- use a common internal function for adding a value to the counter
For the transaction case add a helper function to get the list head
of the per-transaction changed domains, enabling to eliminate the
transaction_entry_*() functions.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Move all code related to struct changed_domain from
xenstored_transaction.c to xenstored_domain.c.
This will be needed later in order to simplify the accounting data
updates in cases of errors during a request.
Split the code to have a more generic base framework.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of storing a pointer to the path which is prepended to
relative paths in struct watch, just use the length of the prepended
path.
It should be noted that the now removed special case of the
relative path being "" in get_watch_path() can't happen at all.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Instead of special casing the permission handling and watch event
firing for the special watch paths "@introduceDomain" and
"@releaseDomain", use static dummy nodes added to the data base when
starting Xenstore.
The node accounting needs to reflect that change by adding the special
nodes in the domain_entry_fix() call in setup_structure().
Note that this requires to rework the calls of fire_watches() for the
special events in order to avoid leaking memory.
Move the check for a valid node name from get_node() to
get_node_canonicalized(), as it allows to use get_node() for the
special nodes, too.
In order to avoid read and write accesses to the special nodes use a
special variant for obtaining the current node data for the permission
handling.
This allows to simplify quite some code. In future sub-nodes of the
special nodes will be possible due to this change, allowing more fine
grained permission control of special events for specific domains.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Move the definition of the log() macro to xenstored_core.h in order
to make it usable from other source files, too.
While at it preserve errno from being modified.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today finding a struct domain by its domain id requires to scan the
list of domains until finding the correct domid.
Add a hashlist for being able to speed this up. This allows to remove
the linking of struct domain in a list. Note that the list of changed
domains per transaction is kept as a list, as there are no known use
cases with more than 4 domains being touched in a single transaction
(this would be a device handled by a driver domain and being assigned
to a HVM domain with device model in a stubdom, plus the control
domain).
Some simple performance tests comparing the scanning and hashlist have
shown that the hashlist will win as soon as more than 6 entries need
to be scanned.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
When a domain has been released by Xen tools, remove all its
registered watches. This avoids sending watch events to the dead domain
when all the nodes related to it are being removed by the Xen tools.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Today talloc_free() is not guaranteed to preserve errno, especially in
case a custom destructor is being used.
So preserve errno in talloc_free().
This allows to remove some errno saving outside of talloc.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The configure option "--enable-trace-backend" isn't accepted anymore
and we should use "--enable-trace-backends" instead which was
introduce in 2014 and allow multiple backends.
"--enable-trace-backends" was introduced by:
5b808275f3bb ("trace: Multi-backend tracing")
The backward compatible option "--enable-trace-backend" is removed by
10229ec3b0ff ("configure: remove backwards-compatibility and obsolete options")
As we already use ./configure options that wouldn't be accepted by
older version of QEMU's configure, we will simply use the new spelling
for the option and avoid trying to detect which spelling to use.
We already make use if "--firmwarepath=" which was introduced by
3d5eecab4a5a ("Add --firmwarepath to configure")
which already include the new spelling for "--enable-trace-backends".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Edwin Török