From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C726C433FE for ; Thu, 26 May 2022 09:45:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346912AbiEZJpn (ORCPT ); Thu, 26 May 2022 05:45:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345513AbiEZJpi (ORCPT ); Thu, 26 May 2022 05:45:38 -0400 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EAB61C1EF9; Thu, 26 May 2022 02:45:30 -0700 (PDT) Received: from kwepemi500013.china.huawei.com (unknown [172.30.72.55]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4L832V41dgzDqKf; Thu, 26 May 2022 17:45:22 +0800 (CST) Received: from [10.67.111.192] (10.67.111.192) by kwepemi500013.china.huawei.com (7.221.188.120) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 26 May 2022 17:45:26 +0800 Message-ID: <985fe022-552c-9d04-16d8-14784c4075f8@huawei.com> Date: Thu, 26 May 2022 17:45:25 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH bpf-next v5 3/6] bpf: Remove is_valid_bpf_tramp_flags() Content-Language: en-US To: Mark Rutland CC: , , , , , Catalin Marinas , Will Deacon , Steven Rostedt , Ingo Molnar , Daniel Borkmann , Alexei Starovoitov , Zi Shen Lim , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , "David S . Miller" , Hideaki YOSHIFUJI , David Ahern , Thomas Gleixner , Borislav Petkov , Dave Hansen , , , Shuah Khan , Jakub Kicinski , Jesper Dangaard Brouer , Pasha Tatashin , Ard Biesheuvel , Daniel Kiss , Steven Price , Sudeep Holla , Marc Zyngier , Peter Collingbourne , Mark Brown , Delyan Kratunov , Kumar Kartikeya Dwivedi References: <20220518131638.3401509-1-xukuohai@huawei.com> <20220518131638.3401509-4-xukuohai@huawei.com> From: Xu Kuohai In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.111.192] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500013.china.huawei.com (7.221.188.120) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/25/2022 9:45 PM, Mark Rutland wrote: > On Wed, May 18, 2022 at 09:16:35AM -0400, Xu Kuohai wrote: >> BPF_TRAM_F_XXX flags are not used by user code and are almost constant >> at compile time, so run time validation is a bit overkill. Remove >> is_valid_bpf_tramp_flags() and add some usage comments. >> >> Signed-off-by: Xu Kuohai >> Acked-by: Song Liu > > Am I right in thinking this is independent of the arm64-specific bits, and > could be taken on its own now? > Currenly is_valid_bpf_tramp_flags() is defined in x86 and called before bpf trampoline is constructed. The check logic is irrelevant to the architecture code. So we also need to call this function on arm64. But as Alexei pointed out, the check is not requried, so it's better to remove it before adding bpf trampoline to arm64. > Mark. > >> --- >> arch/x86/net/bpf_jit_comp.c | 20 -------------------- >> kernel/bpf/bpf_struct_ops.c | 3 +++ >> kernel/bpf/trampoline.c | 3 +++ >> 3 files changed, 6 insertions(+), 20 deletions(-) >> >> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c >> index a2b6d197c226..7698ef3b4821 100644 >> --- a/arch/x86/net/bpf_jit_comp.c >> +++ b/arch/x86/net/bpf_jit_comp.c >> @@ -1922,23 +1922,6 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog, >> return 0; >> } >> >> -static bool is_valid_bpf_tramp_flags(unsigned int flags) >> -{ >> - if ((flags & BPF_TRAMP_F_RESTORE_REGS) && >> - (flags & BPF_TRAMP_F_SKIP_FRAME)) >> - return false; >> - >> - /* >> - * BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, >> - * and it must be used alone. >> - */ >> - if ((flags & BPF_TRAMP_F_RET_FENTRY_RET) && >> - (flags & ~BPF_TRAMP_F_RET_FENTRY_RET)) >> - return false; >> - >> - return true; >> -} >> - >> /* Example: >> * __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev); >> * its 'struct btf_func_model' will be nr_args=2 >> @@ -2017,9 +2000,6 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i >> if (nr_args > 6) >> return -ENOTSUPP; >> >> - if (!is_valid_bpf_tramp_flags(flags)) >> - return -EINVAL; >> - >> /* Generated trampoline stack layout: >> * >> * RBP + 8 [ return address ] >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c >> index d9a3c9207240..0572cc5aeb28 100644 >> --- a/kernel/bpf/bpf_struct_ops.c >> +++ b/kernel/bpf/bpf_struct_ops.c >> @@ -341,6 +341,9 @@ int bpf_struct_ops_prepare_trampoline(struct bpf_tramp_links *tlinks, >> >> tlinks[BPF_TRAMP_FENTRY].links[0] = link; >> tlinks[BPF_TRAMP_FENTRY].nr_links = 1; >> + /* BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, >> + * and it must be used alone. >> + */ >> flags = model->ret_size > 0 ? BPF_TRAMP_F_RET_FENTRY_RET : 0; >> return arch_prepare_bpf_trampoline(NULL, image, image_end, >> model, flags, tlinks, NULL); >> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c >> index 93c7675f0c9e..bd3f2e673874 100644 >> --- a/kernel/bpf/trampoline.c >> +++ b/kernel/bpf/trampoline.c >> @@ -358,6 +358,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr) >> >> if (tlinks[BPF_TRAMP_FEXIT].nr_links || >> tlinks[BPF_TRAMP_MODIFY_RETURN].nr_links) >> + /* NOTE: BPF_TRAMP_F_RESTORE_REGS and BPF_TRAMP_F_SKIP_FRAME >> + * should not be set together. >> + */ >> flags = BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_SKIP_FRAME; >> >> if (ip_arg) >> -- >> 2.30.2 >> > . From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8C136C433F5 for ; Thu, 26 May 2022 09:46:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:CC:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=V5iPU0C6J3en+YPyDBoHUVe4eHACpbU6/04xaebxJGo=; b=L+6g09rpqfXHsv ZKGuMbssXmJL1CLH192YAV6vK5jy4q9MYcqb8Oez1017AyziyFnekArXz6UKcRV47yMHQ9ZuWy7/L WvCUMIEDQ2/8lkZNKVz2uOMaRhMDDpoWoup22cIvT98EGimxs80GyDOM6PeVNt10+SnD/XyHzj0dZ z6PbDKo7UD3aJyJFW66NO4QX/FEpud4zaUxCEFLds5QWE4wKKzPr9EJxfIPBzLtP7NCJjD4UaSfKl oE3PETO0miBL6anUgBUvMxS14eCwYUfMGzw8Nixr7PQFrKoveID/VddxhbdKN3F4nUdY7Qw5yBHm+ ymioAEXKI4HSGI3mUUZw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nuA3x-00EFb6-TO; Thu, 26 May 2022 09:45:50 +0000 Received: from szxga03-in.huawei.com ([45.249.212.189]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nuA3h-00EFSk-B1 for linux-arm-kernel@lists.infradead.org; Thu, 26 May 2022 09:45:35 +0000 Received: from kwepemi500013.china.huawei.com (unknown [172.30.72.55]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4L832V41dgzDqKf; Thu, 26 May 2022 17:45:22 +0800 (CST) Received: from [10.67.111.192] (10.67.111.192) by kwepemi500013.china.huawei.com (7.221.188.120) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 26 May 2022 17:45:26 +0800 Message-ID: <985fe022-552c-9d04-16d8-14784c4075f8@huawei.com> Date: Thu, 26 May 2022 17:45:25 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH bpf-next v5 3/6] bpf: Remove is_valid_bpf_tramp_flags() Content-Language: en-US To: Mark Rutland CC: , , , , , Catalin Marinas , Will Deacon , Steven Rostedt , Ingo Molnar , Daniel Borkmann , Alexei Starovoitov , Zi Shen Lim , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , "David S . Miller" , Hideaki YOSHIFUJI , David Ahern , Thomas Gleixner , Borislav Petkov , Dave Hansen , , , Shuah Khan , Jakub Kicinski , Jesper Dangaard Brouer , Pasha Tatashin , Ard Biesheuvel , Daniel Kiss , Steven Price , Sudeep Holla , Marc Zyngier , Peter Collingbourne , Mark Brown , Delyan Kratunov , Kumar Kartikeya Dwivedi References: <20220518131638.3401509-1-xukuohai@huawei.com> <20220518131638.3401509-4-xukuohai@huawei.com> From: Xu Kuohai In-Reply-To: X-Originating-IP: [10.67.111.192] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500013.china.huawei.com (7.221.188.120) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220526_024533_785987_29460363 X-CRM114-Status: GOOD ( 20.04 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 5/25/2022 9:45 PM, Mark Rutland wrote: > On Wed, May 18, 2022 at 09:16:35AM -0400, Xu Kuohai wrote: >> BPF_TRAM_F_XXX flags are not used by user code and are almost constant >> at compile time, so run time validation is a bit overkill. Remove >> is_valid_bpf_tramp_flags() and add some usage comments. >> >> Signed-off-by: Xu Kuohai >> Acked-by: Song Liu > > Am I right in thinking this is independent of the arm64-specific bits, and > could be taken on its own now? > Currenly is_valid_bpf_tramp_flags() is defined in x86 and called before bpf trampoline is constructed. The check logic is irrelevant to the architecture code. So we also need to call this function on arm64. But as Alexei pointed out, the check is not requried, so it's better to remove it before adding bpf trampoline to arm64. > Mark. > >> --- >> arch/x86/net/bpf_jit_comp.c | 20 -------------------- >> kernel/bpf/bpf_struct_ops.c | 3 +++ >> kernel/bpf/trampoline.c | 3 +++ >> 3 files changed, 6 insertions(+), 20 deletions(-) >> >> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c >> index a2b6d197c226..7698ef3b4821 100644 >> --- a/arch/x86/net/bpf_jit_comp.c >> +++ b/arch/x86/net/bpf_jit_comp.c >> @@ -1922,23 +1922,6 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog, >> return 0; >> } >> >> -static bool is_valid_bpf_tramp_flags(unsigned int flags) >> -{ >> - if ((flags & BPF_TRAMP_F_RESTORE_REGS) && >> - (flags & BPF_TRAMP_F_SKIP_FRAME)) >> - return false; >> - >> - /* >> - * BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, >> - * and it must be used alone. >> - */ >> - if ((flags & BPF_TRAMP_F_RET_FENTRY_RET) && >> - (flags & ~BPF_TRAMP_F_RET_FENTRY_RET)) >> - return false; >> - >> - return true; >> -} >> - >> /* Example: >> * __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev); >> * its 'struct btf_func_model' will be nr_args=2 >> @@ -2017,9 +2000,6 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i >> if (nr_args > 6) >> return -ENOTSUPP; >> >> - if (!is_valid_bpf_tramp_flags(flags)) >> - return -EINVAL; >> - >> /* Generated trampoline stack layout: >> * >> * RBP + 8 [ return address ] >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c >> index d9a3c9207240..0572cc5aeb28 100644 >> --- a/kernel/bpf/bpf_struct_ops.c >> +++ b/kernel/bpf/bpf_struct_ops.c >> @@ -341,6 +341,9 @@ int bpf_struct_ops_prepare_trampoline(struct bpf_tramp_links *tlinks, >> >> tlinks[BPF_TRAMP_FENTRY].links[0] = link; >> tlinks[BPF_TRAMP_FENTRY].nr_links = 1; >> + /* BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, >> + * and it must be used alone. >> + */ >> flags = model->ret_size > 0 ? BPF_TRAMP_F_RET_FENTRY_RET : 0; >> return arch_prepare_bpf_trampoline(NULL, image, image_end, >> model, flags, tlinks, NULL); >> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c >> index 93c7675f0c9e..bd3f2e673874 100644 >> --- a/kernel/bpf/trampoline.c >> +++ b/kernel/bpf/trampoline.c >> @@ -358,6 +358,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr) >> >> if (tlinks[BPF_TRAMP_FEXIT].nr_links || >> tlinks[BPF_TRAMP_MODIFY_RETURN].nr_links) >> + /* NOTE: BPF_TRAMP_F_RESTORE_REGS and BPF_TRAMP_F_SKIP_FRAME >> + * should not be set together. >> + */ >> flags = BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_SKIP_FRAME; >> >> if (ip_arg) >> -- >> 2.30.2 >> > . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel