From: Luke Nelson <lukenels@cs.washington.edu> To: bpf@vger.kernel.org Cc: Luke Nelson <luke.r.nels@gmail.com>, Xi Wang <xi.wang@gmail.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Alexei Starovoitov <ast@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@chromium.org>, Mark Rutland <mark.rutland@arm.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Thomas Gleixner <tglx@linutronix.de>, Christoffer Dall <christoffer.dall@linaro.org>, Marc Zyngier <maz@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, clang-built-linux@googlegroups.com Subject: [RFC PATCH bpf-next 1/3] arm64: insn: Fix two bugs in encoding 32-bit logical immediates Date: Wed, 6 May 2020 18:05:01 -0700 [thread overview] Message-ID: <20200507010504.26352-2-luke.r.nels@gmail.com> (raw) In-Reply-To: <20200507010504.26352-1-luke.r.nels@gmail.com> This patch fixes two issues present in the current function for encoding arm64 logical immediates when using the 32-bit variants of instructions. First, the code does not correctly reject an all-ones 32-bit immediate and returns an undefined instruction encoding, which can crash the kernel. The fix is to add a check for this case. Second, the code incorrectly rejects some 32-bit immediates that are actually encodable as logical immediates. The root cause is that the code uses a default mask of 64-bit all-ones, even for 32-bit immediates. This causes an issue later on when the mask is used to fill the top bits of the immediate with ones, shown here: /* * Pattern: 0..01..10..01..1 * * Fill the unused top bits with ones, and check if * the result is a valid immediate (all ones with a * contiguous ranges of zeroes). */ imm |= ~mask; if (!range_of_ones(~imm)) return AARCH64_BREAK_FAULT; To see the problem, consider an immediate of the form 0..01..10..01..1, where the upper 32 bits are zero, such as 0x80000001. The code checks if ~(imm | ~mask) contains a range of ones: the incorrect mask yields 1..10..01..10..0, which fails the check; the correct mask yields 0..01..10..0, which succeeds. The fix is to use a 32-bit all-ones default mask for 32-bit immediates. Currently, the only user of this function is in arch/arm64/kvm/va_layout.c, which uses 64-bit immediates and won't trigger these bugs. We tested the new code against llvm-mc with all 1,302 encodable 32-bit logical immediates and all 5,334 encodable 64-bit logical immediates. Fixes: ef3935eeebff ("arm64: insn: Add encoder for bitwise operations using literals") Co-developed-by: Xi Wang <xi.wang@gmail.com> Signed-off-by: Xi Wang <xi.wang@gmail.com> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com> --- arch/arm64/kernel/insn.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 4a9e773a177f..42fad79546bb 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -1535,7 +1535,7 @@ static u32 aarch64_encode_immediate(u64 imm, u32 insn) { unsigned int immr, imms, n, ones, ror, esz, tmp; - u64 mask = ~0UL; + u64 mask; /* Can't encode full zeroes or full ones */ if (!imm || !~imm) @@ -1543,13 +1543,15 @@ static u32 aarch64_encode_immediate(u64 imm, switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - if (upper_32_bits(imm)) + if (upper_32_bits(imm) || imm == 0xffffffffUL) return AARCH64_BREAK_FAULT; esz = 32; + mask = 0xffffffffUL; break; case AARCH64_INSN_VARIANT_64BIT: insn |= AARCH64_INSN_SF_BIT; esz = 64; + mask = ~0UL; break; default: pr_err("%s: unknown variant encoding %d\n", __func__, variant); -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Luke Nelson <lukenels@cs.washington.edu> To: bpf@vger.kernel.org Cc: Mark Rutland <mark.rutland@arm.com>, Song Liu <songliubraving@fb.com>, Catalin Marinas <catalin.marinas@arm.com>, Alexei Starovoitov <ast@kernel.org>, Will Deacon <will@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Marc Zyngier <maz@kernel.org>, John Fastabend <john.fastabend@gmail.com>, clang-built-linux@googlegroups.com, Zi Shen Lim <zlim.lnx@gmail.com>, Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>, Xi Wang <xi.wang@gmail.com>, Luke Nelson <luke.r.nels@gmail.com>, KP Singh <kpsingh@chromium.org>, Thomas Gleixner <tglx@linutronix.de>, linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Martin KaFai Lau <kafai@fb.com>, Christoffer Dall <christoffer.dall@linaro.org> Subject: [RFC PATCH bpf-next 1/3] arm64: insn: Fix two bugs in encoding 32-bit logical immediates Date: Wed, 6 May 2020 18:05:01 -0700 [thread overview] Message-ID: <20200507010504.26352-2-luke.r.nels@gmail.com> (raw) In-Reply-To: <20200507010504.26352-1-luke.r.nels@gmail.com> This patch fixes two issues present in the current function for encoding arm64 logical immediates when using the 32-bit variants of instructions. First, the code does not correctly reject an all-ones 32-bit immediate and returns an undefined instruction encoding, which can crash the kernel. The fix is to add a check for this case. Second, the code incorrectly rejects some 32-bit immediates that are actually encodable as logical immediates. The root cause is that the code uses a default mask of 64-bit all-ones, even for 32-bit immediates. This causes an issue later on when the mask is used to fill the top bits of the immediate with ones, shown here: /* * Pattern: 0..01..10..01..1 * * Fill the unused top bits with ones, and check if * the result is a valid immediate (all ones with a * contiguous ranges of zeroes). */ imm |= ~mask; if (!range_of_ones(~imm)) return AARCH64_BREAK_FAULT; To see the problem, consider an immediate of the form 0..01..10..01..1, where the upper 32 bits are zero, such as 0x80000001. The code checks if ~(imm | ~mask) contains a range of ones: the incorrect mask yields 1..10..01..10..0, which fails the check; the correct mask yields 0..01..10..0, which succeeds. The fix is to use a 32-bit all-ones default mask for 32-bit immediates. Currently, the only user of this function is in arch/arm64/kvm/va_layout.c, which uses 64-bit immediates and won't trigger these bugs. We tested the new code against llvm-mc with all 1,302 encodable 32-bit logical immediates and all 5,334 encodable 64-bit logical immediates. Fixes: ef3935eeebff ("arm64: insn: Add encoder for bitwise operations using literals") Co-developed-by: Xi Wang <xi.wang@gmail.com> Signed-off-by: Xi Wang <xi.wang@gmail.com> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com> --- arch/arm64/kernel/insn.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 4a9e773a177f..42fad79546bb 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -1535,7 +1535,7 @@ static u32 aarch64_encode_immediate(u64 imm, u32 insn) { unsigned int immr, imms, n, ones, ror, esz, tmp; - u64 mask = ~0UL; + u64 mask; /* Can't encode full zeroes or full ones */ if (!imm || !~imm) @@ -1543,13 +1543,15 @@ static u32 aarch64_encode_immediate(u64 imm, switch (variant) { case AARCH64_INSN_VARIANT_32BIT: - if (upper_32_bits(imm)) + if (upper_32_bits(imm) || imm == 0xffffffffUL) return AARCH64_BREAK_FAULT; esz = 32; + mask = 0xffffffffUL; break; case AARCH64_INSN_VARIANT_64BIT: insn |= AARCH64_INSN_SF_BIT; esz = 64; + mask = ~0UL; break; default: pr_err("%s: unknown variant encoding %d\n", __func__, variant); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-07 1:05 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-07 1:05 [RFC PATCH bpf-next 0/3] arm64 BPF JIT Optimizations Luke Nelson 2020-05-07 1:05 ` Luke Nelson 2020-05-07 1:05 ` Luke Nelson [this message] 2020-05-07 1:05 ` [RFC PATCH bpf-next 1/3] arm64: insn: Fix two bugs in encoding 32-bit logical immediates Luke Nelson 2020-05-07 8:19 ` Marc Zyngier 2020-05-07 8:19 ` Marc Zyngier 2020-05-07 8:29 ` Will Deacon 2020-05-07 8:29 ` Will Deacon 2020-05-07 9:12 ` Marc Zyngier 2020-05-07 9:12 ` Marc Zyngier 2020-05-07 21:48 ` Luke Nelson 2020-05-07 21:48 ` Luke Nelson 2020-05-08 11:47 ` Will Deacon 2020-05-08 11:47 ` Will Deacon 2020-05-08 18:12 ` Luke Nelson 2020-05-08 18:12 ` Luke Nelson 2020-05-07 1:05 ` [RFC PATCH bpf-next 2/3] bpf, arm64: Optimize AND,OR,XOR,JSET BPF_K using arm64 " Luke Nelson 2020-05-07 1:05 ` [RFC PATCH bpf-next 2/3] bpf, arm64: Optimize AND, OR, XOR, JSET " Luke Nelson 2020-05-07 20:19 ` [RFC PATCH bpf-next 2/3] bpf, arm64: Optimize AND,OR,XOR,JSET " Daniel Borkmann 2020-05-07 20:19 ` Daniel Borkmann 2020-05-07 1:05 ` [RFC PATCH bpf-next 3/3] bpf, arm64: Optimize ADD,SUB,JMP BPF_K using arm64 add/sub immediates Luke Nelson 2020-05-07 1:05 ` [RFC PATCH bpf-next 3/3] bpf, arm64: Optimize ADD, SUB, JMP " Luke Nelson 2020-05-07 20:22 ` [RFC PATCH bpf-next 3/3] bpf, arm64: Optimize ADD,SUB,JMP " Daniel Borkmann 2020-05-07 20:22 ` Daniel Borkmann
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200507010504.26352-2-luke.r.nels@gmail.com \ --to=lukenels@cs.washington.edu \ --cc=andriin@fb.com \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=catalin.marinas@arm.com \ --cc=christoffer.dall@linaro.org \ --cc=clang-built-linux@googlegroups.com \ --cc=daniel@iogearbox.net \ --cc=gregkh@linuxfoundation.org \ --cc=john.fastabend@gmail.com \ --cc=kafai@fb.com \ --cc=kpsingh@chromium.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luke.r.nels@gmail.com \ --cc=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=netdev@vger.kernel.org \ --cc=songliubraving@fb.com \ --cc=tglx@linutronix.de \ --cc=will@kernel.org \ --cc=xi.wang@gmail.com \ --cc=yhs@fb.com \ --cc=zlim.lnx@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.