Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp3239647imu; Sat, 24 Nov 2018 00:50:47 -0800 (PST) X-Google-Smtp-Source: AFSGD/WXvAMmTAZy9PtKRwbGX2a99dQUq/0QffNHW/qu3gCUW4RVR3GyNY0ACKNVh2FQKAGw2BHJ X-Received: by 2002:a63:3703:: with SMTP id e3mr16852563pga.348.1543049447228; Sat, 24 Nov 2018 00:50:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1543049447; cv=none; d=google.com; s=arc-20160816; b=Zl3u7rvAgSHPtd4cbQvfVkMgGyR+5Z+HthERjbf7kXTPmeN61U/hNWdLte23ig9gPx HBqQAXjOQB97pYBOO0gO2Ds0gS1jw4Tv75K3eAHgtZ1K9tBqFvYCZ1RfrW/BWxcGqeTL qv8DQ3Xg0yA70CwsQWVtSp9bT+Tj8x0IaXNltc+crsYpDRL21e0Hfv+4xGbR4/iBMaqA b0DIOkTKziFkA5SpJhkL5pgIxWOsW1hFHZsXR+A8uKByAQw+TWe0hMe43RhnMGraCH5a CVqAm37I6Iw8yiRhzmHifVa5g1ftomkrKJskxkqwCkvrqA8glLoZFWFwgwoVtCvGu67O QSog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=QFGctgpKBD3NI1JWNMibISKplLuHqntj7XUZUqj5vO8=; b=wiefB3GdUwo5qfNSbeE4PTRGAm2/+GVJjWmVvV0jOQLIQUO5aEyqr6EabPFG//mW5W o731mJ4QttsC+YZD41UPY4z0nyzyg8Balq9GGT0XQxSfon9YjHlcgNjg2FG/0VfUoPiq srHUAhTWRAM8Mf3ZOvchKDfHNCxSHMCwtkjDR/5kvJ6wSwhyq+xvpM+rHPGVWfyq7n3K CmodkFYn94jVGXPkRtbwkW7cjdmappsFzfDEv1XwacRYQp5xqsshsFi04JKjoJW69/di B8dlEBAWyrg9bt/U+b2pP4o2Pv8SBtltfsOft3giJXJZdeY4UGCStpJ/bVOaHpRHQ+mq Yb0w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j24si50377974pgn.149.2018.11.24.00.50.32; Sat, 24 Nov 2018 00:50:47 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2403936AbeKXG4P (ORCPT + 99 others); Sat, 24 Nov 2018 01:56:15 -0500 Received: from www62.your-server.de ([213.133.104.62]:54140 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726017AbeKXG4O (ORCPT ); Sat, 24 Nov 2018 01:56:14 -0500 Received: from [78.46.172.3] (helo=sslproxy06.your-server.de) by www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89_1) (envelope-from ) id 1gQHmT-0007PL-If; Fri, 23 Nov 2018 21:10:25 +0100 Received: from [178.197.248.22] (helo=linux.home) by sslproxy06.your-server.de with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1gQHmT-0008kW-Bn; Fri, 23 Nov 2018 21:10:25 +0100 Subject: Re: [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled To: Ben Hutchings , Alexei Starovoitov Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20181123183356.5q4bu47zpj5wdufb@xylophone.i.decadent.org.uk> <20181123183455.qjokyt6zpa2yck6s@xylophone.i.decadent.org.uk> From: Daniel Borkmann Message-ID: Date: Fri, 23 Nov 2018 21:10:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20181123183455.qjokyt6zpa2yck6s@xylophone.i.decadent.org.uk> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.100.2/25146/Fri Nov 23 15:18:52 2018) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/23/2018 07:34 PM, Ben Hutchings wrote: > User-space does not have access to the patched eBPF code, but we > need to be able to test that patches are being applied. Therefore > log distinct messages for each case that requires patching. Thanks for the patches, Ben! Above is actually not the case, e.g. privileged admin can use something like 'bpftool prog dump xlated id ' and then the BPF insns are dumped to user space for the program /after/ the verification, so the rewrites can then be seen. test_verifier temporarily drops caps to load and run the unprivileged cases, but we can extend the test suite to retrieve and check the final insns after that happened. I think this would be a nice extension to the test suite for cases like these and would also provide better insight than verbose() statement saying that something has been patched (but not the actual result of it). Thanks, Daniel > Signed-off-by: Ben Hutchings > --- > kernel/bpf/verifier.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 4ce049cd30a3..ea4bc796e545 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5844,6 +5844,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) > verbose(env, "bpf verifier is misconfigured\n"); > return -EINVAL; > } else if (cnt) { > + verbose(env, "patching in prologue\n"); > new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); > if (!new_prog) > return -ENOMEM; > @@ -5892,6 +5893,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) > }; > > cnt = ARRAY_SIZE(patch); > + verbose(env, > + "patching in sanitization against SSB at %d\n", > + i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt); > if (!new_prog) > return -ENOMEM; > @@ -5973,6 +5977,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) > } > } > > + verbose(env, "patching explicit ctx access at %d\n", i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); > if (!new_prog) > return -ENOMEM; > @@ -6225,6 +6230,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > cnt = ARRAY_SIZE(mask_and_mod) - (is64 ? 1 : 0); > } > > + verbose(env, "patching in divide-by-zero check at %d\n", > + i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt); > if (!new_prog) > return -ENOMEM; > @@ -6244,6 +6251,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > return -EINVAL; > } > > + verbose(env, "patching implicit ctx access at %d\n", > + i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); > if (!new_prog) > return -ENOMEM; > @@ -6307,6 +6316,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > map)->index_mask); > insn_buf[2] = *insn; > cnt = 3; > + verbose(env, "patching in tail-call bounds check at %d", > + i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); > if (!new_prog) > return -ENOMEM; > @@ -6342,6 +6353,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > return -EINVAL; > } > > + verbose(env, "patching in map lookup at %d", > + i + delta); > new_prog = bpf_patch_insn_data(env, i + delta, > insn_buf, cnt); > if (!new_prog) >