From patchwork Mon Sep 21 11:51:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Anderson X-Patchwork-Id: 1368301 X-Patchwork-Delegate: uboot@andestech.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=85.214.62.61; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=MQdoalvw; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Bw2rX477Jz9sV0 for ; Mon, 21 Sep 2020 21:53:16 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9019D8256E; Mon, 21 Sep 2020 13:52:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="MQdoalvw"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ED05882553; Mon, 21 Sep 2020 13:52:05 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,SPF_HELO_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-qt1-x844.google.com (mail-qt1-x844.google.com [IPv6:2607:f8b0:4864:20::844]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 552AB8255A for ; Mon, 21 Sep 2020 13:52:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qt1-x844.google.com with SMTP id n18so11952118qtw.0 for ; Mon, 21 Sep 2020 04:52:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=YhBjPlwgJmlQb6EdQgAWamJfc/azOzOm0WzdY4tCn4k=; b=MQdoalvw34pwvNkgU12uRYdb0zrGBo6r4SPvo4FHg5wKJhXdu6PjQc6ADSdxTEQmVd y+4Y83yO4NEkz+XHgOEBv6hbWOI1SO/C8hylxFs1xIS1iXqTwwF2XXghLpuDUN+Znvem mHSwe+OAkCv0K+KxLZYFkhGWG/LU++xNJaAXx6myE+b6qEmGf/FaDBXcmy6Z/1O8rDyx YjNoft1VTSdN/4Wwrq0L36Nwg5wxBAhi9BMjcSQ+9TyBI/pCRMbu1dlNsjS8u90HkmDL oPQbU8XZoIoi/NR+ybZfKQDRv6VD6QRjv6zShrJf5KhF/zbb/57/yv71rPpvRQmcm4Bb dyag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YhBjPlwgJmlQb6EdQgAWamJfc/azOzOm0WzdY4tCn4k=; b=J+5fULoeEa9yvlPB3KZFLtr5vf9BrB7w8zKUQUPWx3eES8UkkYTl+GKYhZLMnYQ1Hs e0USyx7u4he9CJ6MAzY+N8WJFWmNUVKiBAyGRTPZrD01093guS60+uhOrbSewCrPUI2y h/fmcHDbz67vy8fupz4x8RezPYhw4qehewiNFIM++kG7uzuKXQOE9J5+LO+fH550Qlay I29Pb860JXZcenZZODIZY0TbWCM5V0lBdVCUDZzbTdcuLmpSSeWZislH7c2TNdpd3sMJ OcO/AGm+ALdJs8G0gpUlqspr2w/StUCDX+9CIgtNZT9Qu9/FhLMdVef+3wSIhM29W6Go rJyg== X-Gm-Message-State: AOAM532/oChaG+FCE0sNyFajNj60x/oMycMehHu3TnQz6hrGe8ySct+e jHwXMBB5SXFocyCIy+9Ua6SQnQAual0c7A== X-Google-Smtp-Source: ABdhPJzrHuXFYK7o/Oa50qr5gAt3c+FvCnn8ay+uZflfruqzydsXoIO9MBEO88S4cgoTZGspBIA43Q== X-Received: by 2002:ac8:39a7:: with SMTP id v36mr364316qte.140.1600689120940; Mon, 21 Sep 2020 04:52:00 -0700 (PDT) Received: from godwin.fios-router.home (pool-108-51-35-162.washdc.fios.verizon.net. [108.51.35.162]) by smtp.gmail.com with ESMTPSA id c70sm8658105qkg.4.2020.09.21.04.51.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Sep 2020 04:52:00 -0700 (PDT) From: Sean Anderson To: u-boot@lists.denx.de Cc: Heinrich Schuchardt , Rick Chen , alankao@andestech.com, Leo Liang , Bin Meng , Sean Anderson , Bin Meng Subject: [PATCH v3 6/7] riscv: Ensure gp is NULL or points to valid data Date: Mon, 21 Sep 2020 07:51:40 -0400 Message-Id: <20200921115141.70598-7-seanga2@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200921115141.70598-1-seanga2@gmail.com> References: <20200921115141.70598-1-seanga2@gmail.com> MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.3 at phobos.denx.de X-Virus-Status: Clean This ensures constructs like `if (gd & gd->...) { ... }` work when accessing the global data pointer. Without this change, it was possible for a very early trap to cause _exit_trap to directly or indirectly (through printf) to read arbitrary memory. This could cause a second trap, preventing show_regs from being printed. printf (and specifically puts) uses gd to determine what function to print with. These functions in turn use gd to find the serial device, etc. However, before accessing gd, puts first checks to see if it is non-NULL. This indicates an existing (perhaps undocumented) assumption that either gd is NULL or it is completely valid. Before this patch, gd either points to unexpected data (because it retains the value it did from the prior-stage) or points to uninitialized data (because it has not yet been initialized by board_init_f_init_reserve) until the hart has acquired available_harts_lock. This can cause two problems, depending on the value of gd->flags. If GD_FLG_SERIAL_READY is unset, then some garbage data will be printed to stdout, but there will not be a second trap. However, if GD_FLG_SERIAL_READY is set, then puts will try to print with serial_puts, which will likely cause a second trap. After this patch, gd is zero up until either a hart has set it in wait_for_gd_init, or until it is set by arch_init_gd. This prevents its usage before its data is initialized because both handle_trap and puts ensure that gd is nonzero before using it. After gd has been set, it is OK to access it because its data has been cleared (and so flags is valid). XIP cannot use locks because flash is not writable. This leaves it vulnerable to the same class of bugs regarding already-pending IPIs as before this series. Fixing that would require finding another method of synchronization, which is outside the scope of this series. Fixes: 7c6ca03eae ("riscv: additional crash information") Signed-off-by: Sean Anderson Reviewed-by: Bin Meng Reviewed-by: Rick Chen --- Changes in v3: - Clarify XIP comment Changes in v2: - Set gp early with XIP arch/riscv/cpu/start.S | 28 +++++++++++++++++++++++++--- arch/riscv/lib/interrupts.c | 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 66ca1c7020..eb852538ca 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -47,6 +47,13 @@ _start: mv tp, a0 mv s1, a1 + /* + * Set the global data pointer to a known value in case we get a very + * early trap. The global data pointer will be set its actual value only + * after it has been initialized. + */ + mv gp, zero + la t0, trap_entry csrw MODE_PREFIX(tvec), t0 @@ -85,10 +92,10 @@ call_board_init_f_0: jal board_init_f_alloc_reserve /* - * Set global data pointer here for all harts, uninitialized at this - * point. + * Save global data pointer for later. We don't set it here because it + * is not initialized yet. */ - mv gp, a0 + mv s0, a0 /* setup stack */ #if CONFIG_IS_ENABLED(SMP) @@ -109,6 +116,14 @@ call_board_init_f_0: amoswap.w s2, t1, 0(t0) bnez s2, wait_for_gd_init #else + /* + * FIXME: gp is set before it is initialized. If an XIP U-Boot ever + * encounters a pending IPI on boot it is liable to jump to whatever + * memory happens to be in ipi_data.addr on boot. It may also run into + * problems if it encounters an exception too early (because printf/puts + * accesses gd). + */ + mv gp, s0 bnez tp, secondary_hart_loop #endif @@ -133,6 +148,13 @@ wait_for_gd_init: 1: amoswap.w.aq t1, t1, 0(t0) bnez t1, 1b + /* + * Set the global data pointer only when gd_t has been initialized. + * This was already set by arch_setup_gd on the boot hart, but all other + * harts' global data pointers gets set here. + */ + mv gp, s0 + /* register available harts in the available_harts mask */ li t1, 1 sll t1, t1, tp diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index cd47e64487..ad870e98d8 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -78,7 +78,8 @@ static void _exit_trap(ulong code, ulong epc, ulong tval, struct pt_regs *regs) printf("EPC: " REG_FMT " RA: " REG_FMT " TVAL: " REG_FMT "\n", epc, regs->ra, tval); - if (gd->flags & GD_FLG_RELOC) + /* Print relocation adjustments, but only if gd is initialized */ + if (gd && gd->flags & GD_FLG_RELOC) printf("EPC: " REG_FMT " RA: " REG_FMT " reloc adjusted\n\n", epc - gd->reloc_off, regs->ra - gd->reloc_off);