From nobody Mon Feb 9 19:10:42 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1689335410459196.31056457723332; Fri, 14 Jul 2023 04:50:10 -0700 (PDT) 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 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-Outflank-Mailman: Message body and most headers restored to incoming version 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 Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1689335412254100003 Content-Type: text/plain; charset="utf-8" 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 cont= rol 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/li= b/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 *add= r, unsigned long size, { const unsigned long *p =3D addr + BIT_WORD(offset); unsigned long result =3D offset & ~(BITS_PER_LONG-1); + /* SAF-1-safe MC3R1.R9.1 */ unsigned long tmp; =20 if (offset >=3D 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 *f= dt, 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; =20 @@ -169,7 +170,9 @@ int __init device_tree_for_each_node(const void *fdt, i= nt node, */ int depth =3D 0; const int first_node =3D 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; =20 @@ -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' =3D> 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; =20 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, =20 static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw= 1) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t hw2; uint16_t rt; =20 @@ -151,6 +152,7 @@ static int decode_arm64(register_t pc, mmio_info_t *inf= o) =20 static int decode_thumb(register_t pc, struct hsr_dabt *dabt) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t instr; =20 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); =20 int __init parse_arch_dom0_param(const char *s, const char *e) { - long long val; + long long val =3D LLONG_MAX; =20 if ( !parse_signed_integer("sve", s, e, &val) ) { @@ -1077,6 +1077,7 @@ static void __init assign_static_memory_11(struct dom= ain *d, static int __init handle_linux_pci_domain(struct kernel_info *kinfo, const struct dt_device_node *nod= e) { + /* SAF-1-safe MC3R1.R9.1 */ uint16_t segment; int res; =20 @@ -1351,6 +1352,7 @@ static int __init make_memory_node(const struct domai= n *d, unsigned int i; int res, reg_size =3D addrcells + sizecells; int nr_cells =3D 0; + /* SAF-1-safe MC3R1.R9.1*/ __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */]; __be32 *cells; =20 @@ -1578,6 +1580,7 @@ static int __init find_unallocated_memory(const struc= t kernel_info *kinfo, struct rangeset *unalloc_mem; paddr_t start, end; unsigned int i; + /* SAF-1-safe MC3R1.R9.1 */ int res; =20 dt_dprintk("Find unallocated memory for extended regions\n"); @@ -1727,6 +1730,7 @@ static int __init find_memory_holes(const struct kern= el_info *kinfo, dt_for_each_device_node( dt_host, np ) { unsigned int naddr; + /* SAF-1-safe MC3R1.R9.1 */ paddr_t addr, size; =20 naddr =3D 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 =3D 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 =3D false; @@ -2104,6 +2110,7 @@ static int __init make_gic_node(const struct domain *= d, void *fdt, const struct dt_device_node *gic =3D dt_interrupt_controller; int res =3D 0; const void *addrcells, *sizecells; + /* SAF-1-safe MC3R1.R9.1 */ u32 addrcells_len, sizecells_len; =20 /* @@ -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; =20 @@ -2511,6 +2519,7 @@ static int __init handle_device(struct domain *d, str= uct 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 =3D !dt_device_for_passthrough(dev); /* @@ -2779,6 +2788,7 @@ static int __init make_gicv2_domU_node(struct kernel_= info *kinfo) { void *fdt =3D kinfo->fdt; int res =3D 0; + /* SAF-1-safe MC3R1.R9.1*/ __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2]; __be32 *cells; const struct domain *d =3D kinfo->d; @@ -2914,6 +2924,7 @@ static int __init make_vpl011_uart_node(struct kernel= _info *kinfo) void *fdt =3D 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 =3D kinfo->d; @@ -3435,6 +3446,7 @@ static void __init initrd_load(struct kernel_info *ki= nfo) 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 =3D NULL; + /* SAF-1-safe MC3R1.R9.1 */ uint32_t len; =20 prop =3D dt_get_property(np, "xen,evtchn", &len); @@ -3538,10 +3551,13 @@ static int __init get_evtchn_dt_property(const stru= ct 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 =3D NULL, *d2 =3D NULL; =20 @@ -3789,11 +3805,12 @@ static int __init construct_domain(struct domain *d= , struct kernel_info *kinfo) =20 static int __init alloc_xenstore_evtchn(struct domain *d) { - evtchn_alloc_unbound_t alloc; + evtchn_alloc_unbound_t alloc =3D { + .dom =3D d->domain_id, + .remote_dom =3D hardware_domain->domain_id + }; int rc; =20 - alloc.dom =3D d->domain_id; - alloc.remote_dom =3D hardware_domain->domain_id; rc =3D 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 =3D {}; - const char *dom0less_enhanced; + const char *dom0less_enhanced =3D 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 =3D XEN_DOMCTL_GRANT_version(opt_gnttab_max_versio= n), }; unsigned int flags =3D 0U; + /* SAF-1-safe MC3R1.R9.1 */ uint32_t val; int rc; =20 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 *add= r_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; =20 @@ -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 =3D 0; int fdt_size; int pages; void *new_fdt; @@ -433,7 +434,7 @@ static void __init efi_arch_cfg_file_late(const EFI_LOA= DED_IMAGE *image, =20 static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size) { - void *ptr; + void *ptr =3D NULL; EFI_STATUS status; =20 status =3D 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; =20 if ( file =3D=3D &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, uint= 32_t collection_id, =20 cmd[0] =3D GITS_CMD_MAPC; cmd[1] =3D 0x00; - cmd[2] =3D encode_rdbase(its, cpu, collection_id); - cmd[2] |=3D GITS_VALID_BIT; + cmd[2] =3D encode_rdbase(its, cpu, collection_id) | GITS_VALID_BIT; cmd[3] =3D 0x00; =20 return its_send_command(its, cmd); @@ -215,9 +214,7 @@ static int its_send_cmd_mapd(struct host_its *its, uint= 32_t deviceid, } cmd[0] =3D GITS_CMD_MAPD | ((uint64_t)deviceid << 32); cmd[1] =3D size_bits; - cmd[2] =3D itt_addr; - if ( valid ) - cmd[2] |=3D GITS_VALID_BIT; + cmd[2] =3D itt_addr | (valid ? GITS_VALID_BIT : 0x00); cmd[3] =3D 0x00; =20 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 =3D 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; =20 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 lon= g 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 =3D INVALID_MFN; - p2m_type_t _t; + p2m_type_t _t =3D p2m_invalid; DECLARE_OFFSETS(offsets, addr); =20 ASSERT(p2m_is_locked(p2m)); BUILD_BUG_ON(THIRD_MASK !=3D PAGE_MASK); =20 /* Allow t to be NULL */ - t =3D t ?: &_t; - - *t =3D p2m_invalid; + if( t ) { + *t =3D _t; + } else { + t =3D &_t; + } =20 if ( valid ) *valid =3D false; @@ -1031,6 +1033,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m, unsigned int level =3D 0; unsigned int target =3D 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 =3D mfn_eq(smfn, INVALID_MFN); @@ -1483,6 +1486,7 @@ static inline int p2m_remove_mapping(struct domain *d, { struct p2m_domain *p2m =3D p2m_get_hostp2m(d); unsigned long i; + /* SAF-1-safe MC3R1.R9.1 */ int rc; =20 p2m_write_lock(p2m); @@ -1685,20 +1689,21 @@ static int p2m_alloc_vmid(struct domain *d) =20 ASSERT(nr !=3D INVALID_VMID); =20 - if ( nr =3D=3D MAX_VMID ) - { - rc =3D -EBUSY; - printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->domain= _id); - goto out; - } + do { + if ( nr =3D=3D MAX_VMID ) + { + rc =3D -EBUSY; + printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->doma= in_id); + break; + } =20 - set_bit(nr, vmid_mask); + set_bit(nr, vmid_mask); =20 - p2m->vmid =3D nr; + p2m->vmid =3D nr; =20 - rc =3D 0; + rc =3D 0; + } while ( 0 ); =20 -out: spin_unlock(&vmid_alloc_lock); return rc; } --=20 2.34.1