From patchwork Fri May 25 16:09:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Jones X-Patchwork-Id: 10427941 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2EA5D602D8 for ; Fri, 25 May 2018 16:26:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3385429773 for ; Fri, 25 May 2018 16:26:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 279AA29778; Fri, 25 May 2018 16:26:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9282B29773 for ; Fri, 25 May 2018 16:26:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Type:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To :Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=bhjqZDfVY/gCv6Mt8divUUH0ulW56DZiUumMRuJ6LQk=; b=TZXOjOnyEmcfUlBwSCMWmAkbJ xpvYmIzXrfOflWW2x3GVvDu9iYi77PZM3ltEDhzSigMW5//neBHFv+UN1CYPaFhp4CArQuPRQhXIo nBicBW0GwC+FgY94Wb0u6nCoz3XtuY8tfnviSFQWKJzPaK6aDy3b/posIUWy/5IEoKTpOQkTA2vH3 dp5bFt87N36SvzYSmZmjiTcrBgbVZY5rYK8ZNpiCeKvrHyyXZnXeoiTChGI5AYZshdP1yrNegzb/H 8nWoSUTmEbSqmXYj+arKh0Hc1Hgenwj2goF1NPM7aHOACIKjcuvTs78uaU+HJOBcrGjL0c4mcUIFJ u2beRuaBQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fMFXF-0006mE-TE; Fri, 25 May 2018 16:25:45 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fMFXD-0006BO-8t for linux-arm-kernel@bombadil.infradead.org; Fri, 25 May 2018 16:25:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=pndqeOrlP/psHzlIwBJIMlvbWIbh7LEgDTrgfZyhc5k=; b=nTMQs1j/7owJqyRl6A8Vv74rQ MQnJfJcAZ1NOCHh+7LR5PXYr3oHl5gMpCW4YWNV7nAlf+EJv8MesxH/9vJP8ZCRLFYuZGEUJqAqiv qH+YrnrfMrMdUsc8i/qHEOclNj4vBkkhUywDc175uVFpfFK3I55qMzL6hbm4V/UcCfKB1EtpIRx8p cuZeuJiqQn9t5uvm7hnfRIqie7yWUya9itF4sjbW6ugiU6Hq7Nuhhs4CSl1mnHBbj87NcNIpLXkXO l6zmlWYHqeEMJwUXSTan+jotkcng+gM74GpJZe2vSv5ziAczDxn6z22CFEmLcnOZd92BXWmYoNzd1 5tZj0a7fA==; Received: from mx3-rdu2.redhat.com ([66.187.233.73] helo=mx1.redhat.com) by casper.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fMFHf-0006zO-Ee for linux-arm-kernel@lists.infradead.org; Fri, 25 May 2018 16:09:41 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A033D4023112; Fri, 25 May 2018 16:09:12 +0000 (UTC) Received: from kamzik.brq.redhat.com (unknown [10.43.2.160]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 99972210C6C1; Fri, 25 May 2018 16:09:11 +0000 (UTC) Date: Fri, 25 May 2018 18:09:09 +0200 From: Andrew Jones To: Marc Zyngier Subject: Re: [PATCH] arm64: vgic-v2: Fix proxying of cpuif access Message-ID: <20180525160909.uykqxtxkjfzc7aqg@kamzik.brq.redhat.com> References: <20180427145102.5645-1-marc.zyngier@arm.com> <20180429123432.GA7512@C02W217FHV2R.local> <20180429140507.60bfe2dc@why.wild-wind.fr.eu.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180429140507.60bfe2dc@why.wild-wind.fr.eu.org> User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 25 May 2018 16:09:12 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 25 May 2018 16:09:12 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'drjones@redhat.com' RCPT:'' X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180525_170939_816684_06EFE02C X-CRM114-Status: GOOD ( 38.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, Christoffer Dall , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Sun, Apr 29, 2018 at 02:05:07PM +0100, Marc Zyngier wrote: > On Sun, 29 Apr 2018 14:34:32 +0200 > Christoffer Dall wrote: > > > On Fri, Apr 27, 2018 at 03:51:02PM +0100, Marc Zyngier wrote: > > > Proxying the cpuif accesses at EL2 makes use of vcpu_data_guest_to_host > > > and co, which check the endianness, which call into vcpu_read_sys_reg... > > > which isn't mapped at EL2 (it was inlined before, and got moved OoL > > > with the VHE optimizations). > > > > I thought we relied on static inline functions to always be inlined, but > > apparently not? Does this mean we have potential other bugs looming > > depending on the mood of the compiler, or was there something special > > that went wrong here? > > We do rely on that behaviour. And that was the case until you moved > vcpu_read_sys_reg() to be entirely out of line (see d47533dab9f5). > > At that point, kvm_vcpu_is_be() becomes a death trap. > > We missed it for two reasons: > - It was only indirectly called, making it quite hard to notice the > potential breakage > - Nobody gives a damn about 64k pages, specially on something like Juno > > What we'd need is a way to find cross-section calls (text -> hyp-text > should allowed, but not the reverse). We already have similar things in > the kernel, it is probably only a matter of reusing the infrastructure > for our own purpose. > Hi all, While debugging a series backported to the RHEL kernel, I was suspicious of the problem being an issue like this (a hyp-text to non-hyp-mapped reference), so I went ahead an implemented the cross-section reference checking as Marc suggested. I've attached the patch. I don't really like it though because of all the special casing necessary to kill false alarms - making it too hacky. Also, the need to inline some functions just to match their symbol names is pretty lame. Anyway, maybe it can be helpful, at least as inspiration. Running it on latest master doesn't produce anything. Running it without b220244d4179 ("arm64: vgic-v2: Fix proxying of cpuif access") gives .hyp.text:__vgic_v2_perform_cpuif_access references .text:vcpu_read_sys_reg so I guess it works. Unfortunately it didn't help me with my downstream debug... drew diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S index bffece27b5c1..7a6364e8312f 100644 --- a/arch/arm64/kvm/hyp/hyp-entry.S +++ b/arch/arm64/kvm/hyp/hyp-entry.S @@ -200,6 +200,7 @@ ENDPROC(\label) invalid_vector el2h_fiq_invalid invalid_vector el1_fiq_invalid +hyp_entry_literal_pool: .ltorg .align 11 diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index d9645236e474..16f063af3bd9 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -86,7 +86,7 @@ static void __hyp_text __deactivate_traps_common(void) write_sysreg(0, pmuserenr_el0); } -static void activate_traps_vhe(struct kvm_vcpu *vcpu) +static noinline void activate_traps_vhe(struct kvm_vcpu *vcpu) { u64 val; @@ -125,7 +125,7 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) __activate_traps_nvhe(vcpu); } -static void deactivate_traps_vhe(void) +static noinline void deactivate_traps_vhe(void) { extern char vectors[]; /* kernel exception vectors */ write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); @@ -529,8 +529,8 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par, read_sysreg(hpfar_el2), par, vcpu); } -static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par, - struct kvm_cpu_context *host_ctxt) +static noinline void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par, + struct kvm_cpu_context *host_ctxt) { struct kvm_vcpu *vcpu; vcpu = host_ctxt->__hyp_running_vcpu; diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 4ff08a0ef5d3..6ba0617df985 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -964,6 +964,7 @@ enum mismatch { ANY_EXIT_TO_ANY_INIT, EXPORT_TO_INIT_EXIT, EXTABLE_TO_NON_TEXT, + HYP_TEXT_TO_NON_HYP, }; /** @@ -1003,6 +1004,11 @@ static void extable_mismatch_handler(const char *modname, struct elf_info *elf, Elf_Rela *r, Elf_Sym *sym, const char *fromsec); +static void hyp_mismatch_handler(const char *modname, struct elf_info *elf, + const struct sectioncheck* const mismatch, + Elf_Rela *r, Elf_Sym *sym, + const char *fromsec); + static const struct sectioncheck sectioncheck[] = { /* Do not reference init/exit code/data from * normal code and data @@ -1090,7 +1096,15 @@ static void extable_mismatch_handler(const char *modname, struct elf_info *elf, .good_tosec = {ALL_TEXT_SECTIONS , NULL}, .mismatch = EXTABLE_TO_NON_TEXT, .handler = extable_mismatch_handler, -} +}, +{ + .fromsec = { ".hyp.text", NULL }, + .good_tosec = { ".hyp.text", ".hyp.idmap.text", + ".rodata", ".data..ro_after_init", + ".bss", NULL }, + .mismatch = HYP_TEXT_TO_NON_HYP, + .handler = hyp_mismatch_handler, +}, }; static const struct sectioncheck *section_mismatch( @@ -1525,6 +1539,10 @@ static void report_sec_mismatch(const char *modname, fatal("There's a special handler for this mismatch type, " "we should never get here."); break; + case HYP_TEXT_TO_NON_HYP: + fatal("There's a special handler for this mismatch type, " + "we should never get here."); + break; } fprintf(stderr, "\n"); } @@ -1681,6 +1699,46 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf, } } +static void hyp_mismatch_handler(const char *modname, struct elf_info *elf, + const struct sectioncheck* const mismatch, + Elf_Rela *r, Elf_Sym *sym, + const char *fromsec) +{ + Elf_Sym *from = find_elf_symbol2(elf, r->r_offset, fromsec); + Elf_Sym *to = find_elf_symbol(elf, r->r_addend, sym); + const char *fromsym = sym_name(elf, from); + const char *tosec = sec_name(elf, get_secindex(elf, sym)); + const char *tosym = sym_name(elf, to); + char *s; + + switch (mismatch->mismatch) { + case HYP_TEXT_TO_NON_HYP: +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) + if (!is_valid_name(elf, from) || !is_valid_name(elf, to)) { + /* + * Skip unknown symbols to reduce false alarms, + * of course at the risk of missing something... + */ + return; + } + if (strcmp(tosym, "kvm_arm_hyp_stack_page") == 0 || + strcmp(tosym, "kvm_host_cpu_state") == 0) + return; + if (strcmp(fromsym, "hyp_entry_literal_pool") == 0) + return; + if ((s = strstr(tosym, "_vhe")) && + (strlen(s) == 4 || s[4] == '.')) + return; + fprintf(stderr, + ".hyp.text:%s references %s:%s\n\n", + fromsym, tosec, tosym); +#endif + break; + default: + break; + } +} + static void check_section_mismatch(const char *modname, struct elf_info *elf, Elf_Rela *r, Elf_Sym *sym, const char *fromsec) {