The R11.1 safe cast only matched void noreturn (*)(void *). Accept a
conversion where noreturn is omitted and the signature is otherwise
compatible. canonical() covers typeof destinations, and
compatible_deep_unqualified keeps the parameter and return types aligned.
Discarding noreturn does not change the calling convention. Code after
the call is unreachable when every possible callee is noreturn. Leaving
that code in place is a tolerated Rule 2.1 violation. The full rationale
is in docs/misra/deviations.rst.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Nicola Vetrini <nicola.vetrini@bugseng.com> # automation/eclair_analysis/.*
Acked-by: Jan Beulich <jbeulich@suse.com>
The hide pattern .xen-syms.[013] leaves pass 2 visible. On x86
LAST_LINKING_PASS is 2, so that is the final link. On Arm
LAST_LINKING_PASS is 3 and the final image is .xen-syms.3. Pass 2 is
usually only a symlink, so Eclair saw no program link and reported every
translation unit as unreachable.
During analysis preparation, read LAST_LINKING_PASS from the architecture
makefile and the pass numbers from the foreach list in
xen/scripts/Makefile.link. Hide every pass except that one. An earlier
pass that is a real link stays hidden. The generator runs from
generate_ecl.sh, together with the other ECLAIR configuration scripts.
Fixes: a401b5c9dc ("Eclair: re-enable scanning of the (almost) final linking step")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 10.1 states: "Operands shall not be of an inappropriate
essential type".
bar_write() and guest_mem_bar_write() added the boolean 'hi' to a
pointer difference when printing the ignored BAR index. These handlers
are registered only for the memory BAR registers, so print
(reg - PCI_BASE_ADDRESS_0) / 4 instead. The expression has type
unsigned int, so use %u.
No functional changes.
Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger@xenproject.org>
find_text_region() tests whether an arbitrary address lies inside
[text_start, text_end). The address and those bounds are not known to
point into one object, so relational operators on the pointers are
undefined behaviour and violate Misra C:2012 rule 18.3.
The bounds are cast to unsigned long because addr already has that
type. The type is wide enough to hold a pointer, so the comparison
result stays the same.
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 5.5 states: "Identifiers shall be distinct from macro
names".
Update ECLAIR configuration to deviate identifier and macro name
clashes by specifying the macros that should be ignored.
This includes hypfs_alloc_dyndata(), where the real function takes
an allocation size and the same-named macro is a typed convenience
wrapper that expands calls using sizeof(type).
Update deviations.rst and rules.rst accordingly.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
A malformed partial DTB specifying both '#address-cells = <0>' and
'#size-cells = <0>' causes '(address_cells * 2 + size_cells)' to
evaluate to 0. This sum is subsequently used as a divisor when
calculating the number of regions in the 'xen,reg' property inside
handle_passthrough_prop():
len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
sizeof(uint32_t));
This leads to a division by zero exception in the Xen hypervisor during
boot, causing a hypervisor panic/crash.
Fix this by validating that both 'address_cells' and 'size_cells'
are within the range of [1, 2] at the top of handle_passthrough_prop().
Any invalid cell size combination is safely rejected early with an error
message and return -EINVAL.
Furthermore, update handle_passthrough_prop() to use the sizeof(*cell)
instead of sizeof(uint32_t).
Fixes: 9ce974c475 ("xen/arm: assign devices to boot domains")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
When a node's depth exceeds DEVICE_TREE_MAX_DEPTH inside the
device_tree_for_each_node() loop, the code prints a warning and
executes 'continue;' statement, which jumps to condition check,
bypassing the iterator update step:
node = fdt_next_node(fdt, node, &depth).
The node and depth are not updated, the loop repeatedly evaluates
the same too-deep node, causing a hang.
Fix this by wrapping the node processing logic in an 'else' block.
This ensures the loop update step is executed on every iteration,
safely skipping deeply nested nodes and doing the traversal.
Fixes: 40f2ea3df2 ("xen/arm: pass node to device_tree_for_each_node")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Release-Acked-by: Oleskii Kurochko <oleksii.kurochko@gmail.com>
Convert 'free' macro in 'decompress.h' from object-like to function-like
form.
The object-like macro '#define free xfree' performs unconditional text
replacement, causing conflicts with identifiers named 'free', such as
struct fields in 'page_info' unions defined in 'xen/arch/arm/include/asm/mm.h'.
Function-like macros only match when followed by parentheses, allowing
'free' to be used both as a macro and as a struct field without conflicts.
Applying function-like form to 'malloc', 'large_malloc' and 'large_free'
ensures consistent macro style.
Additionally moved 'large_{malloc,free}' past the #endif to reduce
redundancy.
While function-like macros prevent uses where the underlying function
identifier is needed directly, such as taking a function pointer, no
such uses exist in the current Xen codebase.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
The statement 'return ERR_PTR(-ENOENT);' on the final line of the
function 'hypfs_get_entry_rel()' is unreachable because the logic within
the infinite loop 'for (;;)' provides all possible exit paths for the
function. So there is no execution path to exit the loop and reach the
final that statement.
This unreachable code violates MISRA C Rule 2.1 which states: "A project
shall not contain unreachable code".
To fix that and potential compilers "control reaches end of non-void
function" warning, mark the code path as unreachable using macro
'ASSERT_UNREACHABLE()'.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
When booting Xen with CONFIG_USBAN=y and CONFIG_XSM_FLASK=y,
UBSAN reports undefined behaviour in avc_dump_av() due to a left
shift on a signed int:
(XEN) [ 1.104348] ================================================================================
(XEN) [ 1.105096] UBSAN: Undefined behaviour in xsm/flask/avc.c:184:14
(XEN) [ 1.106052] left shift of 1073741824 by 1 places cannot be represented in type 'int'
(XEN) [ 1.107546] Xen WARN at common/ubsan/ubsan.c:176
(XEN) [ 1.108295] ----[ Xen-4.21.1 arm64 debug=y ubsan=y Not tainted ]----
(XEN) [ 1.108848] CPU: 0
(XEN) [ 1.109147] PC: 00000a00002f64fc ubsan.c#ubsan_epilogue+0x10/0xd4
[...]
(XEN) [ 1.146320] Xen call trace:
(XEN) [ 1.146663] [<00000a00002f64fc>] ubsan.c#ubsan_epilogue+0x10/0xd4 (PC)
(XEN) [ 1.147227] [<00000a00002f7bc4>] __ubsan_handle_shift_out_of_bounds+0x1a0/0x290 (LR)
(XEN) [ 1.147868]
(XEN) [ 1.148177] ================================================================================
This can be solved by making 'perm' an unsigned 32-bit type (u32).
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Clang 22.x.x or Arm Toolchain for Embedded 22.x.x reports:
In file included from arch/arm/tee/ffa.c:75:
arch/arm/tee/ffa_private.h:439:17: error: 'used' attribute ignored on a non-definition declaration [-Werror,-Wignored-attributes]
439 | extern uint32_t __ro_after_init ffa_fw_version;
| ^
There is a fix for clang 16 in the git history:
a1a1e2b7dc ("xen/arm: ffa: fix build with clang")
which was broken by the following commit:
a90376c6ad ("xen/arm: ffa: Add FF-A 1.2 endpoint memory access descriptors")
Remove __ro_after_init from the extern declaration of ffa_fw_version.
The attribute is already present on the definition in ffa.c, which is
sufficient and correct.
Fixes: a90376c6ad ("xen/arm: ffa: Add FF-A 1.2 endpoint memory access descriptors")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
MISRA C:2012 Rule 5.3 states as following: An identifier declared in an
inner scope shall not hide an identifier declared in an outer scope.
Inside the 'if' block, the inner 'i' shadows the outer 'i'.
Reuse the outer variable and remove the inner declaration to fix that.
No functional change.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 20.12 states: "A macro parameter used as an operand to
the # or ## operators, which is itself subject to further macro replacement,
shall only be used as an operand to these operators".
In this case the 'FFA_*' macros are used both as a regular macro argument and
as an operand for '#' stringification in the expansion of 'FW_ABI()'.
This is deviated using a SAF-x-safe comment.
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Rule 11.1 states as following: "Conversions shall not be performed
between a pointer to a function and any other type."
This deviation from Rule 11.1 relies on both ABI definitions and compiler
implementations supported by Xen. The System V x86_64 ABI and the AArch64
ELF ABI define consistent and compatible representations (i.e., having
the same size and memory layout) for (void *), unsigned long, and function
pointers, enabling safe conversions between these types without data loss
or corruption. Additionally, GCC and Clang, faithfully implement the ABI
specifications, ensuring that the generated machine code conforms to these
guarantees. Developers must note that this behavior is not universal and
depends on platform-specific ABIs and compiler implementations.
Configure Eclair to avoid reporting violations for conversions from
unsigned long or (void *) to a function pointer.
Add a compile-time assertion into the file 'xen/common/version.c' to
confirm this conversion compatibility across X86 and ARM platforms
(assuming this file is common for them).
References:
- System V x86_64 ABI: https://gitlab.com/x86-psABIs/x86-64-ABI/-/jobs/artifacts/master/raw/x86-64-ABI/abi.pdf?job=build
- AArch64 ELF ABI: https://github.com/ARM-software/abi-aa/releases
- GCC: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
- Clang: https://clang.llvm.org/docs/CrossCompilation.html
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
[stefano: remove special powerpc check]
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
In certain build configurations the following functions 'prepare_acpi()'
and 'gicv3_its_setup_collection()' are defined as inline functions and
contain the macro 'BUG()'. This resulted in violations due to these
functions became non-returning.
To ensure compliance with MISRA C Rule 2.1 remove inline function
implementations and their 'BUG()'-based unreachable code. Provide
unconditional function declarations for 'gicv3_its_setup_collection()'
and 'prepare_acpi()'. Rely on the compiler's DCE to remove unused function
calls in builds where these configs 'CONFIG_ACPI' or 'CONFIG_HAS_ITS' are
not enabled.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Rule 11.3 prohibits casting between pointer-to-object types that
are different types. The original code used a macro 'NextMemoryDescriptor()'
that performed a non-compliant cast from 'UINT8*' to 'EFI_MEMORY_DESCRIPTOR*'.
Change 'efi_process_memory_map_bootinfo()' function parameter from
'EFI_MEMORY_DESCRIPTOR *' to 'void *' for the memory map parameter. Replace
usage of 'NextMemoryDescriptor()' macro with compliant pointer arithmetic
on 'void *', followed by a single compliant cast to 'EFI_MEMORY_DESCRIPTOR *'.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The function 'check_partial_fdt()' performs an explicit check for the FDT
magic number (FDT_MAGIC) before calling 'fdt_check_header(). This check is
redundant, because 'fdt_check_header()' includes checking the magic number.
Remove the redundant check to simplify the code and rely on the library
function to perform header validation.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The current implementation of 'free_init_memory()' fills the freed '__init'
and '__initdata' memory regions with a faulting instruction before unmapping
and returning them to the heap. However, after unmapping, any access to this
region will result in a page fault, making the instruction fill redundant.
Replace the instruction fill with a call to 'memset()', zeroing the entire
region before it is unmapped and returned to the allocator.
Additionally, this change resolves a violation of MISRA C:2012 Rule 11.3
(cast between pointer to object type and pointer to a different object type),
caused by performing a cast from a 'char *' to a 'uint32_t *' pointer to
write instructions directly into memory, which is not compliant with MISRA
guidelines.
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Remove the redundant local 'DTB_MAGIC' definition and replace its usage
with the canonical 'FDT_MAGIC' from 'libfdt'.
Amends: 66edeb2e39 (arm: copy DTB appended to zImage)
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
These files 'docs/misc/kconfig{,-language}.txt' were deleted, but
references are still present in Xen. Remove them to clean-up.
Fixes: 044503f61c ("docs: Delete kconfig docs to fix licensing violation")
Fixes: f80fe2b34f ("xen: Update Kconfig to Linux v5.4")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Fix an issue in the 'fail:' cleanup path of the 'assign_shared_memory()'
function where the use of an unsigned long 'i' with the condition
'--i >= 0' caused an infinite loop. Update the loop to use 'i--',
ensuring correct loop termination.
This change adheres to MISRA C Rule 14.3: "Controlling expressions shall
not be invariant."
Fixes: 041957bad3 ("xen/arm: Add additional reference to owner domain when the owner is allocated")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>
Fix the following issues:
1. xen/docs/misra/deviations.rst:90: WARNING: Inline interpreted text or
phrase reference start-string without end-string. [docutils]
2. xen/docs/misra/deviations.rst:54: ERROR: Error parsing content block
for the "list-table" directive: uniform two-level bullet list expected,
but row 6 does not contain the same number of items as row 1 (2 vs 3).
* - R2.1
- Calls to the `__builtin_unreachable()` function inside the expansion of
the `ASSERT_UNREACHABLE()` macro may cause a function to be marked as
non-returning. This behavior occurs only in configurations where
assertions are enabled. To address this, the `noreturn` property for
`__builtin_unreachable()` is overridden in these contexts, resulting in
the absence of reports that do not have an impact on safety, despite
being true positives.
Xen expects developers to ensure code remains safe and reliable in builds,
even when debug-only assertions like `ASSERT_UNREACHABLE() are removed.
3. xen/docs/misra/rules.rst:127: WARNING: Inline interpreted text or phrase
reference start-string without end-string. [docutils]
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
MISRA C Rule 2.1 states: "A project shall not contain unreachable code."
The final return statement in the 'read_file()' function is unreachable
due to function 'PrintErrMesg()' which has 'noreturn' attribute:
PrintErrMesg(name, ret);
/* not reached */
return false;
}
No explicit return statement is needed here because 'PrintErrMesg()' is
marked as 'noreturn', which guarantees that it never returns control to
the caller. If the 'noreturn' attribute of 'PrintErrMesg()' is removed
in the future, compiler will emit an error about the missing return
statement (build-time safeguard).
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
MISRA C Rule 5.5 states: "Identifiers shall be distinct from macro names".
Update ECLAIR configuration to deviate clashes: specify the macros that
should be ignored. Update deviations.rst and rules.rst accordingly.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The __div0() function never returns to its caller, causing a violation
of MISRA C Rule 2.1: "A project shall not contain unreachable code".
Add the `noreturn` attribute to fix this.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The conversion from a function pointer with the 'noreturn' attribute
('void noreturn (*)(...)') to a function pointer type ('void (*)(...)'
causes type incompatibility according to MISRA C Rule 11.1, which
forbids conversions between incompatible function pointer types.
The violation occurs at the call site:
smp_call_function(halt_this_cpu, NULL, 0);
where 'halt_this_cpu' with type 'void noreturn (*)(void *)' is passed to
'smp_call_function' expecting function pointer of type 'void (*)(void *)'.
The 'noreturn' attribute does not change the function calling convention
or parameter handling at runtime, making the conversion safe.
For now, configure Eclair to just treat implicit conversions that lose
the "noreturn" attribute on a function 'void (*)(void*)' as safe.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com> # docs
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com> # ECLAIR
These functions never return to their caller,
causing a violation of MISRA C Rule 2.1:
"A project shall not contain unreachable code".
Add the `noreturn' attribute to fix this.
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> # EFI
Add MISRA C rules to the monitored set.
All these rules are 'accepted' for XEN, have zero or few violations,
and should be enabled for the ECLAIR scan.
Rule 17.2 is enabled to detect accidental direct recursion.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Rule 10.1: Operands shall not be of an
inappropriate essential type
The following are non-compliant:
- boolean used as a numeric value.
The result of the '__isleap' macro is a boolean.
Suppress analyser tool finding.
The result of 'NOW() > timeout' is a boolean,
which is compared to a numeric value. Fix this.
Regression was introdiced by commit:
be7f047e08 (xen/arm: smmuv3: Replace linux functions with xen functions.)
Remove pointless cast '(const unsigned short int *)'.
The source array and the destination pointer are both of the same type.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".
Reports for service MC3A2.R5.5:
xen/include/xen/fdt-domain-build.h: non-compliant parameter 'copy_to_guest'
xen/include/xen/guest_access.h: non-compliant macro 'copy_to_guest'
Rename 'copy_to_guest' function parameter to 'cb' for compliance.
No functional changes.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".
Reports for service MC3A2.R5.5:
xen/drivers/passthrough/iommu.c: non-compliant macro 'iommu_quarantine'
xen/include/xen/iommu.h: non-compliant variable 'iommu_quarantine'
There is a clash between function name and macro.
Put the 'extern' declaration for 'iommu_quarantine'
under the same preprocessor condition (#ifdef CONFIG_HAS_PCI).
This ensures that the declaration is consistent
and only exposed when CONFIG_HAS_PCI is defined.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
These external variables ('iommu_pt_cleanup_lock'
and 'iommu_pt_cleanup_list') are no longer used
in the codebase. Remove them.
Fixes: b5622eb627 (iommu: remove unused iommu_ops method and tasklet, 2020-09-22)
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Current script shows message:
Don't forget to add the subject and message to ...
Then perform:
git send-email -to xen-devel@lists.xenproject.org ...
which has wrong option '-to'.
It may be confused for user.
Set double dashes for longer options to avoid that.
Fixes: e1f912cbf7 ("scripts/add_maintainers.pl: New script")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Dmytro Prokopchuk