From patchwork Thu Sep 26 10:14:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 13813157 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 E1920CCFA15 for ; Thu, 26 Sep 2024 10:21:58 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.805201.1216247 (Exim 4.92) (envelope-from ) id 1stld6-0001OU-N6; Thu, 26 Sep 2024 10:21:48 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 805201.1216247; Thu, 26 Sep 2024 10:21:48 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1stld6-0001OL-Jb; Thu, 26 Sep 2024 10:21:48 +0000 Received: by outflank-mailman (input) for mailman id 805201; Thu, 26 Sep 2024 10:21:47 +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 1stld5-0001Nm-Bu for xen-devel@lists.xenproject.org; Thu, 26 Sep 2024 10:21:47 +0000 Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [2a00:1450:4864:20::62a]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 2164b3eb-7bf1-11ef-99a2-01e77a169b0f; Thu, 26 Sep 2024 12:21:45 +0200 (CEST) Received: by mail-ej1-x62a.google.com with SMTP id a640c23a62f3a-a8d29b7edc2so97830066b.1 for ; Thu, 26 Sep 2024 03:21:45 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f3420esm329957766b.40.2024.09.26.03.21.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:21:43 -0700 (PDT) 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: 2164b3eb-7bf1-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1727346104; x=1727950904; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=eDso/Pr8s7RpxPIwn6UDYP8gOZplQOaVxuligGB90sQ=; b=rENQF9NQxCZq4wlrOY70MdmrBlz/SYY+5AJIqhtrDpFrnJ7kBh1Qe7d1fudb347YK6 oOA98F5rzyygUactTzDHd0ViJQn2mUtGzkZdVkhUnK06J80kOLDk9ma10C5hl8n78x7S WgDBa8kMBtIhim0987KmXlBrLsOUnqt0rYNtA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727346104; x=1727950904; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eDso/Pr8s7RpxPIwn6UDYP8gOZplQOaVxuligGB90sQ=; b=MMrKmdUcHzQNXVqLnFKj5ZxeUe6i83XReJb5fA1MxbcOnogkyjr/pKHY4dGvCbKNdn 4AY4d48LwX8XAEDlmRQAKlE0WiwQzUMLLbYXCspopooDIx52DIiwaWa4mJu1Qttj4spp VFTIrwpTkaur9+BGZC11SF6sB5LmtcQDa1tgEhxo8tAShiJ8cUvleLgU0B5wV1GxmIK7 M5h4v1/7SUHhGWvkXNmrhkcKWMAX07tW5ts2n329OIRUdqHZo5RDrU9dYeJFrcvAv8It zRZEDQJlM16wL+21buokqg2pteakHwS5tAAJE5kKJpbYJiufxeljJN9WnLsplz4AvrDI ZI8g== X-Gm-Message-State: AOJu0YychFahvbUAdp+Ytvd/ntxzFc2jzVu7CJ9bbRTgqtzY7BNZZ3oh qt/Z4+BIg0T1cLhSoRIhD7yLOP3YjW2UxpNKyThgBdrMGIn1PH6n8SpD9+wNjuQQ487lLsL2cE9 s X-Google-Smtp-Source: AGHT+IEvsbzLb/qGdqqJ0XTDWcyfYeCjaaAUF7ElCrZjELC3nWpE3pht/kOZv3/RRWA68fzj8NrG0w== X-Received: by 2002:a17:907:1ca1:b0:a8c:78a5:8fc4 with SMTP id a640c23a62f3a-a93a036a36bmr444073166b.19.1727346104080; Thu, 26 Sep 2024 03:21:44 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Ross Lagerwall , Stefano Stabellini , Julien Grall , Bertrand Marquis , Michal Orzel , Volodymyr Babchuk , Jan Beulich , Andrew Cooper Subject: [PATCH v3 1/5] xen/livepatch: drop load_addr Elf section field Date: Thu, 26 Sep 2024 12:14:27 +0200 Message-ID: <20240926101431.97444-2-roger.pau@citrix.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240926101431.97444-1-roger.pau@citrix.com> References: <20240926101431.97444-1-roger.pau@citrix.com> MIME-Version: 1.0 The Elf loading logic will initially use the `data` section field to stash a pointer to the temporary loaded data (from the buffer allocated in livepatch_upload(), which is later relocated and the new pointer stashed in `load_addr`. Remove this dual field usage and use an `addr` uniformly. Initially data will point to the temporary buffer, until relocation happens, at which point the pointer will be updated to the relocated address. This avoids leaking a dangling pointer in the `data` field once the temporary buffer is freed by livepatch_upload(). Note the `addr` field cannot retain the const attribute from the previous `data`field, as there's logic that performs manipulations against the loaded sections, like applying relocations or sorting the exception table. No functional change intended. Signed-off-by: Roger Pau Monné Reviewed-by: Andrew Cooper Acked-by: Michal Orzel Reviewed-by: Ross Lagerwall --- Changes since v2: - New in this version. --- xen/arch/arm/arm32/livepatch.c | 8 +++--- xen/arch/arm/arm64/livepatch.c | 4 +-- xen/arch/x86/livepatch.c | 4 +-- xen/common/livepatch.c | 43 ++++++++++++++++++--------------- xen/common/livepatch_elf.c | 20 +++++++-------- xen/include/xen/livepatch_elf.h | 11 +++++---- 6 files changed, 47 insertions(+), 43 deletions(-) diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c index d50066564666..134d07a175bb 100644 --- a/xen/arch/arm/arm32/livepatch.c +++ b/xen/arch/arm/arm32/livepatch.c @@ -239,20 +239,20 @@ int arch_livepatch_perform(struct livepatch_elf *elf, if ( use_rela ) { - const Elf_RelA *r_a = rela->data + i * rela->sec->sh_entsize; + const Elf_RelA *r_a = rela->addr + i * rela->sec->sh_entsize; symndx = ELF32_R_SYM(r_a->r_info); type = ELF32_R_TYPE(r_a->r_info); - dest = base->load_addr + r_a->r_offset; /* P */ + dest = base->addr + r_a->r_offset; /* P */ addend = r_a->r_addend; } else { - const Elf_Rel *r = rela->data + i * rela->sec->sh_entsize; + const Elf_Rel *r = rela->addr + i * rela->sec->sh_entsize; symndx = ELF32_R_SYM(r->r_info); type = ELF32_R_TYPE(r->r_info); - dest = base->load_addr + r->r_offset; /* P */ + dest = base->addr + r->r_offset; /* P */ addend = get_addend(type, dest); } diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index dfb72be44fb8..d80051f9dc67 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -246,9 +246,9 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf, for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ ) { - const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize; + const Elf_RelA *r = rela->addr + i * rela->sec->sh_entsize; unsigned int symndx = ELF64_R_SYM(r->r_info); - void *dest = base->load_addr + r->r_offset; /* P */ + void *dest = base->addr + r->r_offset; /* P */ bool overflow_check = true; int ovf = 0; uint64_t val; diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 4f76127e1f77..be40f625d206 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -258,9 +258,9 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf, for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ ) { - const Elf_RelA *r = rela->data + i * rela->sec->sh_entsize; + const Elf_RelA *r = rela->addr + i * rela->sec->sh_entsize; unsigned int symndx = ELF64_R_SYM(r->r_info); - uint8_t *dest = base->load_addr + r->r_offset; + uint8_t *dest = base->addr + r->r_offset; uint64_t val; if ( symndx == STN_UNDEF ) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index df41dcce970a..7e6bf58f4408 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -371,18 +371,21 @@ static int move_payload(struct payload *payload, struct livepatch_elf *elf) ASSERT(offset[i] != UINT_MAX); - elf->sec[i].load_addr = buf + offset[i]; + buf += offset[i]; /* Don't copy NOBITS - such as BSS. */ if ( elf->sec[i].sec->sh_type != SHT_NOBITS ) { - memcpy(elf->sec[i].load_addr, elf->sec[i].data, + memcpy(buf, elf->sec[i].addr, elf->sec[i].sec->sh_size); dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Loaded %s at %p\n", - elf->name, elf->sec[i].name, elf->sec[i].load_addr); + elf->name, elf->sec[i].name, buf); } else - memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size); + memset(buf, 0, elf->sec[i].sec->sh_size); + + /* Replace the temporary buffer with the relocated one. */ + elf->sec[i].addr = buf; } } @@ -616,7 +619,7 @@ static inline int livepatch_check_expectations(const struct payload *payload) break; \ if ( !section_ok(elf, __sec, sizeof(*hook)) || __sec->sec->sh_size != sizeof(*hook) ) \ return -EINVAL; \ - hook = __sec->load_addr; \ + hook = __sec->addr; \ } while (0) /* @@ -630,7 +633,7 @@ static inline int livepatch_check_expectations(const struct payload *payload) break; \ if ( !section_ok(elf, __sec, sizeof(*hook)) ) \ return -EINVAL; \ - hook = __sec->load_addr; \ + hook = __sec->addr; \ nhooks = __sec->sec->sh_size / sizeof(*hook); \ } while (0) @@ -650,7 +653,7 @@ static int prepare_payload(struct payload *payload, if ( !section_ok(elf, sec, sizeof(*payload->funcs)) ) return -EINVAL; - payload->funcs = funcs = sec->load_addr; + payload->funcs = funcs = sec->addr; payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs); payload->fstate = xzalloc_array(typeof(*payload->fstate), @@ -709,7 +712,7 @@ static int prepare_payload(struct payload *payload, { const struct payload *data; - n = sec->load_addr; + n = sec->addr; if ( sec->sec->sh_size <= sizeof(*n) ) return -EINVAL; @@ -739,7 +742,7 @@ static int prepare_payload(struct payload *payload, sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_DEPENDS); if ( sec ) { - n = sec->load_addr; + n = sec->addr; if ( sec->sec->sh_size <= sizeof(*n) ) return -EINVAL; @@ -755,7 +758,7 @@ static int prepare_payload(struct payload *payload, sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS); if ( sec ) { - n = sec->load_addr; + n = sec->addr; if ( sec->sec->sh_size <= sizeof(*n) ) return -EINVAL; @@ -794,8 +797,8 @@ static int prepare_payload(struct payload *payload, if ( !section_ok(elf, sec, sizeof(*region->frame[i].start)) ) return -EINVAL; - region->frame[i].start = sec->load_addr; - region->frame[i].stop = sec->load_addr + sec->sec->sh_size; + region->frame[i].start = sec->addr; + region->frame[i].stop = sec->addr + sec->sec->sh_size; } sec = livepatch_elf_sec_by_name(elf, ".altinstructions"); @@ -843,8 +846,8 @@ static int prepare_payload(struct payload *payload, return -EINVAL; } - start = sec->load_addr; - end = sec->load_addr + sec->sec->sh_size; + start = sec->addr; + end = sec->addr + sec->sec->sh_size; for ( a = start; a < end; a++ ) { @@ -867,14 +870,14 @@ static int prepare_payload(struct payload *payload, * repl must be fully within .altinstr_replacement, even if the * replacement and the section happen to both have zero length. */ - if ( repl < repl_sec->load_addr || + if ( repl < repl_sec->addr || a->repl_len > repl_sec->sec->sh_size || - repl + a->repl_len > repl_sec->load_addr + repl_sec->sec->sh_size ) + repl + a->repl_len > repl_sec->addr + repl_sec->sec->sh_size ) { printk(XENLOG_ERR LIVEPATCH "%s Alternative repl %p+%#x outside .altinstr_replacement %p+%#"PRIxElfWord"\n", elf->name, repl, a->repl_len, - repl_sec->load_addr, repl_sec->sec->sh_size); + repl_sec->addr, repl_sec->sec->sh_size); return -EINVAL; } } @@ -896,8 +899,8 @@ static int prepare_payload(struct payload *payload, if ( !section_ok(elf, sec, sizeof(*region->ex)) ) return -EINVAL; - s = sec->load_addr; - e = sec->load_addr + sec->sec->sh_size; + s = sec->addr; + e = sec->addr + sec->sec->sh_size; sort_exception_table(s ,e); @@ -916,7 +919,7 @@ static int prepare_payload(struct payload *payload, if ( !section_ok(elf, sec, sizeof(*payload->metadata.data)) ) return -EINVAL; - payload->metadata.data = sec->load_addr; + payload->metadata.data = sec->addr; payload->metadata.len = sec->sec->sh_size; /* The metadata is required to consists of null terminated strings. */ diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c index 45d73912a3cd..25ce1bd5a0ad 100644 --- a/xen/common/livepatch_elf.c +++ b/xen/common/livepatch_elf.c @@ -36,7 +36,7 @@ static int elf_verify_strtab(const struct livepatch_elf_sec *sec) if ( !s->sh_size ) return -EINVAL; - contents = sec->data; + contents = sec->addr; if ( contents[0] || contents[s->sh_size - 1] ) return -EINVAL; @@ -44,7 +44,7 @@ static int elf_verify_strtab(const struct livepatch_elf_sec *sec) return 0; } -static int elf_resolve_sections(struct livepatch_elf *elf, const void *data) +static int elf_resolve_sections(struct livepatch_elf *elf, void *data) { struct livepatch_elf_sec *sec; unsigned int i; @@ -104,7 +104,7 @@ static int elf_resolve_sections(struct livepatch_elf *elf, const void *data) sec[i].sec->sh_size > LIVEPATCH_MAX_SIZE ) return -EINVAL; - sec[i].data = data + delta; + sec[i].addr = data + delta; /* Name is populated in elf_resolve_section_names. */ sec[i].name = NULL; @@ -226,14 +226,14 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data) strtab_sec = elf->strtab; /* Pointers arithmetic to get file offset. */ - offset = strtab_sec->data - data; + offset = strtab_sec->addr - data; /* Checked already in elf_resolve_sections, but just in case. */ ASSERT(offset == strtab_sec->sec->sh_offset); ASSERT(offset < elf->len && (offset + strtab_sec->sec->sh_size <= elf->len)); - /* symtab_sec->data was computed in elf_resolve_sections. */ - ASSERT((symtab_sec->sec->sh_offset + data) == symtab_sec->data); + /* symtab_sec->addr was computed in elf_resolve_sections. */ + ASSERT((symtab_sec->sec->sh_offset + data) == symtab_sec->addr); /* No need to check values as elf_resolve_sections did it. */ nsym = symtab_sec->sec->sh_size / symtab_sec->sec->sh_entsize; @@ -251,7 +251,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data) for ( i = 1; i < nsym; i++ ) { - const Elf_Sym *s = symtab_sec->data + symtab_sec->sec->sh_entsize * i; + const Elf_Sym *s = symtab_sec->addr + symtab_sec->sec->sh_entsize * i; delta = s->st_name; /* Boundary check within the .strtab. */ @@ -263,7 +263,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data) } sym[i].sym = s; - sym[i].name = strtab_sec->data + delta; + sym[i].name = strtab_sec->addr + delta; if ( arch_livepatch_symbol_deny(elf, &sym[i]) ) { printk(XENLOG_ERR LIVEPATCH "%s: Symbol '%s' should not be in payload\n", @@ -342,7 +342,7 @@ int livepatch_elf_resolve_symbols(struct livepatch_elf *elf) break; } - st_value += (unsigned long)elf->sec[idx].load_addr; + st_value += (unsigned long)elf->sec[idx].addr; if ( elf->sym[i].name ) dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Symbol resolved: %s => %#"PRIxElfAddr" (%s)\n", elf->name, elf->sym[i].name, @@ -503,7 +503,7 @@ static int livepatch_header_check(const struct livepatch_elf *elf) return 0; } -int livepatch_elf_load(struct livepatch_elf *elf, const void *data) +int livepatch_elf_load(struct livepatch_elf *elf, void *data) { int rc; diff --git a/xen/include/xen/livepatch_elf.h b/xen/include/xen/livepatch_elf.h index 7116deaddc28..842111e14518 100644 --- a/xen/include/xen/livepatch_elf.h +++ b/xen/include/xen/livepatch_elf.h @@ -13,10 +13,11 @@ struct livepatch_elf_sec { const Elf_Shdr *sec; /* Hooked up in elf_resolve_sections.*/ const char *name; /* Human readable name hooked in elf_resolve_section_names. */ - const void *data; /* Pointer to the section (done by - elf_resolve_sections). */ - void *load_addr; /* A pointer to the allocated destination. - Done by load_payload_data. */ + void *addr; /* + * Pointer to the section. This is + * first a temporary buffer, then + * later the relocated load address. + */ }; struct livepatch_elf_sym { @@ -41,7 +42,7 @@ struct livepatch_elf { const struct livepatch_elf_sec * livepatch_elf_sec_by_name(const struct livepatch_elf *elf, const char *name); -int livepatch_elf_load(struct livepatch_elf *elf, const void *data); +int livepatch_elf_load(struct livepatch_elf *elf, void *data); void livepatch_elf_free(struct livepatch_elf *elf); int livepatch_elf_resolve_symbols(struct livepatch_elf *elf); From patchwork Thu Sep 26 10:14:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 13813154 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 AAC4BCCFA13 for ; Thu, 26 Sep 2024 10:21:58 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.805202.1216253 (Exim 4.92) (envelope-from ) id 1stld7-0001Re-1B; Thu, 26 Sep 2024 10:21:49 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 805202.1216253; Thu, 26 Sep 2024 10:21:48 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1stld6-0001Qk-Qr; Thu, 26 Sep 2024 10:21:48 +0000 Received: by outflank-mailman (input) for mailman id 805202; Thu, 26 Sep 2024 10:21:48 +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 1stld6-0001Nm-0p for xen-devel@lists.xenproject.org; Thu, 26 Sep 2024 10:21:48 +0000 Received: from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com [2a00:1450:4864:20::12f]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 220ccade-7bf1-11ef-99a2-01e77a169b0f; Thu, 26 Sep 2024 12:21:46 +0200 (CEST) Received: by mail-lf1-x12f.google.com with SMTP id 2adb3069b0e04-53653ee23adso768328e87.3 for ; Thu, 26 Sep 2024 03:21:46 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f4fa95sm330147866b.54.2024.09.26.03.21.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:21:44 -0700 (PDT) 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: 220ccade-7bf1-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1727346105; x=1727950905; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=o74cvTmFfSOR3GfXakT+C/j8XZSivGfstOI/1D0MD/I=; b=LodCBx/3DnNSEKTeoiz4WBR61x5QAlcUPQaPIc6cffnY43YGYXKO3LkdxtJyaVHXkE 9+a2Jt/Teq1HkraoLlejqmTDSGpHk9Muv3GQqL2Ye8H82dYagAfEdjCc8IzOSfJBOJvR ZijhZ/WXPijsQrBawSTS/vaEyjxBKucG9NaS0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727346105; x=1727950905; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=o74cvTmFfSOR3GfXakT+C/j8XZSivGfstOI/1D0MD/I=; b=jzQJ0Xpg6vKTOAcQ26OvejhQdZjRAZzw9ID9DhdjVrnsi7RAZBc7lf8MkQuvStbOJP /yQD9U5mVUJtnNyYtRcqg3gH99EjaOz8Hm7zKci5jT1U+KmsyGstioxM1wSt5nfepIgz AL7bsUc3IOodYlWxRQ9Q1y3knWTjxkp5qe6imBkOIlPM00q2QHNKh2ld4yW86T1jDkWE I7aH2CMxT6M+iu9ESd76VncIUMBaCsc+Bs68akznLYfUSrEXxl8uBPzjhBx/uNdlT//u lBpgBMmAN6PrUqqwc+onXebMWUH4sf6NaFTvskZEppzWY2dLFc4+LXu7xFRKpA/b1mHA ruXQ== X-Gm-Message-State: AOJu0Yzx40GAr93JNBvvQHigsTyu8MX501xz/f8GXLXuV3kXsRkVe+FD J9M8oKTF1swB21wU5p6rOpd7XrNT1JaWvBLKNri07dzzN2aucUtz70SQ8TZyaVTj1rZNLxdvjPY z X-Google-Smtp-Source: AGHT+IEKLwJbQY4UX1HEyFW8X29MtHCp83ov6Sj5aShomUs3mfYct0EL9ch1rHzx+qPjwbsE0i43uw== X-Received: by 2002:a05:6512:3b0e:b0:535:6cca:9453 with SMTP id 2adb3069b0e04-5387048ab7fmr3572971e87.12.1727346105246; Thu, 26 Sep 2024 03:21:45 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Ross Lagerwall , Andrew Cooper Subject: [PATCH v3 2/5] xen/livepatch: simplify and unify logic in prepare_payload() Date: Thu, 26 Sep 2024 12:14:28 +0200 Message-ID: <20240926101431.97444-3-roger.pau@citrix.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240926101431.97444-1-roger.pau@citrix.com> References: <20240926101431.97444-1-roger.pau@citrix.com> MIME-Version: 1.0 The following sections: .note.gnu.build-id, .livepatch.xen_depends and .livepatch.depends are mandatory and ensured to be present by check_special_sections() before prepare_payload() is called. Simplify the logic in prepare_payload() by introducing a generic function to parse the sections that contain a buildid. Note the function assumes the buildid related section to always be present. No functional change intended. Signed-off-by: Roger Pau Monné Reviewed-by: Andrew Cooper Reviewed-by: Ross Lagerwall --- Changes since v1: - Rename. - Change order of assert. --- xen/common/livepatch.c | 110 +++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 60 deletions(-) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 7e6bf58f4408..50e2268e19a3 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -470,6 +470,31 @@ static int xen_build_id_dep(const struct payload *payload) return 0; } +/* Parses build-id sections into the given destination. */ +static int parse_buildid(const struct livepatch_elf_sec *sec, + struct livepatch_build_id *id) +{ + const Elf_Note *n; + int rc; + + /* Presence of the sections is ensured by check_special_sections(). */ + ASSERT(sec); + + n = sec->addr; + + if ( sec->sec->sh_size <= sizeof(*n) ) + return -EINVAL; + + rc = xen_build_id_check(n, sec->sec->sh_size, &id->p, &id->len); + if ( rc ) + return rc; + + if ( !id->len || !id->p ) + return -EINVAL; + + return 0; +} + static int check_special_sections(const struct livepatch_elf *elf) { unsigned int i; @@ -641,11 +666,12 @@ static int prepare_payload(struct payload *payload, struct livepatch_elf *elf) { const struct livepatch_elf_sec *sec; + const struct payload *data; unsigned int i; struct livepatch_func *funcs; struct livepatch_func *f; struct virtual_region *region; - const Elf_Note *n; + int rc; sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_FUNC); if ( sec ) @@ -663,8 +689,6 @@ static int prepare_payload(struct payload *payload, for ( i = 0; i < payload->nfuncs; i++ ) { - int rc; - f = &(funcs[i]); if ( f->version != LIVEPATCH_PAYLOAD_VERSION ) @@ -707,69 +731,35 @@ static int prepare_payload(struct payload *payload, LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.action, ELF_LIVEPATCH_REVERT_HOOK); LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.post, ELF_LIVEPATCH_POSTREVERT_HOOK); - sec = livepatch_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE); - if ( sec ) - { - const struct payload *data; - - n = sec->addr; - - if ( sec->sec->sh_size <= sizeof(*n) ) - return -EINVAL; - - if ( xen_build_id_check(n, sec->sec->sh_size, - &payload->id.p, &payload->id.len) ) - return -EINVAL; - - if ( !payload->id.len || !payload->id.p ) - return -EINVAL; + rc = parse_buildid(livepatch_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE), + &payload->id); + if ( rc ) + return rc; - /* Make sure it is not a duplicate. */ - list_for_each_entry ( data, &payload_list, list ) + /* Make sure it is not a duplicate. */ + list_for_each_entry ( data, &payload_list, list ) + { + /* No way _this_ payload is on the list. */ + ASSERT(data != payload); + if ( data->id.len == payload->id.len && + !memcmp(data->id.p, payload->id.p, data->id.len) ) { - /* No way _this_ payload is on the list. */ - ASSERT(data != payload); - if ( data->id.len == payload->id.len && - !memcmp(data->id.p, payload->id.p, data->id.len) ) - { - dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Already loaded as %s!\n", - elf->name, data->name); - return -EEXIST; - } + dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Already loaded as %s!\n", + elf->name, data->name); + return -EEXIST; } } - sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_DEPENDS); - if ( sec ) - { - n = sec->addr; - - if ( sec->sec->sh_size <= sizeof(*n) ) - return -EINVAL; - - if ( xen_build_id_check(n, sec->sec->sh_size, - &payload->dep.p, &payload->dep.len) ) - return -EINVAL; - - if ( !payload->dep.len || !payload->dep.p ) - return -EINVAL; - } - - sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS); - if ( sec ) - { - n = sec->addr; - - if ( sec->sec->sh_size <= sizeof(*n) ) - return -EINVAL; - - if ( xen_build_id_check(n, sec->sec->sh_size, - &payload->xen_dep.p, &payload->xen_dep.len) ) - return -EINVAL; + rc = parse_buildid(livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_DEPENDS), + &payload->dep); + if ( rc ) + return rc; - if ( !payload->xen_dep.len || !payload->xen_dep.p ) - return -EINVAL; - } + rc = parse_buildid(livepatch_elf_sec_by_name(elf, + ELF_LIVEPATCH_XEN_DEPENDS), + &payload->xen_dep); + if ( rc ) + return rc; /* Setup the virtual region with proper data. */ region = &payload->region; From patchwork Thu Sep 26 10:14:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 13813155 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 EF415CCFA1A for ; Thu, 26 Sep 2024 10:21:59 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.805203.1216267 (Exim 4.92) (envelope-from ) id 1stld9-0001uY-9x; Thu, 26 Sep 2024 10:21:51 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 805203.1216267; Thu, 26 Sep 2024 10:21:51 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1stld9-0001uP-6D; Thu, 26 Sep 2024 10:21:51 +0000 Received: by outflank-mailman (input) for mailman id 805203; Thu, 26 Sep 2024 10:21:49 +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 1stld7-0001Nm-2H for xen-devel@lists.xenproject.org; Thu, 26 Sep 2024 10:21:49 +0000 Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [2a00:1450:4864:20::633]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 22c8ae8b-7bf1-11ef-99a2-01e77a169b0f; Thu, 26 Sep 2024 12:21:47 +0200 (CEST) Received: by mail-ej1-x633.google.com with SMTP id a640c23a62f3a-a8d446adf6eso112691666b.2 for ; Thu, 26 Sep 2024 03:21:47 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f5489dsm330891566b.75.2024.09.26.03.21.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:21:46 -0700 (PDT) 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: 22c8ae8b-7bf1-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1727346106; x=1727950906; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=TMV7sv85uJEHqRmzmClkVSBMQ6SQAz9qB3gfIkmcOdA=; b=YiE2Vp+U4LIJ78Og8lxnsu5Ip/qLjeKL171zoiSiAXacv5qvhlbSjuk+p9O1ISuyXD Ni504QrodzNYYKcy1Yb3mILdH+axCkrlB63UJewr8mNWf885JWnl9KQfixoi0iztyBtu 02NUaxqbG/spobpcUeZaARNVa1U6gdNEbSMts= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727346106; x=1727950906; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TMV7sv85uJEHqRmzmClkVSBMQ6SQAz9qB3gfIkmcOdA=; b=Edus3jaqjIKCMHhATbc0R4wCAGzJQqdiFB4EvwBZmoZLfSZc7lpi/b810pBOPidkPb anBJIq1LT4lgPE8p994+2r078m1a9V4gNygOPJ8QzDV8tCSYPH9dxG0DBsf9UZWSKH+C Xqo6Cc3bIWKG/QIoIvWYvx1NU/RYdDDDae7BNhyXDIPmD0pk2Lqb168gzWRbizcc7yMF vIGy0jHtvNf968gLQ5FBegFaV9YwshEouVs2QFIDhzQJzGo/17Mj8VlkSGogE1546eQl 1Y3JmtWvwj4bGLV8rSXBJj43+K5DnH4vdFW/IkagsRDMjkiMG0iOM8vW9AXb2DWLzZcn vefg== X-Gm-Message-State: AOJu0YwgIGOpqcWrEBuTlgJ0DaU3ZepeY7kxPubVjT24nwDEeQSmBvqX qza9ktITSQeHccRsvY+ebBpDIZ9kspH4L0dAwrhaqCFLBnQg1jCJO6Wcz6y8e/WiHF2JZwlpcbt d X-Google-Smtp-Source: AGHT+IGdLpUWffGqwTkOPLSM37brc+qOUsph25Y3A0BhmHbiDsclnm68sZ/sRfzvWI6JtjzVFsE27g== X-Received: by 2002:a17:907:ea8:b0:a8a:9246:7f57 with SMTP id a640c23a62f3a-a93a0341a1cmr582888466b.4.1727346106447; Thu, 26 Sep 2024 03:21:46 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Ross Lagerwall Subject: [PATCH v3 3/5] xen/livepatch: do Xen build-id check earlier Date: Thu, 26 Sep 2024 12:14:29 +0200 Message-ID: <20240926101431.97444-4-roger.pau@citrix.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240926101431.97444-1-roger.pau@citrix.com> References: <20240926101431.97444-1-roger.pau@citrix.com> MIME-Version: 1.0 The check against the expected Xen build ID should be done ahead of attempting to apply the alternatives contained in the livepatch. If the CPUID in the alternatives patching data is out of the scope of the running Xen featureset the BUG() in _apply_alternatives() will trigger thus bringing the system down. Note the layout of struct alt_instr could also change between versions. It's also possible for struct exception_table_entry to have changed format, hence leading to other kind of errors if parsing of the payload is done ahead of checking if the Xen build-id matches. Move the Xen build ID check as early as possible. To do so introduce a new check_xen_buildid() function that parses and checks the Xen build-id before moving the payload. Since the expected Xen build-id is used early to detect whether the livepatch payload could be loaded, there's no reason to store it in the payload struct, as a non-matching Xen build-id won't get the payload populated in the first place. Note printing the expected Xen build ID has part of dumping the payload information is no longer done: all loaded payloads would have Xen build IDs matching the running Xen, otherwise they would have failed to load. Fixes: 879615f5db1d ('livepatch: Always check hypervisor build ID upon livepatch upload') Signed-off-by: Roger Pau Monné Reviewed-by: Andrew Cooper Reviewed-by: Ross Lagerwall --- Changes since v2: - Move contents of xen_build_id_dep() into check_xen_buildid(). Changes since v1: - Do the Xen build-id check even earlier. --- xen/common/livepatch.c | 86 +++++++++++++++++------------ xen/include/xen/livepatch_payload.h | 1 - 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 50e2268e19a3..f7db4be96e66 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -448,28 +448,6 @@ static bool section_ok(const struct livepatch_elf *elf, return true; } -static int xen_build_id_dep(const struct payload *payload) -{ - const void *id = NULL; - unsigned int len = 0; - int rc; - - ASSERT(payload->xen_dep.len); - ASSERT(payload->xen_dep.p); - - rc = xen_build_id(&id, &len); - if ( rc ) - return rc; - - if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) { - printk(XENLOG_ERR LIVEPATCH "%s: check against hypervisor build-id failed\n", - payload->name); - return -EINVAL; - } - - return 0; -} - /* Parses build-id sections into the given destination. */ static int parse_buildid(const struct livepatch_elf_sec *sec, struct livepatch_build_id *id) @@ -495,11 +473,56 @@ static int parse_buildid(const struct livepatch_elf_sec *sec, return 0; } +static int check_xen_buildid(const struct livepatch_elf *elf) +{ + const void *id; + unsigned int len; + struct livepatch_build_id lp_id; + const struct livepatch_elf_sec *sec = + livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS); + int rc; + + if ( !sec ) + { + printk(XENLOG_ERR LIVEPATCH "%s: section %s is missing\n", + elf->name, ELF_LIVEPATCH_XEN_DEPENDS); + return -EINVAL; + } + + rc = parse_buildid(sec, &lp_id); + if ( rc ) + { + printk(XENLOG_ERR LIVEPATCH + "%s: failed to parse section %s as build-id: %d\n", + elf->name, ELF_LIVEPATCH_XEN_DEPENDS, rc); + return -EINVAL; + } + + rc = xen_build_id(&id, &len); + if ( rc ) + { + printk(XENLOG_ERR LIVEPATCH + "%s: unable to get running Xen build-id: %d\n", + elf->name, rc); + return rc; + } + + if ( lp_id.len != len || memcmp(id, lp_id.p, len) ) + { + printk(XENLOG_ERR LIVEPATCH "%s: build-id mismatch:\n" + "  livepatch: %*phN\n" + "        xen: %*phN\n", + elf->name, lp_id.len, lp_id.p, len, id); + return -EINVAL; + } + + return 0; +} + static int check_special_sections(const struct livepatch_elf *elf) { unsigned int i; static const char *const names[] = { ELF_LIVEPATCH_DEPENDS, - ELF_LIVEPATCH_XEN_DEPENDS, ELF_BUILD_ID_NOTE}; for ( i = 0; i < ARRAY_SIZE(names); i++ ) @@ -755,12 +778,6 @@ static int prepare_payload(struct payload *payload, if ( rc ) return rc; - rc = parse_buildid(livepatch_elf_sec_by_name(elf, - ELF_LIVEPATCH_XEN_DEPENDS), - &payload->xen_dep); - if ( rc ) - return rc; - /* Setup the virtual region with proper data. */ region = &payload->region; @@ -1069,6 +1086,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len) if ( rc ) goto out; + rc = check_xen_buildid(&elf); + if ( rc ) + goto out; + rc = move_payload(payload, &elf); if ( rc ) goto out; @@ -1093,10 +1114,6 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len) if ( rc ) goto out; - rc = xen_build_id_dep(payload); - if ( rc ) - goto out; - rc = build_symbol_table(payload, &elf); if ( rc ) goto out; @@ -2199,9 +2216,6 @@ static void cf_check livepatch_printall(unsigned char key) if ( data->dep.len ) printk("depend-on=%*phN\n", data->dep.len, data->dep.p); - - if ( data->xen_dep.len ) - printk("depend-on-xen=%*phN\n", data->xen_dep.len, data->xen_dep.p); } spin_unlock(&payload_lock); diff --git a/xen/include/xen/livepatch_payload.h b/xen/include/xen/livepatch_payload.h index 472d6a4a63c1..c6dc7cb5fa21 100644 --- a/xen/include/xen/livepatch_payload.h +++ b/xen/include/xen/livepatch_payload.h @@ -62,7 +62,6 @@ struct payload { unsigned int nsyms; /* Nr of entries in .strtab and symbols. */ struct livepatch_build_id id; /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */ struct livepatch_build_id dep; /* ELFNOTE_DESC(.livepatch.depends). */ - struct livepatch_build_id xen_dep; /* ELFNOTE_DESC(.livepatch.xen_depends). */ livepatch_loadcall_t *const *load_funcs; /* The array of funcs to call after */ livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the payload. */ struct livepatch_hooks hooks; /* Pre and post hooks for apply and revert */ From patchwork Thu Sep 26 10:14:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 13813156 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 DEA6ECCFA19 for ; Thu, 26 Sep 2024 10:21:59 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.805204.1216270 (Exim 4.92) (envelope-from ) id 1stld9-0001xI-JJ; Thu, 26 Sep 2024 10:21:51 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 805204.1216270; Thu, 26 Sep 2024 10:21:51 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1stld9-0001vw-E6; Thu, 26 Sep 2024 10:21:51 +0000 Received: by outflank-mailman (input) for mailman id 805204; Thu, 26 Sep 2024 10:21:50 +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 1stld8-0001Nm-5k for xen-devel@lists.xenproject.org; Thu, 26 Sep 2024 10:21:50 +0000 Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [2a00:1450:4864:20::52f]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 237aee3b-7bf1-11ef-99a2-01e77a169b0f; Thu, 26 Sep 2024 12:21:48 +0200 (CEST) Received: by mail-ed1-x52f.google.com with SMTP id 4fb4d7f45d1cf-5c5cc65a8abso703067a12.3 for ; Thu, 26 Sep 2024 03:21:48 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f342b2sm332765566b.35.2024.09.26.03.21.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:21:47 -0700 (PDT) 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: 237aee3b-7bf1-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1727346108; x=1727950908; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=APxF9Bl1InkMM9iMqBKqPkfFhMVAK4FzRCdwpSCpgSI=; b=LLfsei3ZSuvzxybLHuct7NkEBdsx8w9YQ0FP3k68a0BB70ySA1j3bxXHQmGxil8jnm eqhzndLXTzGA1+OT5h4WRPxyM7ECltej8vn7/XMrsk6DqIUkgA0Vy6LNZgvScYnP7fx4 Uf82aCJ6KixBWsznTwvg6wH+L4i+V/6DvHe/U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727346108; x=1727950908; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=APxF9Bl1InkMM9iMqBKqPkfFhMVAK4FzRCdwpSCpgSI=; b=EYmouOv9LuB2BMhiDcJD8SJwOf5l/FY9r8bC+FO4LIdzOYtS477wK2SxHB0YfkMXa3 ltv9IMK9sT6j6uhWhYywEFVuLuOYrufG0K/Qm73StjJcjBsLkYZgfUEfcPKRA0KC0DY9 +ofj60YMGIdvulOdYQXAl8dL0HPytYcB8EzIjFS04K3SJbtXYR9WL2vNJW0v6o6EVQL8 qbsljQBHoATXDOAIJVgempzbRFuXXSodjRqI8EthAgmW+WmPX08gNvYgS3cAPjtn7JOh Y2o4Nbp2a6tPolTBZUtAXTJOrdAzkLbO4pCTY82xnlBh/0sNhlLQm+RtDcDPZ34wL69U GA2Q== X-Gm-Message-State: AOJu0YyBmN+y5UHQ1cExtitQAFEp2GliKaE9n0+DLKj6Ih89xOaBrejS HxT2ybO2o4/BfQ3bijCmOP+QRiJeFqGPXqXo7a85ldLWCAEM5DSNjJ+BclLZN2nWi3MM2FJC1Z6 Z X-Google-Smtp-Source: AGHT+IEpnI5inZgEGWW/leVgs7iqBEFDMfSTnMsBkyRIvPpmzzNgAKGYoFC3AMcGgvdgENCgr1jRyA== X-Received: by 2002:a17:907:940d:b0:a8d:3338:a492 with SMTP id a640c23a62f3a-a93a031e15fmr520438866b.7.1727346107669; Thu, 26 Sep 2024 03:21:47 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Jan Beulich , Andrew Cooper , Ross Lagerwall Subject: [PATCH v3 4/5] x86/alternatives: do not BUG during apply Date: Thu, 26 Sep 2024 12:14:30 +0200 Message-ID: <20240926101431.97444-5-roger.pau@citrix.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240926101431.97444-1-roger.pau@citrix.com> References: <20240926101431.97444-1-roger.pau@citrix.com> MIME-Version: 1.0 alternatives is used both at boot time, and when loading livepatch payloads. While for the former it makes sense to panic, it's not useful for the later, as for livepatches it's possible to fail to load the livepatch if alternatives cannot be resolved and continue operating normally. Relax the BUGs in _apply_alternatives() to instead return an error code. The caller will figure out whether the failures are fatal and panic. Print an error message to provide some user-readable information about what went wrong. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Andrew Cooper --- Changes since v2: - Improve log messages. Changes since v1: - Unconditionally return from _apply_alternative() and let the caller panic if required. - Remove test, as next patch imposes restrictions that break the test. --- xen/arch/x86/alternative.c | 46 ++++++++++++++++++++------ xen/arch/x86/include/asm/alternative.h | 2 +- xen/common/livepatch.c | 10 +++++- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 7824053c9d33..990b7c600932 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -175,9 +175,9 @@ extern void *const __initdata_cf_clobber_end[]; * invocation, such that no CALLs/JMPs to NULL pointers will be left * around. See also the further comment below. */ -static void init_or_livepatch _apply_alternatives(struct alt_instr *start, - struct alt_instr *end, - bool force) +static int init_or_livepatch _apply_alternatives(struct alt_instr *start, + struct alt_instr *end, + bool force) { struct alt_instr *a, *base; @@ -198,9 +198,29 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start, uint8_t buf[MAX_PATCH_LEN]; unsigned int total_len = a->orig_len + a->pad_len; - BUG_ON(a->repl_len > total_len); - BUG_ON(total_len > sizeof(buf)); - BUG_ON(a->cpuid >= NCAPINTS * 32); + if ( a->repl_len > total_len ) + { + printk(XENLOG_ERR + "alt for %ps, replacement size %#x larger than origin %#x\n", + ALT_ORIG_PTR(a), a->repl_len, total_len); + return -ENOSPC; + } + + if ( total_len > sizeof(buf) ) + { + printk(XENLOG_ERR + "alt for %ps, origin size %#x bigger than buffer %#zx\n", + ALT_ORIG_PTR(a), total_len, sizeof(buf)); + return -ENOSPC; + } + + if ( a->cpuid >= NCAPINTS * 32 ) + { + printk(XENLOG_ERR + "alt for %ps, feature %#x outside of featureset range %#x\n", + ALT_ORIG_PTR(a), a->cpuid, NCAPINTS * 32); + return -ERANGE; + } /* * Detect sequences of alt_instr's patching the same origin site, and @@ -356,12 +376,14 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start, printk("altcall: Optimised away %u endbr64 instructions\n", clobbered); } + + return 0; } #ifdef CONFIG_LIVEPATCH -void apply_alternatives(struct alt_instr *start, struct alt_instr *end) +int apply_alternatives(struct alt_instr *start, struct alt_instr *end) { - _apply_alternatives(start, end, true); + return _apply_alternatives(start, end, true); } #endif @@ -383,6 +405,8 @@ static int __init cf_check nmi_apply_alternatives( */ if ( !(alt_done & alt_todo) ) { + int rc; + /* * Relax perms on .text to be RWX, so we can modify them. * @@ -394,8 +418,10 @@ static int __init cf_check nmi_apply_alternatives( PAGE_HYPERVISOR_RWX); flush_local(FLUSH_TLB_GLOBAL); - _apply_alternatives(__alt_instructions, __alt_instructions_end, - alt_done); + rc = _apply_alternatives(__alt_instructions, __alt_instructions_end, + alt_done); + if ( rc ) + panic("Unable to apply alternatives: %d\n", rc); /* * Reinstate perms on .text to be RX. This also cleans out the dirty diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h index a86eadfaecbd..69555d781ef9 100644 --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -24,7 +24,7 @@ struct __packed alt_instr { extern void add_nops(void *insns, unsigned int len); /* Similar to alternative_instructions except it can be run with IRQs enabled. */ -extern void apply_alternatives(struct alt_instr *start, struct alt_instr *end); +extern int apply_alternatives(struct alt_instr *start, struct alt_instr *end); extern void alternative_instructions(void); extern void alternative_branches(void); diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index f7db4be96e66..6793cb60d1e2 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -888,7 +888,15 @@ static int prepare_payload(struct payload *payload, return -EINVAL; } } - apply_alternatives(start, end); + + rc = apply_alternatives(start, end); + if ( rc ) + { + printk(XENLOG_ERR LIVEPATCH "%s applying alternatives failed: %d\n", + elf->name, rc); + return rc; + } + alt_done:; #else printk(XENLOG_ERR LIVEPATCH "%s: We don't support alternative patching\n", From patchwork Thu Sep 26 10:14:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Roger_Pau_Monn=C3=A9?= X-Patchwork-Id: 13813158 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 D4A90CCFA13 for ; Thu, 26 Sep 2024 10:22:01 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.805205.1216287 (Exim 4.92) (envelope-from ) id 1stldA-0002Q3-US; Thu, 26 Sep 2024 10:21:52 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 805205.1216287; Thu, 26 Sep 2024 10:21:52 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1stldA-0002P3-Q4; Thu, 26 Sep 2024 10:21:52 +0000 Received: by outflank-mailman (input) for mailman id 805205; Thu, 26 Sep 2024 10:21:51 +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 1stld9-0001Nm-9o for xen-devel@lists.xenproject.org; Thu, 26 Sep 2024 10:21:51 +0000 Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [2a00:1450:4864:20::531]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 24352dd8-7bf1-11ef-99a2-01e77a169b0f; Thu, 26 Sep 2024 12:21:49 +0200 (CEST) Received: by mail-ed1-x531.google.com with SMTP id 4fb4d7f45d1cf-5c5c3a1f474so794922a12.0 for ; Thu, 26 Sep 2024 03:21:49 -0700 (PDT) Received: from localhost ([213.195.124.163]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c5cf48c2e1sm2934235a12.16.2024.09.26.03.21.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:21:48 -0700 (PDT) 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: 24352dd8-7bf1-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=citrix.com; s=google; t=1727346109; x=1727950909; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ywZGPtsIfa0h17GkvBRUIsYKnFHzO/XQjadqj7/ZxH8=; b=SX+gbVyyzHlceh4E64MVs2KN4rNoUvL+XcaAdOoqqOrY2l9jPZlcBh1QKUD+mQmzRh /+/0sTOgZhWIbs2FKpRXRJqUD0l3w9VXAsgIPwdI1vcIb+He6HUWeO33/p8xBbFB68yp JKYVtvpYxZSE2OEtopgh18zkQfkDXVMN7tJeE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727346109; x=1727950909; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ywZGPtsIfa0h17GkvBRUIsYKnFHzO/XQjadqj7/ZxH8=; b=CfV31MWnc8DaRSK6BkFqHz8sN1/zC7zeOgz3yXYe3U9uCV6S4HwVOPzCvHlinc2IwO Za2zpCe7Jq0MyWDJq9mBPeYaOVFqdZAdQ6CsgWAMA/5whtJyGn30PvK5aO29Vk1+sKHc 2VGxhWQOiGja5SI4HFzLpH8rSAdZ7CgLs/zW2pFlf3sL2Qeqs8KD7la1q9aceikqnE5Q aUdD+7H5PYEMECrdetLtjz4ivzEF9C9C82jT/SteFRwfZomQhFt+MBRIYmtlbeEY4NcE 4mcUrjn7RQKmJLNCWTT8zhhczIglpH1eQd434/vSMZmgVKPopoS+3432vYNKHnEBHnBU bgqQ== X-Gm-Message-State: AOJu0YyeqdY7yQCOQeIMlUsjKuABoQK2t3iI4dzQBkTIv+MVVtrAK5IQ SEOQOkG5YdoKZtHBcLxwy1HUf6JxaIQSSU9UNiZort1qIvulcfj/8hc9tPDYhY4ZCQ6KKtsFKj7 C X-Google-Smtp-Source: AGHT+IGnFTZi2ZuHArJb2u9Y1LXrvap6o8KJDZdB8pC7pVyxINPY0snPdKghJfyCQIj5P7u4QlTU0A== X-Received: by 2002:a05:6402:2788:b0:5c7:2131:5d3 with SMTP id 4fb4d7f45d1cf-5c721310723mr3896532a12.12.1727346108796; Thu, 26 Sep 2024 03:21:48 -0700 (PDT) From: Roger Pau Monne To: xen-devel@lists.xenproject.org Cc: Roger Pau Monne , Jan Beulich , Andrew Cooper Subject: [PATCH v3 5/5] x86/alternative: build time check feature is in range Date: Thu, 26 Sep 2024 12:14:31 +0200 Message-ID: <20240926101431.97444-6-roger.pau@citrix.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240926101431.97444-1-roger.pau@citrix.com> References: <20240926101431.97444-1-roger.pau@citrix.com> MIME-Version: 1.0 Ensure at build time the feature(s) used for the alternative blocks are in range of the featureset. No functional change intended, as all current usages are correct. Signed-off-by: Roger Pau Monné Reviewed-by: Andrew Cooper --- Changes since v2: - s/__stringify/STR/. Changes since v1: - New in this version. --- xen/arch/x86/include/asm/alternative-asm.h | 3 +++ xen/arch/x86/include/asm/alternative.h | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/xen/arch/x86/include/asm/alternative-asm.h b/xen/arch/x86/include/asm/alternative-asm.h index 4092f5ba70a6..83e8594f0eaf 100644 --- a/xen/arch/x86/include/asm/alternative-asm.h +++ b/xen/arch/x86/include/asm/alternative-asm.h @@ -12,6 +12,9 @@ * instruction. See apply_alternatives(). */ .macro altinstruction_entry orig, repl, feature, orig_len, repl_len, pad_len + .if \feature >= NCAPINTS * 32 + .error "alternative feature outside of featureset range" + .endif .long \orig - . .long \repl - . .word \feature diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h index 69555d781ef9..38472fb58e2d 100644 --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -7,6 +7,7 @@ #include #include #include +#include struct __packed alt_instr { int32_t orig_offset; /* original instruction */ @@ -59,6 +60,9 @@ extern void alternative_branches(void); alt_repl_len(n2)) "-" alt_orig_len) #define ALTINSTR_ENTRY(feature, num) \ + " .if " STR(feature) " >= " STR(NCAPINTS * 32) "\n" \ + " .error \"alternative feature outside of featureset range\"\n" \ + " .endif\n" \ " .long .LXEN%=_orig_s - .\n" /* label */ \ " .long " alt_repl_s(num)" - .\n" /* new instruction */ \ " .word " __stringify(feature) "\n" /* feature bit */ \