From patchwork Fri Jul 14 11:49:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Vetrini X-Patchwork-Id: 13313615 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 08FFAEB64DC for ; Fri, 14 Jul 2023 11:50:03 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.563583.880876 (Exim 4.92) (envelope-from ) id 1qKHJ4-0005Xi-6F; Fri, 14 Jul 2023 11:49:54 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 563583.880876; Fri, 14 Jul 2023 11:49:54 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ4-0005Xa-3Q; Fri, 14 Jul 2023 11:49:54 +0000 Received: by outflank-mailman (input) for mailman id 563583; Fri, 14 Jul 2023 11:49:52 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ2-0005Gy-OM for xen-devel@lists.xenproject.org; Fri, 14 Jul 2023 11:49:52 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 89fcd3e5-223c-11ee-8611-37d641c3527e; Fri, 14 Jul 2023 13:49:50 +0200 (CEST) Received: from nico.bugseng.com (unknown [37.163.94.163]) by support.bugseng.com (Postfix) with ESMTPSA id A002C4EE0C89; Fri, 14 Jul 2023 13:49:48 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 89fcd3e5-223c-11ee-8611-37d641c3527e From: Nicola Vetrini To: xen-devel@lists.xenproject.org Cc: sstabellini@kernel.org, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, Nicola Vetrini , Andrew Cooper , George Dunlap , Jan Beulich , Julien Grall , Wei Liu , Bertrand Marquis , Volodymyr Babchuk Subject: [RFC PATCH 1/4] xen/arm: justify or initialize conditionally uninitialized variables Date: Fri, 14 Jul 2023 13:49:18 +0200 Message-Id: <1ad20473a031eca75db4007bdc169616b512ef44.1689329728.git.nicola.vetrini@bugseng.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 This patch aims to fix some occurrences of possibly uninitialized variables, that may be read before being written. This behaviour would violate MISRA C:2012 Rule 9.1, besides being generally undesirable. In all the analyzed cases, such accesses were actually safe, but it's quite difficult to prove so by automatic checking, therefore a safer route is to change the code so as to avoid the behaviour from occurring, while preserving the semantics. To achieve this goal, I adopted the following strategies: - Add a suitably formatted local deviation comment (as indicated in 'docs/misra/documenting-violations.rst') to exempt the following line from checking. - Provide an initialization for the variable at the declaration. - Substitute a goto breaking out of control flow logic with a semantically equivalent do { .. } while(0). Signed-off-by: Nicola Vetrini --- docs/misra/safe.json | 8 +++++++ xen/arch/arm/arm64/lib/find_next_bit.c | 1 + xen/arch/arm/bootfdt.c | 6 +++++ xen/arch/arm/decode.c | 2 ++ xen/arch/arm/domain_build.c | 29 ++++++++++++++++++---- xen/arch/arm/efi/efi-boot.h | 6 +++-- xen/arch/arm/gic-v3-its.c | 9 ++++--- xen/arch/arm/mm.c | 1 + xen/arch/arm/p2m.c | 33 +++++++++++++++----------- 9 files changed, 69 insertions(+), 26 deletions(-) diff --git a/docs/misra/safe.json b/docs/misra/safe.json index e3c8a1d8eb..244001f5be 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -12,6 +12,14 @@ }, { "id": "SAF-1-safe", + "analyser": { + "eclair": "MC3R1.R9.1" + }, + "name": "Rule 9.1: initializer not needed", + "text": "The following local variables are possibly subject to being read before being written, but code inspection ensured that the control flow in the construct where they appear ensures that no such event may happen." + }, + { + "id": "SAF-2-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/arch/arm/arm64/lib/find_next_bit.c b/xen/arch/arm/arm64/lib/find_next_bit.c index ca6f82277e..51b852c595 100644 --- a/xen/arch/arm/arm64/lib/find_next_bit.c +++ b/xen/arch/arm/arm64/lib/find_next_bit.c @@ -67,6 +67,7 @@ unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size, { const unsigned long *p = addr + BIT_WORD(offset); unsigned long result = offset & ~(BITS_PER_LONG-1); + /* SAF-1-safe MC3R1.R9.1 */ unsigned long tmp; if (offset >= size) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 2673ad17a1..1292a64e8d 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -34,6 +34,7 @@ static bool __init device_tree_node_matches(const void *fdt, int node, static bool __init device_tree_node_compatible(const void *fdt, int node, const char *match) { + /* SAF-1-safe MC3R1.R9.1 */ int len, l; const void *prop; @@ -169,7 +170,9 @@ int __init device_tree_for_each_node(const void *fdt, int node, */ int depth = 0; const int first_node = node; + /* SAF-1-safe MC3R1.R9.1 */ u32 address_cells[DEVICE_TREE_MAX_DEPTH]; + /* SAF-1-safe MC3R1.R9.1 */ u32 size_cells[DEVICE_TREE_MAX_DEPTH]; int ret; @@ -249,8 +252,10 @@ static void __init process_multiboot_node(const void *fdt, int node, const __be32 *cell; bootmodule_kind kind; paddr_t start, size; + /* SAF-1-safe MC3R1.R9.1 */ int len; /* sizeof("/chosen/") + DT_MAX_NAME + '/' + DT_MAX_NAME + '/0' => 92 */ + /* SAF-1-safe MC3R1.R9.1*/ char path[92]; int parent_node, ret; bool domU; @@ -326,6 +331,7 @@ static int __init process_chosen_node(const void *fdt, int node, { const struct fdt_property *prop; paddr_t start, end; + /* SAF-1-safe MC3R1.R9.1 */ int len; if ( fdt_get_property(fdt, node, "xen,static-heap", NULL) ) diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c index 2537dbebc1..b3ea504356 100644 --- a/xen/arch/arm/decode.c +++ b/xen/arch/arm/decode.c @@ -27,6 +27,7 @@ static void update_dabt(struct hsr_dabt *dabt, int reg, static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t hw2; uint16_t rt; @@ -151,6 +152,7 @@ static int decode_arm64(register_t pc, mmio_info_t *info) static int decode_thumb(register_t pc, struct hsr_dabt *dabt) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t instr; if ( raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr)) ) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d0d6be922d..d43f86c2f0 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -62,7 +62,7 @@ custom_param("dom0_mem", parse_dom0_mem); int __init parse_arch_dom0_param(const char *s, const char *e) { - long long val; + long long val = LLONG_MAX; if ( !parse_signed_integer("sve", s, e, &val) ) { @@ -1077,6 +1077,7 @@ static void __init assign_static_memory_11(struct domain *d, static int __init handle_linux_pci_domain(struct kernel_info *kinfo, const struct dt_device_node *node) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t segment; int res; @@ -1351,6 +1352,7 @@ static int __init make_memory_node(const struct domain *d, unsigned int i; int res, reg_size = addrcells + sizecells; int nr_cells = 0; + /* SAF-1-safe MC3R1.R9.1*/ __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */]; __be32 *cells; @@ -1578,6 +1580,7 @@ static int __init find_unallocated_memory(const struct kernel_info *kinfo, struct rangeset *unalloc_mem; paddr_t start, end; unsigned int i; + /* SAF-1-safe MC3R1.R9.1 */ int res; dt_dprintk("Find unallocated memory for extended regions\n"); @@ -1727,6 +1730,7 @@ static int __init find_memory_holes(const struct kernel_info *kinfo, dt_for_each_device_node( dt_host, np ) { unsigned int naddr; + /* SAF-1-safe MC3R1.R9.1 */ paddr_t addr, size; naddr = dt_number_of_address(np); @@ -1976,9 +1980,11 @@ static int __init make_cpus_node(const struct domain *d, void *fdt) const struct dt_device_node *npcpu; unsigned int cpu; const void *compatible = NULL; + /* SAF-1-safe MC3R1.R9.1 */ u32 len; /* Placeholder for cpu@ + a 32-bit hexadecimal number + \0 */ char buf[13]; + /* SAF-1-safe MC3R1.R9.1 */ u32 clock_frequency; /* Keep the compiler happy with -Og */ bool clock_valid = false; @@ -2104,6 +2110,7 @@ static int __init make_gic_node(const struct domain *d, void *fdt, const struct dt_device_node *gic = dt_interrupt_controller; int res = 0; const void *addrcells, *sizecells; + /* SAF-1-safe MC3R1.R9.1 */ u32 addrcells_len, sizecells_len; /* @@ -2179,6 +2186,7 @@ static int __init make_timer_node(const struct kernel_info *kinfo) int res; unsigned int irq[MAX_TIMER_PPI]; gic_interrupt_t intrs[3]; + /* SAF-1-safe MC3R1.R9.1 */ u32 clock_frequency; bool clock_valid; @@ -2511,6 +2519,7 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, unsigned int naddr; unsigned int i; int res; + /* SAF-1-safe MC3R1.R9.1 */ paddr_t addr, size; bool own_device = !dt_device_for_passthrough(dev); /* @@ -2779,6 +2788,7 @@ static int __init make_gicv2_domU_node(struct kernel_info *kinfo) { void *fdt = kinfo->fdt; int res = 0; + /* SAF-1-safe MC3R1.R9.1*/ __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2]; __be32 *cells; const struct domain *d = kinfo->d; @@ -2914,6 +2924,7 @@ static int __init make_vpl011_uart_node(struct kernel_info *kinfo) void *fdt = kinfo->fdt; int res; gic_interrupt_t intr; + /* SAF-1-safe MC3R1.R9.1*/ __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS]; __be32 *cells; struct domain *d = kinfo->d; @@ -3435,6 +3446,7 @@ static void __init initrd_load(struct kernel_info *kinfo) paddr_t paddr, len; int node; int res; + /* SAF-1-safe MC3R1.R9.1 */ __be32 val[2]; __be32 *cellp; void __iomem *initrd; @@ -3514,6 +3526,7 @@ static int __init get_evtchn_dt_property(const struct dt_device_node *np, uint32_t *port, uint32_t *phandle) { const __be32 *prop = NULL; + /* SAF-1-safe MC3R1.R9.1 */ uint32_t len; prop = dt_get_property(np, "xen,evtchn", &len); @@ -3538,10 +3551,13 @@ static int __init get_evtchn_dt_property(const struct dt_device_node *np, static int __init alloc_domain_evtchn(struct dt_device_node *node) { int rc; + /* SAF-1-safe MC3R1.R9.1 */ uint32_t domU1_port, domU2_port, remote_phandle; struct dt_device_node *remote_node; const struct dt_device_node *p1_node, *p2_node; + /* SAF-1-safe MC3R1.R9.1 */ struct evtchn_alloc_unbound alloc_unbound; + /* SAF-1-safe MC3R1.R9.1 */ struct evtchn_bind_interdomain bind_interdomain; struct domain *d1 = NULL, *d2 = NULL; @@ -3789,11 +3805,12 @@ static int __init construct_domain(struct domain *d, struct kernel_info *kinfo) static int __init alloc_xenstore_evtchn(struct domain *d) { - evtchn_alloc_unbound_t alloc; + evtchn_alloc_unbound_t alloc = { + .dom = d->domain_id, + .remote_dom = hardware_domain->domain_id + }; int rc; - alloc.dom = d->domain_id; - alloc.remote_dom = hardware_domain->domain_id; rc = evtchn_alloc_unbound(&alloc, 0); if ( rc ) { @@ -3810,8 +3827,9 @@ static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { struct kernel_info kinfo = {}; - const char *dom0less_enhanced; + const char *dom0less_enhanced = NULL; int rc; + /* SAF-1-safe MC3R1.R9.1 */ u64 mem; u32 p2m_mem_mb; unsigned long p2m_pages; @@ -3939,6 +3957,7 @@ void __init create_domUs(void) .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version), }; unsigned int flags = 0U; + /* SAF-1-safe MC3R1.R9.1 */ uint32_t val; int rc; diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index bb64925d70..25f39364d1 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -117,6 +117,7 @@ static int __init setup_chosen_node(void *fdt, int *addr_cells, int *size_cells) static int __init fdt_set_reg(void *fdt, int node, int addr_cells, int size_cells, uint64_t addr, uint64_t len) { + /* SAF-1-safe MC3R1.R9.1 */ __be32 val[4]; /* At most 2 64 bit values to be stored */ __be32 *cellp; @@ -308,7 +309,7 @@ fdt_set_fail: static void __init *fdt_increase_size(struct file *fdtfile, int add_size) { EFI_STATUS status; - EFI_PHYSICAL_ADDRESS fdt_addr; + EFI_PHYSICAL_ADDRESS fdt_addr = 0; int fdt_size; int pages; void *new_fdt; @@ -433,7 +434,7 @@ static void __init efi_arch_cfg_file_late(const EFI_LOADED_IMAGE *image, static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size) { - void *ptr; + void *ptr = NULL; EFI_STATUS status; status = efi_bs->AllocatePool(EfiLoaderData, map_size, &ptr); @@ -538,6 +539,7 @@ static void __init efi_arch_handle_module(const struct file *file, { int node; int chosen; + /* SAF-1-safe MC3R1.R9.1 */ int addr_len, size_len; if ( file == &dtbfile ) diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 3aa4edda10..aa0180ab5b 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -192,8 +192,7 @@ static int its_send_cmd_mapc(struct host_its *its, uint32_t collection_id, cmd[0] = GITS_CMD_MAPC; cmd[1] = 0x00; - cmd[2] = encode_rdbase(its, cpu, collection_id); - cmd[2] |= GITS_VALID_BIT; + cmd[2] = encode_rdbase(its, cpu, collection_id) | GITS_VALID_BIT; cmd[3] = 0x00; return its_send_command(its, cmd); @@ -215,9 +214,7 @@ static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid, } cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32); cmd[1] = size_bits; - cmd[2] = itt_addr; - if ( valid ) - cmd[2] |= GITS_VALID_BIT; + cmd[2] = itt_addr | (valid ? GITS_VALID_BIT : 0x00); cmd[3] = 0x00; return its_send_command(its, cmd); @@ -911,6 +908,7 @@ int gicv3_its_make_hwdom_dt_nodes(const struct domain *d, const struct dt_device_node *gic, void *fdt) { + /* SAF-1-safe MC3R1.R9.1 */ uint32_t len; int res; const void *prop = NULL; @@ -1004,6 +1002,7 @@ static void gicv3_its_dt_init(const struct dt_device_node *node) */ dt_for_each_child_node(node, its) { + /* SAF-1-safe MC3R1.R9.1 */ paddr_t addr, size; if ( !dt_device_is_compatible(its, "arm,gic-v3-its") ) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index c688227abd..a36068b2d8 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -935,6 +935,7 @@ static int xen_pt_update_entry(mfn_t root, unsigned long virt, mfn_t mfn, unsigned int target, unsigned int flags) { + /* SAF-1-safe MC3R1.R9.1 */ int rc; unsigned int level; lpae_t *table; diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index de32a2d638..83c56cf1cb 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -496,16 +496,18 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, lpae_t entry, *table; int rc; mfn_t mfn = INVALID_MFN; - p2m_type_t _t; + p2m_type_t _t = p2m_invalid; DECLARE_OFFSETS(offsets, addr); ASSERT(p2m_is_locked(p2m)); BUILD_BUG_ON(THIRD_MASK != PAGE_MASK); /* Allow t to be NULL */ - t = t ?: &_t; - - *t = p2m_invalid; + if( t ) { + *t = _t; + } else { + t = &_t; + } if ( valid ) *valid = false; @@ -1031,6 +1033,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m, unsigned int level = 0; unsigned int target = 3 - (page_order / XEN_PT_LPAE_SHIFT); lpae_t *entry, *table, orig_pte; + /* SAF-1-safe MC3R1.R9.1 */ int rc; /* A mapping is removed if the MFN is invalid. */ bool removing_mapping = mfn_eq(smfn, INVALID_MFN); @@ -1483,6 +1486,7 @@ static inline int p2m_remove_mapping(struct domain *d, { struct p2m_domain *p2m = p2m_get_hostp2m(d); unsigned long i; + /* SAF-1-safe MC3R1.R9.1 */ int rc; p2m_write_lock(p2m); @@ -1685,20 +1689,21 @@ static int p2m_alloc_vmid(struct domain *d) ASSERT(nr != INVALID_VMID); - if ( nr == MAX_VMID ) - { - rc = -EBUSY; - printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->domain_id); - goto out; - } + do { + if ( nr == MAX_VMID ) + { + rc = -EBUSY; + printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->domain_id); + break; + } - set_bit(nr, vmid_mask); + set_bit(nr, vmid_mask); - p2m->vmid = nr; + p2m->vmid = nr; - rc = 0; + rc = 0; + } while ( 0 ); -out: spin_unlock(&vmid_alloc_lock); return rc; } From patchwork Fri Jul 14 11:49:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Vetrini X-Patchwork-Id: 13313617 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E9301C0015E for ; Fri, 14 Jul 2023 11:50:09 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.563584.880887 (Exim 4.92) (envelope-from ) id 1qKHJ5-0005nk-KG; Fri, 14 Jul 2023 11:49:55 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 563584.880887; Fri, 14 Jul 2023 11:49:55 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ5-0005nb-Gl; Fri, 14 Jul 2023 11:49:55 +0000 Received: by outflank-mailman (input) for mailman id 563584; Fri, 14 Jul 2023 11:49:53 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ3-0005Gy-UA for xen-devel@lists.xenproject.org; Fri, 14 Jul 2023 11:49:53 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 8b19dbbb-223c-11ee-8611-37d641c3527e; Fri, 14 Jul 2023 13:49:52 +0200 (CEST) Received: from nico.bugseng.com (unknown [37.163.94.163]) by support.bugseng.com (Postfix) with ESMTPSA id 7A3D04EE0739; Fri, 14 Jul 2023 13:49:50 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 8b19dbbb-223c-11ee-8611-37d641c3527e From: Nicola Vetrini To: xen-devel@lists.xenproject.org Cc: sstabellini@kernel.org, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, Nicola Vetrini , Andrew Cooper , George Dunlap , Jan Beulich , Julien Grall , Wei Liu , Bertrand Marquis , Volodymyr Babchuk Subject: [RFC PATCH 2/4] xen/arm64: bitops: justify uninitialized variable inside a macro Date: Fri, 14 Jul 2023 13:49:19 +0200 Message-Id: X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 The macro 'testop' expands to a function that declares the local variable 'oldbit', which is written before being set, but is such a way that is not amenable to automatic checking. Therefore, a deviation comment, is introduced to document this situation. A similar reasoning applies to macro 'guest_testop'. Signed-off-by: Nicola Vetrini --- docs/misra/safe.json | 16 ++++++++++++++++ xen/arch/arm/arm64/lib/bitops.c | 3 +++ xen/arch/arm/include/asm/guest_atomics.h | 3 +++ 3 files changed, 22 insertions(+) diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 244001f5be..4cf7cbf57b 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -20,6 +20,22 @@ }, { "id": "SAF-2-safe", + "analyser": { + "eclair": "MC3R1.R9.1" + }, + "name": "Rule 9.1: initializer not needed", + "text": "The following local variables are possibly subject to being read before being written, but code inspection ensured that the control flow in the construct where they appear ensures that no such event may happen." + }, + { + "id": "SAF-3-safe", + "analyser": { + "eclair": "MC3R1.R9.1" + }, + "name": "Rule 9.1: initializer not needed", + "text": "The following local variables are possibly subject to being read before being written, but code inspection ensured that the control flow in the construct where they appear ensures that no such event may happen." + }, + { + "id": "SAF-4-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/arch/arm/arm64/lib/bitops.c b/xen/arch/arm/arm64/lib/bitops.c index 20e3f3d6ce..e0728bb29d 100644 --- a/xen/arch/arm/arm64/lib/bitops.c +++ b/xen/arch/arm/arm64/lib/bitops.c @@ -114,8 +114,11 @@ bitop(change_bit, eor) bitop(clear_bit, bic) bitop(set_bit, orr) +/* SAF-2-safe MC3R1.R9.1 */ testop(test_and_change_bit, eor) +/* SAF-2-safe MC3R1.R9.1 */ testop(test_and_clear_bit, bic) +/* SAF-2-safe MC3R1.R9.1 */ testop(test_and_set_bit, orr) static always_inline bool int_clear_mask16(uint16_t mask, volatile uint16_t *p, diff --git a/xen/arch/arm/include/asm/guest_atomics.h b/xen/arch/arm/include/asm/guest_atomics.h index a1745f8613..9d8f8ec3a3 100644 --- a/xen/arch/arm/include/asm/guest_atomics.h +++ b/xen/arch/arm/include/asm/guest_atomics.h @@ -67,8 +67,11 @@ guest_bitop(change_bit) /* test_bit does not use load-store atomic operations */ #define guest_test_bit(d, nr, p) ((void)(d), test_bit(nr, p)) +/* SAF-3-safe MC3R1.R9.1 */ guest_testop(test_and_set_bit) +/* SAF-3-safe MC3R1.R9.1 */ guest_testop(test_and_clear_bit) +/* SAF-3-safe MC3R1.R9.1 */ guest_testop(test_and_change_bit) #undef guest_testop From patchwork Fri Jul 14 11:49:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Vetrini X-Patchwork-Id: 13313616 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D43E0EB64DA for ; Fri, 14 Jul 2023 11:50:05 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.563585.880893 (Exim 4.92) (envelope-from ) id 1qKHJ5-0005ra-W0; Fri, 14 Jul 2023 11:49:55 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 563585.880893; Fri, 14 Jul 2023 11:49:55 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ5-0005qg-QH; Fri, 14 Jul 2023 11:49:55 +0000 Received: by outflank-mailman (input) for mailman id 563585; Fri, 14 Jul 2023 11:49:54 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ4-0005Xk-EZ for xen-devel@lists.xenproject.org; Fri, 14 Jul 2023 11:49:54 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 8bae35d5-223c-11ee-b239-6b7b168915f2; Fri, 14 Jul 2023 13:49:53 +0200 (CEST) Received: from nico.bugseng.com (unknown [37.163.94.163]) by support.bugseng.com (Postfix) with ESMTPSA id 464074EE0C8B; Fri, 14 Jul 2023 13:49:52 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 8bae35d5-223c-11ee-b239-6b7b168915f2 From: Nicola Vetrini To: xen-devel@lists.xenproject.org Cc: sstabellini@kernel.org, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, Nicola Vetrini , Julien Grall , Bertrand Marquis , Volodymyr Babchuk Subject: [RFC PATCH 3/4] xen/arm: initialize conditionally uninitialized local variables Date: Fri, 14 Jul 2023 13:49:20 +0200 Message-Id: X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 This patch aims to fix some occurrences of possibly uninitialized variables, that may be read before being written. This behaviour would violate MISRA C:2012 Rule 9.1, besides being generally undesirable. In all the analyzed cases, such accesses were actually safe, but it's quite difficult to prove so by automatic checking, therefore a safer route is to change the code so as to avoid the behaviour from occurring, while preserving the semantics. An initialization to a safe value is provided to reach this aim. Signed-off-by: Nicola Vetrini --- Additional input on which values may be 'safe' in each context is surely welcome, to avoid possibly compromising the correctness of the function semantics. --- xen/arch/arm/cpuerrata.c | 6 +++--- xen/arch/arm/domctl.c | 8 ++++---- xen/arch/arm/gic-v3-lpi.c | 17 +++++++++-------- xen/arch/arm/include/asm/p2m.h | 10 ++++++---- xen/arch/arm/platforms/xilinx-zynqmp-eemi.c | 10 ++-------- xen/arch/arm/psci.c | 10 +++++----- xen/drivers/char/pl011.c | 2 +- 7 files changed, 30 insertions(+), 33 deletions(-) diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index d0658aedb6..14694c6081 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -159,7 +159,7 @@ extern char __mitigate_spectre_bhb_loop_start_32[], static int enable_smccc_arch_workaround_1(void *data) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; const struct arm_cpu_capabilities *entry = data; /* @@ -252,7 +252,7 @@ static int enable_spectre_bhb_workaround(void *data) if ( cpus_have_cap(ARM_WORKAROUND_BHB_SMCC_3) ) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; if ( smccc_ver < SMCCC_VERSION(1, 1) ) goto warn; @@ -393,7 +393,7 @@ DEFINE_PER_CPU_READ_MOSTLY(register_t, ssbd_callback_required); static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; bool required; if ( smccc_ver < SMCCC_VERSION(1, 1) ) diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index ad56efb0f5..b38fed72be 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -29,10 +29,10 @@ static int handle_vuart_init(struct domain *d, struct xen_domctl_vuart_op *vuart_op) { int rc; - struct vpl011_init_info info; - - info.console_domid = vuart_op->console_domid; - info.gfn = _gfn(vuart_op->gfn); + struct vpl011_init_info info = { + .console_domid = vuart_op->console_domid, + .gfn = _gfn(vuart_op->gfn) + }; if ( d->creation_finished ) return -EPERM; diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c index eb0a5535e4..12f2af2e4d 100644 --- a/xen/arch/arm/gic-v3-lpi.c +++ b/xen/arch/arm/gic-v3-lpi.c @@ -210,7 +210,10 @@ out: void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, uint32_t virt_lpi) { - union host_lpi *hlpip, hlpi; + union host_lpi *hlpip, hlpi = { + .virt_lpi = virt_lpi, + .dom_id = domain_id + }; ASSERT(host_lpi >= LPI_OFFSET); @@ -218,9 +221,6 @@ void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, hlpip = &lpi_data.host_lpis[host_lpi / HOST_LPIS_PER_PAGE][host_lpi % HOST_LPIS_PER_PAGE]; - hlpi.virt_lpi = virt_lpi; - hlpi.dom_id = domain_id; - write_u64_atomic(&hlpip->data, hlpi.data); } @@ -542,14 +542,15 @@ int gicv3_allocate_host_lpi_block(struct domain *d, uint32_t *first_lpi) for ( i = 0; i < LPI_BLOCK; i++ ) { - union host_lpi hlpi; - /* * Mark this host LPI as belonging to the domain, but don't assign * any virtual LPI or a VCPU yet. */ - hlpi.virt_lpi = INVALID_LPI; - hlpi.dom_id = d->domain_id; + union host_lpi hlpi = { + .virt_lpi = INVALID_LPI, + .dom_id = d->domain_id + }; + write_u64_atomic(&lpi_data.host_lpis[chunk][lpi_idx + i].data, hlpi.data); diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h index 940495d42b..413e2a7add 100644 --- a/xen/arch/arm/include/asm/p2m.h +++ b/xen/arch/arm/include/asm/p2m.h @@ -345,7 +345,7 @@ static inline struct page_info *get_page_from_gfn( struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) { mfn_t mfn; - p2m_type_t _t; + p2m_type_t _t = p2m_invalid; struct page_info *page; /* @@ -355,10 +355,12 @@ static inline struct page_info *get_page_from_gfn( if ( likely(d != dom_xen) ) return p2m_get_page_from_gfn(d, _gfn(gfn), t); - if ( !t ) + /* Allow t to be NULL */ + if ( t ) + *t = _t; + else { t = &_t; - - *t = p2m_invalid; + } /* * DOMID_XEN sees 1-1 RAM. The p2m_type is based on the type of the diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c index 2053ed7ac5..39d9ab4fa9 100644 --- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c @@ -51,11 +51,11 @@ static inline bool domain_has_reset_access(struct domain *d, uint32_t rst) bool zynqmp_eemi(struct cpu_user_regs *regs) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; uint32_t fid = get_user_reg(regs, 0); uint32_t nodeid = get_user_reg(regs, 1); unsigned int pm_fn = fid & 0xFFFF; - enum pm_ret_status ret; + enum pm_ret_status ret = XST_PM_NO_ACCESS; switch ( fid ) { @@ -89,7 +89,6 @@ bool zynqmp_eemi(struct cpu_user_regs *regs) { gprintk(XENLOG_WARNING, "zynqmp-pm: fn=%u No access to node %u\n", pm_fn, nodeid); - ret = XST_PM_NO_ACCESS; goto done; } goto forward_to_fw; @@ -100,7 +99,6 @@ bool zynqmp_eemi(struct cpu_user_regs *regs) { gprintk(XENLOG_WARNING, "zynqmp-pm: fn=%u No access to reset %u\n", pm_fn, nodeid); - ret = XST_PM_NO_ACCESS; goto done; } goto forward_to_fw; @@ -116,7 +114,6 @@ bool zynqmp_eemi(struct cpu_user_regs *regs) case EEMI_FID(PM_MMIO_READ): gprintk(XENLOG_WARNING, "zynqmp-pm: fn=%u No MMIO access to %u\n", pm_fn, nodeid); - ret = XST_PM_NO_ACCESS; goto done; /* Exclusive to the hardware domain. */ @@ -146,14 +143,12 @@ bool zynqmp_eemi(struct cpu_user_regs *regs) if ( !is_hardware_domain(current->domain) ) { gprintk(XENLOG_WARNING, "eemi: fn=%u No access", pm_fn); - ret = XST_PM_NO_ACCESS; goto done; } goto forward_to_fw; /* These calls are never allowed. */ case EEMI_FID(PM_SYSTEM_SHUTDOWN): - ret = XST_PM_NO_ACCESS; goto done; case IPI_MAILBOX_FID(IPI_MAILBOX_OPEN): @@ -166,7 +161,6 @@ bool zynqmp_eemi(struct cpu_user_regs *regs) if ( !is_hardware_domain(current->domain) ) { gprintk(XENLOG_WARNING, "IPI mailbox: fn=%u No access", pm_fn); - ret = XST_PM_NO_ACCESS; goto done; } goto forward_to_fw; diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c index 695d2fa1f1..47e46af608 100644 --- a/xen/arch/arm/psci.c +++ b/xen/arch/arm/psci.c @@ -38,7 +38,7 @@ static uint32_t psci_cpu_on_nr; int call_psci_cpu_on(int cpu) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary), &res); @@ -50,7 +50,7 @@ void call_psci_cpu_off(void) { if ( psci_ver > PSCI_VERSION(0, 1) ) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; /* If successfull the PSCI cpu_off call doesn't return */ arm_smccc_smc(PSCI_0_2_FN32_CPU_OFF, &res); @@ -73,7 +73,7 @@ void call_psci_system_reset(void) static int __init psci_features(uint32_t psci_func_id) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; if ( psci_ver < PSCI_VERSION(1, 0) ) return PSCI_NOT_SUPPORTED; @@ -115,7 +115,7 @@ static void __init psci_init_smccc(void) if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED ) { - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; arm_smccc_smc(ARM_SMCCC_VERSION_FID, &res); if ( PSCI_RET(res) != ARM_SMCCC_NOT_SUPPORTED ) @@ -168,7 +168,7 @@ static int __init psci_init_0_2(void) { /* sentinel */ }, }; int ret; - struct arm_smccc_res res; + struct arm_smccc_res res = {0}; if ( acpi_disabled ) { diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c index f7bf3ad117..34ce90be52 100644 --- a/xen/drivers/char/pl011.c +++ b/xen/drivers/char/pl011.c @@ -285,7 +285,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev, const char *config = data; int res; paddr_t addr, size; - uint32_t io_width; + uint32_t io_width = 0; bool mmio32 = false, sbsa; if ( strcmp(config, "") ) From patchwork Fri Jul 14 11:49:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Vetrini X-Patchwork-Id: 13313618 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 236E7EB64DC for ; Fri, 14 Jul 2023 11:50:15 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.563586.880907 (Exim 4.92) (envelope-from ) id 1qKHJ8-0006Lk-8M; Fri, 14 Jul 2023 11:49:58 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 563586.880907; Fri, 14 Jul 2023 11:49:58 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ8-0006LX-2l; Fri, 14 Jul 2023 11:49:58 +0000 Received: by outflank-mailman (input) for mailman id 563586; Fri, 14 Jul 2023 11:49:56 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qKHJ5-0005Gy-UM for xen-devel@lists.xenproject.org; Fri, 14 Jul 2023 11:49:55 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 8c51e747-223c-11ee-8611-37d641c3527e; Fri, 14 Jul 2023 13:49:54 +0200 (CEST) Received: from nico.bugseng.com (unknown [37.163.94.163]) by support.bugseng.com (Postfix) with ESMTPSA id 463314EE0C8D; Fri, 14 Jul 2023 13:49:53 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 8c51e747-223c-11ee-8611-37d641c3527e From: Nicola Vetrini To: xen-devel@lists.xenproject.org Cc: sstabellini@kernel.org, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, Nicola Vetrini , Julien Grall , Bertrand Marquis , Volodymyr Babchuk Subject: [RFC PATCH 4/4] xen/arm: initialize conditionally uninitialized local variables Date: Fri, 14 Jul 2023 13:49:21 +0200 Message-Id: <6640fc480d550bb337455afc0c2663d4b288dd4f.1689329728.git.nicola.vetrini@bugseng.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 The function 'guest_walk_tables' contains an initialization pattern for the pointee of parameter 'perms' that is not easy for automatic checkers to reason about. A modified pattern that does not alter the semantics of the code is introduced. A const qualifier is added in 'xen/arch/arm/dm.c' because 'copy_to_guest_offset' doesn't modify that parameter. Signed-off-by: Nicola Vetrini --- xen/arch/arm/dm.c | 2 +- xen/arch/arm/guest_walk.c | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c index 5569efa121..910788e098 100644 --- a/xen/arch/arm/dm.c +++ b/xen/arch/arm/dm.c @@ -129,7 +129,7 @@ int dm_op(const struct dmop_args *op_args) if ( (!rc || rc == -ERESTART) && !const_op && copy_to_guest_offset(op_args->buf[0].h, offset, - (void *)&op.u, op_size[op.op]) ) + (const void *)&op.u, op_size[op.op]) ) rc = -EFAULT; out: diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index d99b411f3b..6c017242f2 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -552,15 +552,12 @@ bool guest_walk_tables(const struct vcpu *v, vaddr_t gva, { register_t sctlr = READ_SYSREG(SCTLR_EL1); register_t tcr = READ_SYSREG(TCR_EL1); - unsigned int _perms; + unsigned int _perms = GV2M_READ; /* We assume that the domain is running on the currently active domain. */ if ( v != current ) return false; - /* Allow perms to be NULL. */ - perms = perms ?: &_perms; - /* * Currently, we assume a GVA to IPA translation with EL1 privileges. * Since, valid mappings in the first stage address translation table are @@ -570,7 +567,12 @@ bool guest_walk_tables(const struct vcpu *v, vaddr_t gva, * attributes that distinguish between EL0 and EL1 permissions (EL0 might * not have permissions on the particular mapping). */ - *perms = GV2M_READ; + /* Allow perms to be NULL. */ + if( perms ) { + *perms = _perms; + } else { + perms = &_perms; + } /* If the MMU is disabled, there is no need to translate the gva. */ if ( !(sctlr & SCTLR_Axx_ELx_M) )