From patchwork Thu Jan 14 07:33:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zi Shen Lim X-Patchwork-Id: 8030461 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id DF6EDBEEE5 for ; Thu, 14 Jan 2016 07:35:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8E041204A2 for ; Thu, 14 Jan 2016 07:35:55 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 11E6720451 for ; Thu, 14 Jan 2016 07:35:54 +0000 (UTC) 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 1aJcQ5-0000Z8-U1; Thu, 14 Jan 2016 07:34:09 +0000 Received: from mail-pa0-x244.google.com ([2607:f8b0:400e:c03::244]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aJcQ3-0000Xe-6b for linux-arm-kernel@lists.infradead.org; Thu, 14 Jan 2016 07:34:08 +0000 Received: by mail-pa0-x244.google.com with SMTP id yy13so27991191pab.1 for ; Wed, 13 Jan 2016 23:33:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=VndswX94w7o9qA0HO61A/+Jn+Gkfojz0W7VO2CnG/Hk=; b=FEwOuA06isUbwRxIGhDLuzaV6ugeyF4aVlek0FNCpR4nicfDPWqCZT4vySeWRGdwQy k+E8FsfD5ShuAvbTWcZtZxAazwtxWA8qBFgBEcN2oBBGhd3YNaAqW2oDLMQEioWHJuLP 0w9nArezrk6ERjRC5Cw+8+K5T+/7MKgDc5rxRe60nYt+jgD96lB44rUqUlF9w2G8rqg/ qTB7veFT7U7yQFjUgF17aTpemRDoY8aTsUQhX0G4SJtAV55ZMKy8NSa4KVCpA369huIC OJVmQyJ0TIfR/Dd+JwKF6Rsj52HTtG8+Dc0ga/B6KRxFESW/ruWr/65Pd3AQOMY+r8gG Iv6w== 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=VndswX94w7o9qA0HO61A/+Jn+Gkfojz0W7VO2CnG/Hk=; b=daJrwhQzBO8sV+sg4kpVpp6YyMQijMnZzSSfrSyT+igfMpXOK27K3NklfIlTn3MoVK bjIZK5m0zmzVpUbCUIKeehL7YPEOwi4SGI1RCSxay3Zmk5GbAahih2qnwma07XwMcvET 0lUO2fsPilqMV6ZcKKCWj1UTIyXOFgeMRJrEkp1rogIlO8mYYi25kjjc/+RrxsR2VFtf idQL0bfUYgQ3rRdhmv3v155CobhjNbD/1gL12WZXZ8QrAxRyxYf0PuPS+oK7ygCpF25q +MjlPaWzeUGGwOeE6kecdWjBezryraKfTt8+e3brqPHogZWq/vk50Qi2LqdeWXRl8FOq U9zw== X-Gm-Message-State: ALoCoQnlQItgVENET4bIlJZsTs+e3fakqdIWR5KwY7SP/oXakBusqZEe95eQGBwqDKR8JC5WphuP0IpypKcYSMiPJ/6JWzbJBg== X-Received: by 10.66.100.163 with SMTP id ez3mr3823258pab.5.1452756825277; Wed, 13 Jan 2016 23:33:45 -0800 (PST) Received: from localhost.localdomain (c-73-223-118-172.hsd1.ca.comcast.net. [73.223.118.172]) by smtp.gmail.com with ESMTPSA id q27sm7021578pfi.80.2016.01.13.23.33.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 13 Jan 2016 23:33:44 -0800 (PST) From: Zi Shen Lim To: Will Deacon , Alexei Starovoitov , "David S. Miller" , Catalin Marinas Subject: [PATCH 1/2] arm64: insn: remove BUG_ON from codegen Date: Wed, 13 Jan 2016 23:33:21 -0800 Message-Id: <1452756802-16511-1-git-send-email-zlim.lnx@gmail.com> X-Mailer: git-send-email 1.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160113_233407_335553_47D8F63B X-CRM114-Status: GOOD ( 14.50 ) X-Spam-Score: -2.7 (--) 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: Rabin Vincent , Zi Shen Lim , linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP During code generation, we used to BUG_ON unknown/unsupported encoding or invalid parameters. Instead, now we report these as errors and simply return the instruction AARCH64_BREAK_FAULT. Users of these codegen helpers should check for and handle this failure condition as appropriate. Otherwise, unhandled codegen failure will result in trapping at run-time due to AARCH64_BREAK_FAULT, which is arguably better than a BUG_ON. Signed-off-by: Zi Shen Lim Cc: Will Deacon Reviewed-by: Masami Hiramatsu --- Per discussion here: http://www.spinics.net/lists/arm-kernel/msg474179.html arch/arm64/kernel/insn.c | 165 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 112 insertions(+), 53 deletions(-) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index c08b9ad..7371455 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -2,7 +2,7 @@ * Copyright (C) 2013 Huawei Ltd. * Author: Jiang Liu * - * Copyright (C) 2014 Zi Shen Lim + * Copyright (C) 2014-2016 Zi Shen Lim * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -363,6 +363,9 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, u32 immlo, immhi, mask; int shift; + if (insn == AARCH64_BREAK_FAULT) + return AARCH64_BREAK_FAULT; + switch (type) { case AARCH64_INSN_IMM_ADR: shift = 0; @@ -377,7 +380,7 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) { pr_err("aarch64_insn_encode_immediate: unknown immediate encoding %d\n", type); - return 0; + return AARCH64_BREAK_FAULT; } } @@ -394,9 +397,12 @@ static u32 aarch64_insn_encode_register(enum aarch64_insn_register_type type, { int shift; + if (insn == AARCH64_BREAK_FAULT) + return AARCH64_BREAK_FAULT; + if (reg < AARCH64_INSN_REG_0 || reg > AARCH64_INSN_REG_SP) { pr_err("%s: unknown register encoding %d\n", __func__, reg); - return 0; + return AARCH64_BREAK_FAULT; } switch (type) { @@ -417,7 +423,7 @@ static u32 aarch64_insn_encode_register(enum aarch64_insn_register_type type, default: pr_err("%s: unknown register type encoding %d\n", __func__, type); - return 0; + return AARCH64_BREAK_FAULT; } insn &= ~(GENMASK(4, 0) << shift); @@ -446,7 +452,7 @@ static u32 aarch64_insn_encode_ldst_size(enum aarch64_insn_size_type type, break; default: pr_err("%s: unknown size encoding %d\n", __func__, type); - return 0; + return AARCH64_BREAK_FAULT; } insn &= ~GENMASK(31, 30); @@ -460,14 +466,17 @@ static inline long branch_imm_common(unsigned long pc, unsigned long addr, { long offset; - /* - * PC: A 64-bit Program Counter holding the address of the current - * instruction. A64 instructions must be word-aligned. - */ - BUG_ON((pc & 0x3) || (addr & 0x3)); + if ((pc & 0x3) || (addr & 0x3)) { + pr_err("%s: A64 instructions must be word aligned\n", __func__); + return range; + } offset = ((long)addr - (long)pc); - BUG_ON(offset < -range || offset >= range); + + if (offset < -range || offset >= range) { + pr_err("%s: offset out of range\n", __func__); + return range; + } return offset; } @@ -484,6 +493,8 @@ u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, * texts are within +/-128M. */ offset = branch_imm_common(pc, addr, SZ_128M); + if (offset >= SZ_128M) + return AARCH64_BREAK_FAULT; switch (type) { case AARCH64_INSN_BRANCH_LINK: @@ -493,7 +504,7 @@ u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, insn = aarch64_insn_get_b_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown branch encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -510,6 +521,8 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr, long offset; offset = branch_imm_common(pc, addr, SZ_1M); + if (offset >= SZ_1M) + return AARCH64_BREAK_FAULT; switch (type) { case AARCH64_INSN_BRANCH_COMP_ZERO: @@ -519,7 +532,7 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr, insn = aarch64_insn_get_cbnz_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown branch encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -530,7 +543,7 @@ u32 aarch64_insn_gen_comp_branch_imm(unsigned long pc, unsigned long addr, insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -550,7 +563,10 @@ u32 aarch64_insn_gen_cond_branch_imm(unsigned long pc, unsigned long addr, insn = aarch64_insn_get_bcond_value(); - BUG_ON(cond < AARCH64_INSN_COND_EQ || cond > AARCH64_INSN_COND_AL); + if (cond < AARCH64_INSN_COND_EQ || cond > AARCH64_INSN_COND_AL) { + pr_err("%s: unknown condition encoding %d\n", __func__, cond); + return AARCH64_BREAK_FAULT; + } insn |= cond; return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn, @@ -583,7 +599,7 @@ u32 aarch64_insn_gen_branch_reg(enum aarch64_insn_register reg, insn = aarch64_insn_get_ret_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown branch encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -606,7 +622,7 @@ u32 aarch64_insn_gen_load_store_reg(enum aarch64_insn_register reg, insn = aarch64_insn_get_str_reg_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown load/store encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -645,26 +661,30 @@ u32 aarch64_insn_gen_load_store_pair(enum aarch64_insn_register reg1, insn = aarch64_insn_get_stp_post_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown load/store encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - /* offset must be multiples of 4 in the range [-256, 252] */ - BUG_ON(offset & 0x3); - BUG_ON(offset < -256 || offset > 252); + if ((offset & 0x3) || (offset < -256) || (offset > 252)) { + pr_err("%s: offset must be multiples of 4 in the range of [-256, 252] %d\n", + __func__, offset); + return AARCH64_BREAK_FAULT; + } shift = 2; break; case AARCH64_INSN_VARIANT_64BIT: - /* offset must be multiples of 8 in the range [-512, 504] */ - BUG_ON(offset & 0x7); - BUG_ON(offset < -512 || offset > 504); + if ((offset & 0x7) || (offset < -512) || (offset > 504)) { + pr_err("%s: offset must be multiples of 8 in the range of [-512, 504] %d\n", + __func__, offset); + return AARCH64_BREAK_FAULT; + } shift = 3; insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -702,7 +722,7 @@ u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst, insn = aarch64_insn_get_subs_imm_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown add/sub encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -713,11 +733,14 @@ u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst, insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } - BUG_ON(imm & ~(SZ_4K - 1)); + if (imm & ~(SZ_4K - 1)) { + pr_err("%s: invalid immediate encoding %d\n", __func__, imm); + return AARCH64_BREAK_FAULT; + } insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RD, insn, dst); @@ -746,7 +769,7 @@ u32 aarch64_insn_gen_bitfield(enum aarch64_insn_register dst, insn = aarch64_insn_get_sbfm_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown bitfield encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -759,12 +782,18 @@ u32 aarch64_insn_gen_bitfield(enum aarch64_insn_register dst, mask = GENMASK(5, 0); break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } - BUG_ON(immr & ~mask); - BUG_ON(imms & ~mask); + if (immr & ~mask) { + pr_err("%s: invalid immr encoding %d\n", __func__, immr); + return AARCH64_BREAK_FAULT; + } + if (imms & ~mask) { + pr_err("%s: invalid imms encoding %d\n", __func__, imms); + return AARCH64_BREAK_FAULT; + } insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RD, insn, dst); @@ -793,23 +822,33 @@ u32 aarch64_insn_gen_movewide(enum aarch64_insn_register dst, insn = aarch64_insn_get_movn_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown movewide encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } - BUG_ON(imm & ~(SZ_64K - 1)); + if (imm & ~(SZ_64K - 1)) { + pr_err("%s: invalid immediate encoding %d\n", __func__, imm); + return AARCH64_BREAK_FAULT; + } switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - BUG_ON(shift != 0 && shift != 16); + if (shift != 0 && shift != 16) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; case AARCH64_INSN_VARIANT_64BIT: insn |= AARCH64_INSN_SF_BIT; - BUG_ON(shift != 0 && shift != 16 && shift != 32 && - shift != 48); + if (shift != 0 && shift != 16 && shift != 32 && shift != 48) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -843,20 +882,28 @@ u32 aarch64_insn_gen_add_sub_shifted_reg(enum aarch64_insn_register dst, insn = aarch64_insn_get_subs_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown add/sub encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - BUG_ON(shift & ~(SZ_32 - 1)); + if (shift & ~(SZ_32 - 1)) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; case AARCH64_INSN_VARIANT_64BIT: insn |= AARCH64_INSN_SF_BIT; - BUG_ON(shift & ~(SZ_64 - 1)); + if (shift & ~(SZ_64 - 1)) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -885,11 +932,15 @@ u32 aarch64_insn_gen_data1(enum aarch64_insn_register dst, insn = aarch64_insn_get_rev32_value(); break; case AARCH64_INSN_DATA1_REVERSE_64: - BUG_ON(variant != AARCH64_INSN_VARIANT_64BIT); + if (variant != AARCH64_INSN_VARIANT_64BIT) { + pr_err("%s: invalid variant for reverse64 %d\n", + __func__, variant); + return AARCH64_BREAK_FAULT; + } insn = aarch64_insn_get_rev64_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown data1 encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -900,7 +951,7 @@ u32 aarch64_insn_gen_data1(enum aarch64_insn_register dst, insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -937,7 +988,7 @@ u32 aarch64_insn_gen_data2(enum aarch64_insn_register dst, insn = aarch64_insn_get_rorv_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown data2 encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -948,7 +999,7 @@ u32 aarch64_insn_gen_data2(enum aarch64_insn_register dst, insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -976,7 +1027,7 @@ u32 aarch64_insn_gen_data3(enum aarch64_insn_register dst, insn = aarch64_insn_get_msub_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown data3 encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } @@ -987,7 +1038,7 @@ u32 aarch64_insn_gen_data3(enum aarch64_insn_register dst, insn |= AARCH64_INSN_SF_BIT; break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; } @@ -1037,20 +1088,28 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst, insn = aarch64_insn_get_bics_value(); break; default: - BUG_ON(1); + pr_err("%s: unknown logical encoding %d\n", __func__, type); return AARCH64_BREAK_FAULT; } switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - BUG_ON(shift & ~(SZ_32 - 1)); + if (shift & ~(SZ_32 - 1)) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; case AARCH64_INSN_VARIANT_64BIT: insn |= AARCH64_INSN_SF_BIT; - BUG_ON(shift & ~(SZ_64 - 1)); + if (shift & ~(SZ_64 - 1)) { + pr_err("%s: invalid shift encoding %d\n", __func__, + shift); + return AARCH64_BREAK_FAULT; + } break; default: - BUG_ON(1); + pr_err("%s: unknown variant encoding %d\n", __func__, variant); return AARCH64_BREAK_FAULT; }