From patchwork Wed Nov 22 18:32:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gianluca Borello X-Patchwork-Id: 840470 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@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; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="DAtB0XcZ"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yhrgf6XrFz9s5L for ; Thu, 23 Nov 2017 05:33:34 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751804AbdKVSdc (ORCPT ); Wed, 22 Nov 2017 13:33:32 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36997 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751762AbdKVSdb (ORCPT ); Wed, 22 Nov 2017 13:33:31 -0500 Received: by mail-pg0-f66.google.com with SMTP id m4so2995776pgc.4 for ; Wed, 22 Nov 2017 10:33:31 -0800 (PST) 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; bh=sq3gV0X7wGqi4PZpnj0/ouXaxJo2Rs62Ic5F7LcaZjw=; b=DAtB0XcZhzzbPkDrDvE0rSAi0XIKdiwaNbD6cca9M17P/FTMPPq44U+/DyQRZqguVA 16jQenE4MDUKzhMpdvOTOa5ekUAUyQ5/8i3GFy04hioRBiDsE9fTo4dDMmUWxJI9+BGy 2xo/jvbBQokFACDJqsqeTRZOZYmOcrjLoPHHtH+w1TiLClYSPPNs5qLIf1IrLuSdQU1b 9a20hr8LxGYr06v5cc5y/rPlm1XqZjoec8KJgKvZnOcFh14Cz497uD8FgzNgNJIX7oDY 0CDJOREMA26LLQ6XNxEPd5wduY7N6xKjlAqMW4iXTLmiTXZ5PS8VO8Jfjx/0aC7meCL6 LqaA== 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; bh=sq3gV0X7wGqi4PZpnj0/ouXaxJo2Rs62Ic5F7LcaZjw=; b=bu3XrUh83H8OPw3L7aDQqkDBFDk3Cj2eGW9qNfBzOA3gxDeYllA9R89GM/MorRHGEo Y+4xnJkWsbNpJ+AUU/DvO9bk+jXh/Jc7aAvoeP+RZodvGkHn6/xP4Pd0BGLdpZpdoK5s V2ICIVroG1wQ7xp3XaCezZHcr6gWe3okENnaXPIVA6dX5zF0+JoO74FEm8s7sPh7NVtp TL1IEZ3YEA2Hy1w0Y9zOs05BpijKEQ2VNZQcLbcadKIuw2BXdR/IG3bBSdsTJn55UlEd oO/0bQGCt8sAqQmL14kLB0we7FEORmCeOkz+hD4eZu9TGmVZUyAxODgWYcYmsCqwIHm4 0Rwg== X-Gm-Message-State: AJaThX5Ub3gzumPDdcnfAOAqQQbB3U+Zye25DLayxfg2x3u8+cGnTSg4 t9dJqlVhDDO07/R/puFL2WFtzlqd X-Google-Smtp-Source: AGs4zMaKlHDFIDJeJWViA/eSVICMDe2AD1l0YTCoF9ZlGT4GAebEQoSik//fSk952fmg1pF1VvGJPA== X-Received: by 10.101.64.198 with SMTP id u6mr21498707pgp.44.1511375610592; Wed, 22 Nov 2017 10:33:30 -0800 (PST) Received: from localhost.localdomain (c-67-172-180-56.hsd1.ca.comcast.net. [67.172.180.56]) by smtp.gmail.com with ESMTPSA id k3sm34888075pfc.44.2017.11.22.10.33.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Nov 2017 10:33:29 -0800 (PST) From: Gianluca Borello To: netdev@vger.kernel.org Cc: daniel@iogearbox.net, ast@kernel.org, yhs@fb.com, Gianluca Borello Subject: [PATCH net 1/4] bpf: introduce ARG_PTR_TO_MEM_OR_NULL Date: Wed, 22 Nov 2017 18:32:53 +0000 Message-Id: <20171122183256.7219-2-g.borello@gmail.com> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171122183256.7219-1-g.borello@gmail.com> References: <20171122183256.7219-1-g.borello@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org With the current ARG_PTR_TO_MEM/ARG_PTR_TO_UNINIT_MEM semantics, an helper argument can be NULL when the next argument type is ARG_CONST_SIZE_OR_ZERO and the verifier can prove the value of this next argument is 0. However, most helpers are just interested in handling , so forcing them to deal with makes the implementation of those helpers more complicated for no apparent benefits, requiring them to explicitly handle those corner cases with checks that bpf programs could start relying upon, preventing the possibility of removing them later. Solve this by making ARG_PTR_TO_MEM/ARG_PTR_TO_UNINIT_MEM never accept NULL even when ARG_CONST_SIZE_OR_ZERO is set, and introduce a new argument type ARG_PTR_TO_MEM_OR_NULL to explicitly deal with the NULL case. Currently, the only helper that needs this is bpf_csum_diff_proto(), so change arg1 and arg3 to this new type as well. Also add a new battery of tests that explicitly test the !ARG_PTR_TO_MEM_OR_NULL combination: all the current ones testing the various variations are focused on bpf_csum_diff, so cover also other helpers. Signed-off-by: Gianluca Borello Acked-by: Alexei Starovoitov Acked-by: Daniel Borkmann --- include/linux/bpf.h | 1 + kernel/bpf/verifier.c | 4 +- net/core/filter.c | 4 +- tools/testing/selftests/bpf/test_verifier.c | 113 ++++++++++++++++++++++++++-- 4 files changed, 112 insertions(+), 10 deletions(-) -- 2.14.1 diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 76c577281d78..e55e4255a210 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -78,6 +78,7 @@ enum bpf_arg_type { * functions that access data on eBPF program stack */ ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value) */ + ARG_PTR_TO_MEM_OR_NULL, /* pointer to valid memory or NULL */ ARG_PTR_TO_UNINIT_MEM, /* pointer to memory does not need to be initialized, * helper function must fill all bytes or clear * them in error case. diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index dd54d20ace2f..308b0638ec5d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1384,13 +1384,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno, if (type != expected_type) goto err_type; } else if (arg_type == ARG_PTR_TO_MEM || + arg_type == ARG_PTR_TO_MEM_OR_NULL || arg_type == ARG_PTR_TO_UNINIT_MEM) { expected_type = PTR_TO_STACK; /* One exception here. In case function allows for NULL to be * passed in as argument, it's a SCALAR_VALUE type. Final test * happens during stack boundary checking. */ - if (register_is_null(*reg)) + if (register_is_null(*reg) && + arg_type == ARG_PTR_TO_MEM_OR_NULL) /* final test in check_stack_boundary() */; else if (!type_is_pkt_pointer(type) && type != PTR_TO_MAP_VALUE && diff --git a/net/core/filter.c b/net/core/filter.c index 1afa17935954..6a85e67fafce 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1646,9 +1646,9 @@ static const struct bpf_func_proto bpf_csum_diff_proto = { .gpl_only = false, .pkt_access = true, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM_OR_NULL, .arg2_type = ARG_CONST_SIZE_OR_ZERO, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM_OR_NULL, .arg4_type = ARG_CONST_SIZE_OR_ZERO, .arg5_type = ARG_ANYTHING, }; diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 2a5267bef160..3c64f30cf63c 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -5631,7 +5631,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_TRACEPOINT, }, { - "helper access to variable memory: size = 0 allowed on NULL", + "helper access to variable memory: size = 0 allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_MOV64_IMM(BPF_REG_1, 0), BPF_MOV64_IMM(BPF_REG_2, 0), @@ -5645,7 +5645,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size > 0 not allowed on NULL", + "helper access to variable memory: size > 0 not allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_MOV64_IMM(BPF_REG_1, 0), BPF_MOV64_IMM(BPF_REG_2, 0), @@ -5663,7 +5663,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size = 0 allowed on != NULL stack pointer", + "helper access to variable memory: size = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8), @@ -5680,7 +5680,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size = 0 allowed on != NULL map pointer", + "helper access to variable memory: size = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), @@ -5702,7 +5702,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer", + "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), @@ -5727,7 +5727,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size possible = 0 allowed on != NULL map pointer", + "helper access to variable memory: size possible = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), @@ -5750,7 +5750,7 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, { - "helper access to variable memory: size possible = 0 allowed on != NULL packet pointer", + "helper access to variable memory: size possible = 0 allowed on != NULL packet pointer (ARG_PTR_TO_MEM_OR_NULL)", .insns = { BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1, offsetof(struct __sk_buff, data)), @@ -5771,6 +5771,105 @@ static struct bpf_test tests[] = { .result = ACCEPT, .prog_type = BPF_PROG_TYPE_SCHED_CLS, }, + { + "helper access to variable memory: size = 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_MOV64_IMM(BPF_REG_1, 0), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .errstr = "R1 type=inv expected=fp", + .result = REJECT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, + { + "helper access to variable memory: size > 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_MOV64_IMM(BPF_REG_1, 0), + BPF_MOV64_IMM(BPF_REG_2, 1), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .errstr = "R1 type=inv expected=fp", + .result = REJECT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, + { + "helper access to variable memory: size = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .result = ACCEPT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, + { + "helper access to variable memory: size = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .fixup_map1 = { 3 }, + .result = ACCEPT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, + { + "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), + BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0), + BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 4), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .fixup_map1 = { 3 }, + .result = ACCEPT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, + { + "helper access to variable memory: size possible = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)", + .insns = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 5), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), + BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0), + BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 2), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_EMIT_CALL(BPF_FUNC_probe_read), + BPF_EXIT_INSN(), + }, + .fixup_map1 = { 3 }, + .result = ACCEPT, + .prog_type = BPF_PROG_TYPE_TRACEPOINT, + }, { "helper access to variable memory: 8 bytes leak", .insns = { From patchwork Wed Nov 22 18:32:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gianluca Borello X-Patchwork-Id: 840471 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@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; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="qsEcTuzi"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yhrgk6Fbtz9s5L for ; Thu, 23 Nov 2017 05:33:38 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751869AbdKVSdg (ORCPT ); Wed, 22 Nov 2017 13:33:36 -0500 Received: from mail-pf0-f193.google.com ([209.85.192.193]:40340 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751762AbdKVSde (ORCPT ); Wed, 22 Nov 2017 13:33:34 -0500 Received: by mail-pf0-f193.google.com with SMTP id q63so1123137pfd.7 for ; Wed, 22 Nov 2017 10:33:34 -0800 (PST) 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; bh=wuo43/SvJ/ItF5z2q6znb6i1YqTOrUBZyZUnZdYipN4=; b=qsEcTuziPjZ09AmL2SdR4SaBoOZXbsisQlirAZBPhPF46REjuYSwfbw00hW16VxTDV VLMZXR0PtXGsoUsdN6EMQ1bjP1CqAfgem8V/Anklyyui0c2MP9SGG0/e7Laruk2+CUR+ 1lj+8CVawnHtKzWU1O0V9NlsInpLEtXqei5gUarNSTW+NkOTMqRaJt6Rtmika2J6Bh/I Uxp7wXfzk/qyJhw9bnmt3bC51LJyOK0GDcAEUzYfzXxQT8VXRD1ol2ITeINzzs8jQCQO T30S27JYUrAVW27jMCC68JpFMz89wcBOM3meE2Dero5rYFhUesrHlFYFJQatqnYKm8SJ 1/gA== 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; bh=wuo43/SvJ/ItF5z2q6znb6i1YqTOrUBZyZUnZdYipN4=; b=DGSDk3a3f3tut4q0Sq4Jsr0wt6LoR2B7kFJsJJG5BmRE0dv2Q7sNFeNoRWtSZ+0+nX DnMKQy0ydCDMXNghJgk7CoDdN77vift9L88JjhWiJDjkAacGnU/dgo8lH8ouViqneanF CXx13Xn1/Y+DIP3wzX8jjHqBRcpSE9jeuMvu6tKvwHRY4/s+0cpH8IzjnAJ78Dly9BU7 cUc2XfYNnOoNoTe40wAQfGIou+V48UTvEUVXIg+1L3bcaHd1Hss7rhDyfqbW3J13boU6 yus6wtHca07zKNlzYVSVWP0nWSYsr0vgE6cPnVsrtNo0nOQMe+1sx4kvxHjWEtKmzE51 4z+Q== X-Gm-Message-State: AJaThX4AkW7JHsEdMhFAM2pg8rJthNr4/vtG9e/KrAJRygmLlW5xDopg LcyJSjp/F7BhIrPNyAvMLUT7odQO X-Google-Smtp-Source: AGs4zMbniawjMA2LGEjoyKDy7J5UvCk7OSUO/s0K3NX7DrZwIWh6HHLrfoMFvtFpi5qjuYTdzmR5QA== X-Received: by 10.99.119.79 with SMTP id s76mr21549802pgc.192.1511375613551; Wed, 22 Nov 2017 10:33:33 -0800 (PST) Received: from localhost.localdomain (c-67-172-180-56.hsd1.ca.comcast.net. [67.172.180.56]) by smtp.gmail.com with ESMTPSA id k3sm34888075pfc.44.2017.11.22.10.33.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Nov 2017 10:33:33 -0800 (PST) From: Gianluca Borello To: netdev@vger.kernel.org Cc: daniel@iogearbox.net, ast@kernel.org, yhs@fb.com, Gianluca Borello Subject: [PATCH net 2/4] bpf: remove explicit handling of 0 for arg2 in bpf_probe_read Date: Wed, 22 Nov 2017 18:32:54 +0000 Message-Id: <20171122183256.7219-3-g.borello@gmail.com> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171122183256.7219-1-g.borello@gmail.com> References: <20171122183256.7219-1-g.borello@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Commit 9c019e2bc4b2 ("bpf: change helper bpf_probe_read arg2 type to ARG_CONST_SIZE_OR_ZERO") changed arg2 type to ARG_CONST_SIZE_OR_ZERO to simplify writing bpf programs by taking advantage of the new semantics introduced for ARG_CONST_SIZE_OR_ZERO which allows arguments. In order to prevent the helper from actually passing a NULL pointer to probe_kernel_read, which can happen when is passed to the helper, the commit also introduced an explicit check against size == 0. After the recent introduction of the ARG_PTR_TO_MEM_OR_NULL type, bpf_probe_read can not receive a pair of arguments anymore, thus the check is not needed anymore and can be removed, since probe_kernel_read can correctly handle a call. This also fixes the semantics of the helper before it gets officially released and bpf programs start relying on this check. Fixes: 9c019e2bc4b2 ("bpf: change helper bpf_probe_read arg2 type to ARG_CONST_SIZE_OR_ZERO") Signed-off-by: Gianluca Borello Acked-by: Alexei Starovoitov Acked-by: Daniel Borkmann Acked-by: Yonghong Song --- kernel/trace/bpf_trace.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) -- 2.14.1 diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index a5580c670866..728909f7951c 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -78,16 +78,12 @@ EXPORT_SYMBOL_GPL(trace_call_bpf); BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr) { - int ret = 0; - - if (unlikely(size == 0)) - goto out; + int ret; ret = probe_kernel_read(dst, unsafe_ptr, size); if (unlikely(ret < 0)) memset(dst, 0, size); - out: return ret; } From patchwork Wed Nov 22 18:32:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gianluca Borello X-Patchwork-Id: 840472 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@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; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="bpGZU1zO"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yhrgm32T7z9s71 for ; Thu, 23 Nov 2017 05:33:40 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751934AbdKVSdi (ORCPT ); Wed, 22 Nov 2017 13:33:38 -0500 Received: from mail-pg0-f67.google.com ([74.125.83.67]:36220 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751861AbdKVSdg (ORCPT ); Wed, 22 Nov 2017 13:33:36 -0500 Received: by mail-pg0-f67.google.com with SMTP id 199so1294382pgg.3 for ; Wed, 22 Nov 2017 10:33:36 -0800 (PST) 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; bh=C32SPnQw6gbi9bZfqNmLIJpQGUrglQVZJaVv3SP0tFg=; b=bpGZU1zOYQDt8m17gLgI9hkRdbj1R1m/BB4KVNuIzJ7cjaNrmD1Shcfy/Nqq54EgCO LJX3WSgQ/2Ap8ptK5Lsh1Y/CzhCmaaT+/nnMHdLdfNRmaWS/r5aHfKcynZNcLZ3Q9alg jl710HiKGxPdBnINfiB+huJ+Fyo/w+cQ1ZqKiS4z4LxUkBCh3u54nJ7GL/YNEeOlIwV0 23cQRTkXHC54sNUlbwBlPEXZ2DyhkZe+2CTzUM18KJSvVJ8Ff7QL4NU99hiy/3zf8Swu B9us66GKERpFL0Q3lkYMyca5w/Ulo0AQCtL1RN44sRe8iya4ipeLi5T5Y9e0Br0fkBed quWQ== 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; bh=C32SPnQw6gbi9bZfqNmLIJpQGUrglQVZJaVv3SP0tFg=; b=cTNLVHTRYWdL9MBi3HafFOqn+3Phf7l97BXQT0+qrTqdFWbMlWZ8jidwiTJWWFaviu JZZSSHrJKVGYMjnLYNQAXflN+LyyX2pZkZdGl6zFrmHXFcWoPlZbBcwfkBzEfYmiQDdi OFyKqox3s6jZmqH8dk2FTUWVlNiPrgzp4m+eFc+TfKXolue3fa6NJ/QmjjDLHTpTWmns xUJQ1DSpLWWVVIvWu8cqvFjAho/hdjYD3ZeooeipmHr+FhDaEbJdsig1ZVmisfuu0atZ d+gHwZpR+eeXM6zgrzmsKkFX1PxsjgIA5sRL61Bfjxcc1+vZqysgxBnMD4kJ1urYjSQ3 uNLg== X-Gm-Message-State: AJaThX6A8t4EWXdDGD8hSl3qkHan0d0c0mCg6XXxkfxNvl5cH19lW13g RWYmzy9JlKYXzdesWygMxdL9a9bD X-Google-Smtp-Source: AGs4zMau5cmMlKSom8B3u/P/X7fkFk5QKwt2p/LefptvLFGh5m0hHbviM3/bBRiUTqk9DFp2FFB8bg== X-Received: by 10.98.133.65 with SMTP id u62mr19918489pfd.22.1511375615541; Wed, 22 Nov 2017 10:33:35 -0800 (PST) Received: from localhost.localdomain (c-67-172-180-56.hsd1.ca.comcast.net. [67.172.180.56]) by smtp.gmail.com with ESMTPSA id k3sm34888075pfc.44.2017.11.22.10.33.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Nov 2017 10:33:34 -0800 (PST) From: Gianluca Borello To: netdev@vger.kernel.org Cc: daniel@iogearbox.net, ast@kernel.org, yhs@fb.com, Gianluca Borello Subject: [PATCH net 3/4] bpf: change bpf_probe_read_str arg2 type to ARG_CONST_SIZE_OR_ZERO Date: Wed, 22 Nov 2017 18:32:55 +0000 Message-Id: <20171122183256.7219-4-g.borello@gmail.com> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171122183256.7219-1-g.borello@gmail.com> References: <20171122183256.7219-1-g.borello@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Commit 9fd29c08e520 ("bpf: improve verifier ARG_CONST_SIZE_OR_ZERO semantics") relaxed the treatment of ARG_CONST_SIZE_OR_ZERO due to the way the compiler generates optimized BPF code when checking boundaries of an argument from C code. A typical example of this optimized code can be generated using the bpf_probe_read_str helper when operating on variable memory: /* len is a generic scalar */ if (len > 0 && len <= 0x7fff) bpf_probe_read_str(p, len, s); 251: (79) r1 = *(u64 *)(r10 -88) 252: (07) r1 += -1 253: (25) if r1 > 0x7ffe goto pc-42 254: (bf) r1 = r7 255: (79) r2 = *(u64 *)(r10 -88) 256: (bf) r8 = r4 257: (85) call bpf_probe_read_str#45 R2 min value is negative, either use unsigned or 'var &= const' With this code, the verifier loses track of the variable. Replacing arg2 with ARG_CONST_SIZE_OR_ZERO is thus desirable since it avoids this quite common case which leads to usability issues, and the compiler generates code that the verifier can more easily test: if (len <= 0x7fff) bpf_probe_read_str(p, len, s); or bpf_probe_read_str(p, len & 0x7fff, s); No changes to the bpf_probe_read_str helper are necessary since strncpy_from_unsafe itself immediately returns if the size passed is 0. Signed-off-by: Gianluca Borello Acked-by: Alexei Starovoitov Acked-by: Daniel Borkmann --- kernel/trace/bpf_trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.14.1 diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 728909f7951c..ed8601a1a861 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -494,7 +494,7 @@ static const struct bpf_func_proto bpf_probe_read_str_proto = { .gpl_only = true, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_UNINIT_MEM, - .arg2_type = ARG_CONST_SIZE, + .arg2_type = ARG_CONST_SIZE_OR_ZERO, .arg3_type = ARG_ANYTHING, }; From patchwork Wed Nov 22 18:32:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gianluca Borello X-Patchwork-Id: 840473 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@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; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="BlTM5qcp"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yhrgq69HXz9s5L for ; Thu, 23 Nov 2017 05:33:43 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751965AbdKVSdl (ORCPT ); Wed, 22 Nov 2017 13:33:41 -0500 Received: from mail-pg0-f67.google.com ([74.125.83.67]:34643 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751762AbdKVSdi (ORCPT ); Wed, 22 Nov 2017 13:33:38 -0500 Received: by mail-pg0-f67.google.com with SMTP id 4so12944138pge.1 for ; Wed, 22 Nov 2017 10:33:38 -0800 (PST) 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; bh=/SXwh3oqxTfc2Q0QaCTEMpn661dXsEFB9BPgp8RF/6E=; b=BlTM5qcpDUASc77x0IW3+jjtxfxI6MnuJUhXdiQ/qMC7AOoMHtZZNIHVkNqBobDU8H u43Bh81W7HBXXfEEy2NTjBJyMl5lp7KDFm8L+nIo7aIIpMW3MSPgJr5SnlSNLTvQo8W0 sE7JrdMRLTHts8dhFDZfeoaCF2EseWF9avx0Z+NLOe6wpnBB4dtUKdhYHftMPQUQb3iU uK7InpTjBGjN3gbX4ftHOtVxBqC5i1XCFY1ANtDRY4BHumU0v00vfKVjPPnutieEBJqv HTo85tFnaV3KeSjgnqUyslsywCKs3FDCfH/LYsEfpLQCwLgX5mxrCERJB5PBRqkVA3U4 cNrA== 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; bh=/SXwh3oqxTfc2Q0QaCTEMpn661dXsEFB9BPgp8RF/6E=; b=QX2t0bGu6crKNqzS57NX5alivn10r94qcJ4y554SbEwS6BLvaIzn4sosPGzApGptPD InC5UvSCgM5WL5fs86OLFS/Q4HiN0GziPlMelEPtZ+Hsmgm97N105iytd6Nv9f8MjUV2 wVHbuodQbHGXvYGFfgEgcDE/duhUfrWu/ioPQiPgSxQvc4sTW5X6ch6J0GP+ckvPtzaP RjNLz3/qMo9tdPLt/RDbLYDjZizXe1AWz6iO1HXb4tqz1A8RZG0NAHhHZG18g7aminHj Ux3MqmHuPnHXJflClRO4Hf6x/LjsvKm/aH1dsBEIVvTOXUZjz3+0aVw7j8nX8h7MJHE8 ha1A== X-Gm-Message-State: AJaThX46VHKRGoN4WVrAcF/rXhyREK2iqhhXMtxSfQdygaOlBqRkWDPd vbEgvv4EFJSSdU7vGaUQHObHR43c X-Google-Smtp-Source: AGs4zMZo9EH7YdIGALcqT5VYLBtaDRgQKkPQ/I7WsBdOx6k4P6dQMiSyNJ4z7NsIqk0UewqRKAqGUg== X-Received: by 10.98.19.202 with SMTP id 71mr20214694pft.181.1511375617453; Wed, 22 Nov 2017 10:33:37 -0800 (PST) Received: from localhost.localdomain (c-67-172-180-56.hsd1.ca.comcast.net. [67.172.180.56]) by smtp.gmail.com with ESMTPSA id k3sm34888075pfc.44.2017.11.22.10.33.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Nov 2017 10:33:36 -0800 (PST) From: Gianluca Borello To: netdev@vger.kernel.org Cc: daniel@iogearbox.net, ast@kernel.org, yhs@fb.com, Gianluca Borello Subject: [PATCH net 4/4] bpf: change bpf_perf_event_output arg5 type to ARG_CONST_SIZE_OR_ZERO Date: Wed, 22 Nov 2017 18:32:56 +0000 Message-Id: <20171122183256.7219-5-g.borello@gmail.com> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171122183256.7219-1-g.borello@gmail.com> References: <20171122183256.7219-1-g.borello@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Commit 9fd29c08e520 ("bpf: improve verifier ARG_CONST_SIZE_OR_ZERO semantics") relaxed the treatment of ARG_CONST_SIZE_OR_ZERO due to the way the compiler generates optimized BPF code when checking boundaries of an argument from C code. A typical example of this optimized code can be generated using the bpf_perf_event_output helper when operating on variable memory: /* len is a generic scalar */ if (len > 0 && len <= 0x7fff) bpf_perf_event_output(ctx, &perf_map, 0, buf, len); 110: (79) r5 = *(u64 *)(r10 -40) 111: (bf) r1 = r5 112: (07) r1 += -1 113: (25) if r1 > 0x7ffe goto pc+6 114: (bf) r1 = r6 115: (18) r2 = 0xffff94e5f166c200 117: (b7) r3 = 0 118: (bf) r4 = r7 119: (85) call bpf_perf_event_output#25 R5 min value is negative, either use unsigned or 'var &= const' With this code, the verifier loses track of the variable. Replacing arg5 with ARG_CONST_SIZE_OR_ZERO is thus desirable since it avoids this quite common case which leads to usability issues, and the compiler generates code that the verifier can more easily test: if (len <= 0x7fff) bpf_perf_event_output(ctx, &perf_map, 0, buf, len); or bpf_perf_event_output(ctx, &perf_map, 0, buf, len & 0x7fff); No changes to the bpf_perf_event_output helper are necessary since it can handle a case where size is 0, and an empty frame is pushed. Reported-by: Arnaldo Carvalho de Melo Signed-off-by: Gianluca Borello Acked-by: Alexei Starovoitov Acked-by: Daniel Borkmann --- kernel/trace/bpf_trace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.14.1 diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index ed8601a1a861..27d1f4ffa3de 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -403,7 +403,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = { .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, .arg4_type = ARG_PTR_TO_MEM, - .arg5_type = ARG_CONST_SIZE, + .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs); @@ -605,7 +605,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_tp = { .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, .arg4_type = ARG_PTR_TO_MEM, - .arg5_type = ARG_CONST_SIZE, + .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,