From patchwork Tue Jan 5 09:18:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 59172 Delivered-To: patch@linaro.org Received: by 10.112.130.2 with SMTP id oa2csp5855391lbb; Tue, 5 Jan 2016 01:20:38 -0800 (PST) X-Received: by 10.66.158.169 with SMTP id wv9mr131905478pab.138.1451985638404; Tue, 05 Jan 2016 01:20:38 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id hs7si50299721pad.141.2016.01.05.01.20.38 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Jan 2016 01:20:38 -0800 (PST) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dkim=neutral (body hash did not verify) header.i=@linaro.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aGNm5-0002Xg-Uy; Tue, 05 Jan 2016 09:19:29 +0000 Received: from mail-wm0-x22c.google.com ([2a00:1450:400c:c09::22c]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aGNm2-0002Ua-0z for linux-arm-kernel@lists.infradead.org; Tue, 05 Jan 2016 09:19:27 +0000 Received: by mail-wm0-x22c.google.com with SMTP id f206so15114567wmf.0 for ; Tue, 05 Jan 2016 01:19:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=Pa7V4Ex+j+wg1ND9uCGmDCjEGh1BfMf5C/EZVPwI2wY=; b=cfxM+t5UcYCBf0RMxznoZf9YIuHElsh0Oqzj60YBb0jgTWNxYObm+4QDt8gJYYPA6w 1AcGUOP2pXPPWpvpOZsT+Y2tLn8/e6b7kxJ/Y6z5RjMYpjd3fFcoMRaV/cc4UJ0HEfI9 xKlu5Tig00iFfD9W5r7ya+AM9dxlhrEQpw9b8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=Pa7V4Ex+j+wg1ND9uCGmDCjEGh1BfMf5C/EZVPwI2wY=; b=NAefOZy0/mt/aoKdVUkeWul3kJ4SwJHyY4GQy25tLAPWx3YoIi4rK4sDxP1KKyyKo/ FhMm0aVyhecJ/CnIiPr1pnyi/8U6MuGARg7osvVx3mzbyzL/ZDeE+4b8xqraM9WvzGeg KliqoPyZV5fpCUNScDA4xAYP2+mzVkZh/OXSUzV040tiWs4IXMH4XjJ51zeTcp/iES61 XOllIRVTZUkSPCQ4gGS1fEIzPwKwtEP+2llj6AfZpbXOyutKQWuMf8PpFRzB+0tUjibe c9XUuRamhKDZ0ZMZ3jIvg72q2mw7Q2QpzN5uPGkjfWO+rd1lyG8Ex7eLggxRIqMg70jx DUkA== X-Gm-Message-State: ALoCoQkQQIvLhwvL1KEITKxHbeWleZaJzNORevXgwwcHQIyJEtkBXCBAO8E5wES8nyC8Gi4+WjdCR4ICZIc7I55lONs0LuKnwQ== X-Received: by 10.28.232.221 with SMTP id f90mr2836540wmi.60.1451985544634; Tue, 05 Jan 2016 01:19:04 -0800 (PST) Received: from localhost.localdomain (cag06-7-83-153-85-71.fbx.proxad.net. [83.153.85.71]) by smtp.gmail.com with ESMTPSA id c203sm2550612wmd.5.2016.01.05.01.19.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 05 Jan 2016 01:19:04 -0800 (PST) From: Ard Biesheuvel To: linux-arm-kernel@lists.infradead.org, will.deacon@arm.com, Dave.Martin@arm.com Subject: [PATCH v2 1/2] arm64/module: fix relocation of movz instruction with negative immediate Date: Tue, 5 Jan 2016 10:18:51 +0100 Message-Id: <1451985532-6487-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.5.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160105_011926_233181_04EE8DBF X-CRM114-Status: GOOD ( 19.24 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2a00:1450:400c:c09:0:0:0:22c listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org The test whether a movz instruction with a signed immediate should be turned into a movn instruction (i.e., when the immediate is negative) is flawed, since the value of imm is always positive. Also, the subsequent bounds check is incorrect since the limit update never executes, due to the fact that the imm_type comparison will always be false for negative signed immediates. Let's fix this by performing the sign test on sval directly, and replacing the bounds check with a simple comparison against U16_MAX. Signed-off-by: Ard Biesheuvel --- arch/arm64/kernel/module.c | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-) -- 2.5.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index f4bc779e62e8..400141549b28 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -30,9 +30,6 @@ #include #include -#define AARCH64_INSN_IMM_MOVNZ AARCH64_INSN_IMM_MAX -#define AARCH64_INSN_IMM_MOVK AARCH64_INSN_IMM_16 - void *module_alloc(unsigned long size) { void *p; @@ -110,16 +107,21 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len) return 0; } +enum aarch64_insn_movw_imm_type { + AARCH64_INSN_IMM_MOVNZ, + AARCH64_INSN_IMM_MOVK +}; + static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, - int lsb, enum aarch64_insn_imm_type imm_type) + int lsb, enum aarch64_insn_movw_imm_type imm_type) { - u64 imm, limit = 0; + u64 imm; s64 sval; u32 insn = le32_to_cpu(*(u32 *)place); sval = do_reloc(op, place, val); sval >>= lsb; - imm = sval & 0xffff; + imm = sval; if (imm_type == AARCH64_INSN_IMM_MOVNZ) { /* @@ -128,7 +130,7 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, * immediate is less than zero. */ insn &= ~(3 << 29); - if ((s64)imm >= 0) { + if (sval >= 0) { /* >=0: Set the instruction to MOVZ (opcode 10b). */ insn |= 2 << 29; } else { @@ -138,31 +140,15 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, * don't need to do anything other than * inverting the new immediate field. */ - imm = ~imm; + imm = ~sval; } - imm_type = AARCH64_INSN_IMM_MOVK; } /* Update the instruction with the new encoding. */ - insn = aarch64_insn_encode_immediate(imm_type, insn, imm); + insn = aarch64_insn_encode_immediate(AARCH64_INSN_IMM_16, insn, imm); *(u32 *)place = cpu_to_le32(insn); - /* Shift out the immediate field. */ - sval >>= 16; - - /* - * For unsigned immediates, the overflow check is straightforward. - * For signed immediates, the sign bit is actually the bit past the - * most significant bit of the field. - * The AARCH64_INSN_IMM_16 immediate type is unsigned. - */ - if (imm_type != AARCH64_INSN_IMM_16) { - sval++; - limit++; - } - - /* Check the upper bits depending on the sign of the immediate. */ - if ((u64)sval > limit) + if (imm > U16_MAX) return -ERANGE; return 0; @@ -267,25 +253,25 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, overflow_check = false; case R_AARCH64_MOVW_UABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0, - AARCH64_INSN_IMM_16); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_UABS_G1_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G1: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16, - AARCH64_INSN_IMM_16); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_UABS_G2_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G2: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32, - AARCH64_INSN_IMM_16); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_UABS_G3: /* We're using the top bits so we can't overflow. */ overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 48, - AARCH64_INSN_IMM_16); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_SABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0,