From patchwork Sat Dec 15 08:34:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiong Wang X-Patchwork-Id: 1013862 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=netronome.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=netronome-com.20150623.gappssmtp.com header.i=@netronome-com.20150623.gappssmtp.com header.b="CyaP2Sqt"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43H12849H1z9s3C for ; Sat, 15 Dec 2018 19:35:12 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729192AbeLOIfK (ORCPT ); Sat, 15 Dec 2018 03:35:10 -0500 Received: from mail-wm1-f67.google.com ([209.85.128.67]:39486 "EHLO mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726030AbeLOIfK (ORCPT ); Sat, 15 Dec 2018 03:35:10 -0500 Received: by mail-wm1-f67.google.com with SMTP id f81so7858918wmd.4 for ; Sat, 15 Dec 2018 00:35:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=8lFzlj7sQAt+E2tU6bob406D8Cp49UMmsgDMRyEipzc=; b=CyaP2SqtxEuGKzS6kQ4wsiPoORmh7dJlz21Ch4FRWerPGiFn1m0Tl2LMr9MTT7kwQE FKOnlA2uGCsFJAJLS+/8zr64xbGf/xY3KwDAEYFM3BWAc0LnqQDD4WMdGZiqmeP/Z3hw WyYIHnYki4usyAAnQsUPuL0xuXBgl1ln0rOlv2gJaGfy+3V7diNY6on6+C6eXvDFMt4i uOzQgRIZVppTTknM2VRyTTWBU+BCRC18dD3poI/TM729n9DBrPaJYO/H9gNM1e7trW1f MFfRPw50YVchtUTvAFFJIbpLDaTmr9oViHuwhDJrBYSk4hsmS5wHfmOXaSregVZnGf+C pHUA== 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; bh=8lFzlj7sQAt+E2tU6bob406D8Cp49UMmsgDMRyEipzc=; b=K2ZkSemgPDyPP/D2lQx/8CQ2n/3vq90+in8Wm1QIGTuG68D5dJzvBGVESJqIVJA4TY Wah7Ss7qGA1IMYs/0MMuohncSuURxqlYLLqRKPlfwqhuPPUwNbwhypuhjAf+p371odYh Yvj+o9ymLOqjEqJPu4sA3KOblwzVa7QXQCYNBK+wyZGFgNwtjQXmGAzZEbuh3Pjbz0dG 9yQZS1aIL46QS0o6LUQx+TKIDH6n/FtHyscZ5drAGEhzkqi5vC8DQcisRwf2ChCXtwa/ JeT/MnI+Ca9LuLnIo34AH2MbYcp8vXpvZ1Q/sTmHCaBKS7mKTjciPkkHckfpcxwNZ6QC adgg== X-Gm-Message-State: AA+aEWapCtlQmGmD3Au/hFmi3nawya0zW+EsL0I+F94b5+IGVRd5R5II 1Jf28uKhe1wvsW3wL25zw/YtpVVebDI= X-Google-Smtp-Source: AFSGD/Ve/Ue0fEeL/FK3PxY002WsySkbWseeVXj3QylFuEx1csSHOOw+EeBWnJSKH2l8JE+78B8Nwg== X-Received: by 2002:a1c:c64e:: with SMTP id w75mr5978333wmf.46.1544862907571; Sat, 15 Dec 2018 00:35:07 -0800 (PST) Received: from cbtest28.netronome.com ([217.38.71.146]) by smtp.gmail.com with ESMTPSA id v1sm7585900wrr.88.2018.12.15.00.35.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 15 Dec 2018 00:35:06 -0800 (PST) From: Jiong Wang To: ast@kernel.org, daniel@iogearbox.net Cc: netdev@vger.kernel.org, oss-drivers@netronome.com, Jiong Wang Subject: [PATCH bpf-next] bpf: correct slot_type marking logic to allow more stack slot sharing Date: Sat, 15 Dec 2018 03:34:40 -0500 Message-Id: <1544862880-23291-1-git-send-email-jiong.wang@netronome.com> X-Mailer: git-send-email 2.7.4 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Verifier is supposed to support sharing stack slot allocated to ptr with SCALAR_VALUE for privileged program. However this doesn't happen for some cases. The reason is verifier is not clearing slot_type STACK_SPILL for all bytes, it only clears part of them, while verifier is using: slot_type[0] == STACK_SPILL as a convention to check one slot is ptr type. So, the consequence of partial clearing slot_type is verifier could treat a partially overridden ptr slot, which should now be a SCALAR_VALUE slot, still as ptr slot, and rejects some valid programs. Before this patch, test_xdp_noinline.o under bpf selftests, bpf_lxc.o and bpf_netdev.o under Cilium bpf repo, when built with -mattr=+alu32 are rejected due to this issue. After this patch, they all accepted. There is no processed insn number change before and after this patch on Cilium bpf programs. Reviewed-by: Jakub Kicinski Signed-off-by: Jiong Wang Reviewed-by: Daniel Borkmann --- kernel/bpf/verifier.c | 5 +++++ tools/testing/selftests/bpf/test_verifier.c | 34 +++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8b511a4..352183b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1220,6 +1220,10 @@ static int check_stack_write(struct bpf_verifier_env *env, /* regular write of data into stack destroys any spilled ptr */ state->stack[spi].spilled_ptr.type = NOT_INIT; + /* Mark slots as STACK_MISC if they belonged to spilled ptr. */ + if (state->stack[spi].slot_type[0] == STACK_SPILL) + for (i = 0; i < BPF_REG_SIZE; i++) + state->stack[spi].slot_type[i] = STACK_MISC; /* only mark the slot as written if all 8 bytes were written * otherwise read propagation may incorrectly stop too soon @@ -1237,6 +1241,7 @@ static int check_stack_write(struct bpf_verifier_env *env, register_is_null(&cur->regs[value_regno])) type = STACK_ZERO; + /* Mark slots affected by this stack write. */ for (i = 0; i < size; i++) state->stack[spi].slot_type[(slot - i) % BPF_REG_SIZE] = type; diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index a08c67c..51eea7f 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -1001,15 +1001,45 @@ static struct bpf_test tests[] = { BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8), /* mess up with R1 pointer on stack */ BPF_ST_MEM(BPF_B, BPF_REG_10, -7, 0x23), - /* fill back into R0 should fail */ + /* fill back into R0 is fine for priv. + * R0 now becomes SCALAR_VALUE. + */ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8), + /* Load from R0 should fail. */ + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 8), BPF_EXIT_INSN(), }, .errstr_unpriv = "attempt to corrupt spilled", - .errstr = "corrupted spill", + .errstr = "R0 invalid mem access 'inv", .result = REJECT, }, { + "check corrupted spill/fill, LSB", + .insns = { + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8), + BPF_ST_MEM(BPF_H, BPF_REG_10, -8, 0xcafe), + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8), + BPF_EXIT_INSN(), + }, + .errstr_unpriv = "attempt to corrupt spilled", + .result_unpriv = REJECT, + .result = ACCEPT, + .retval = POINTER_VALUE, + }, + { + "check corrupted spill/fill, MSB", + .insns = { + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8), + BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0x12345678), + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8), + BPF_EXIT_INSN(), + }, + .errstr_unpriv = "attempt to corrupt spilled", + .result_unpriv = REJECT, + .result = ACCEPT, + .retval = POINTER_VALUE, + }, + { "invalid src register in STX", .insns = { BPF_STX_MEM(BPF_B, BPF_REG_10, -1, -1),