Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8b57320c-df41-a19f-e433-07782a709a5c@intel.com>
Date:   Fri, 25 Nov 2022 21:28:28 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.5.0
Subject: Re: [PATCH bpf v2] selftests/bpf: Fix "missing ENDBR" BUG for
 destructor kfunc
Content-Language: en-US
To:     Jiri Olsa <olsajiri@gmail.com>
CC:     <jpoimboe@kernel.org>, <memxor@gmail.com>, <bpf@vger.kernel.org>,
        "Pengfei Xu" <pengfei.xu@intel.com>,
        Alexei Starovoitov <ast@kernel.org>,
        "Daniel Borkmann" <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        "KP Singh" <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        "Chen, Hu1" <hu1.chen@intel.com>
References: <20221122073244.21279-1-hu1.chen@intel.com>
 <Y3zTF0CjQFt/dR2M@krava>
From:   "Chen, Hu1" <hu1.chen@intel.com>
In-Reply-To: <Y3zTF0CjQFt/dR2M@krava>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QUNKallyNGY1ci81TEVrRVliSVZLVDhPcUcwdFJpRk1vdGpkRDlacERDWXlo?=
 =?utf-8?B?cXd5Vm92bjZZOW9RR2w0THF5ZjFlTEdDTkFUNFE2d0JNeHhjc1pWRDdMMDJO?=
 =?utf-8?B?UC8xdlQ4VUY2bFRTRndMY1BBTzJDUDB4bUY4R1dpZFVieVNTcHNOazE4V3BU?=
 =?utf-8?B?OXZ6QXY3RWZqS1RGM0w1YVJRV2RjcUYxZU5VNG41bWIwQ3QrWUN6eEJIL2x2?=
 =?utf-8?B?SzRlZGxHTTg2K2IwVmdjNWZZWm5OcGpEdFJUemN1KzJsK2hDaXRQWHNMKzlu?=
 =?utf-8?B?a1JLR005blRHdVZnd2p6NU15RjUvb1pGWFpkZFJJY1BlVG96MkF1UnM1OUc0?=
 =?utf-8?B?d2VEVjFkSTlZME0rdG1PZHhFMXk3Ym42eHhZMjVJNm5BcmRYYmNlZk9abDlv?=
 =?utf-8?B?U3BtQXlKQVlGcUFzV2wrM3Y0U3BBQU9MeFZrYmh3bzlUUk82K2grWTM1SVNL?=
 =?utf-8?B?dHh0bk96VXBHeTh3K1k1bTl5U0czK0twZUJSVGdjVUwzeEdXVGQ5bzd2N1lm?=
 =?utf-8?B?cG5EMzJPK0ZUOG9FWXF1aHBSU0pIRGhNQkdOZE51QTFSZk1wWkR1L2MreTZh?=
 =?utf-8?B?ZEVrSmdnam54M2dMUU44bndESWZ5Tnc1Mk1neVFjRVQ5TFQ3RE54U2hadHZE?=
 =?utf-8?B?RG15ZEphMHZXMHc1SUJabGlzeGx6REVrVU5uNWF3RGpOZG9TdTR1UkEwSjVz?=
 =?utf-8?B?Y3VIc0ZkVTBTNUl3Y0RCY0JXdnU5VFlGdVBvVlM1SWMvZjNuUjBCbm1obG10?=
 =?utf-8?B?YlFSVzc5djNha2xMZWNjKzB1SFJsdm51R3JJVEVJQ3lEMUpLbWtGWWpEMk5y?=
 =?utf-8?B?ZDlKUFlSbUljb0pGUkxuYllaNVluajFFVmhXMjdwZlRpQ3ZsK3JSb1ZOcVpi?=
 =?utf-8?B?VHVUNmhpazJ5cENwbFErOEZUK0lLdGNjOHMxYXlvZzVVRGZ1QTRaMjFSd3RC?=
 =?utf-8?B?RHlpMzZJaWR6UDhNbzZWZ0dKWS80b2lQczlVR2RLbUtiMWlHSnRYYzlCaldu?=
 =?utf-8?B?TlIwTmxUL2FHOTFuQkVEZ2ZkSjgvSDl0UGh1V3ZFSVBMNWZDdXRxQjZ4MjJD?=
 =?utf-8?B?NlRtUmJsc3h6TjFhdVhRa0lCQkZFT2tMQUdrYzY1UWRPMEJsVncwOG9GZURI?=
 =?utf-8?B?UXBneWJxZ09BMGgzeTYwVWJIenBPRnNSb0RWSHlnV3hzZHJoSGJsS0hGNjBQ?=
 =?utf-8?B?cis5TDZoSEE4NHZ3ZlVDQWNuMkhVMnhJcmkwTEZMTG9RcW43S1k5SEdVZ0tE?=
 =?utf-8?B?NFQyaWVzbWJmNVp4N0J4enBoR1JyVk5TWlpmdHNpNEpoeHR0RTRtSFREaDk5?=
 =?utf-8?B?MXpNcGZmQkNuejl3NXNlS1o0Nmd4aVYzWStpWG5zQm5RWitDbG5OQ2pnY3Fr?=
 =?utf-8?B?RllDd1IwVVZYeVZJRHNZckhteG1OVTc1UmlreCtoVkhrc1dqRlFFc3Irdk1Y?=
 =?utf-8?B?Q1ZxWForYzF1c2xyaUNWTi9PMFJGTDdaV1pRQW5TUzFzTnVRcDhNZFpHUmpJ?=
 =?utf-8?B?d2FWcWRidlgzSlc4dUprb2poUWVWdVcxQjRicHZseUdYWWc0OHI2MzlpSmxr?=
 =?utf-8?B?azg5dml4cVFCWkZoKy9kOWRjZ0E0N0cxN0U5ZDRXTmZ2NjNOTU9GYUFOU0JZ?=
 =?utf-8?B?cmJ6Um80WkViMkpTNjBsNHFFbzlmbDBYOWF3NmpWYU1vSWZhMUhvYWp3UXRW?=
 =?utf-8?B?UjV1Q1RycjhqSHJNWGdVQ202aTJKallpeGJ2ZkQvMXg0THc0bW1WaVpFSTk2?=
 =?utf-8?B?QzdjVlZYUEpKSHBkbi8walpHNU8zd1ZXUHdiUDJBR3k0M3RRWEl5YUtWUTF3?=
 =?utf-8?B?SjJFcXpYMHYrZzBIcnBJOGxrR0ZpQnBuZW9lZmx3d1lrVXNEa05XL1FtSVRS?=
 =?utf-8?B?TEwrY1VJZGZQSE9DdkE2MGhNUURacHN0c0ZtUEMrVjZXMlE0dzNlMWYzdVFk?=
 =?utf-8?B?cjdVNVpkSTVjRHVuZTJrYzJtOHZzcUVleUJpcHB0aTRBaWt0VjZZWGpJU0pQ?=
 =?utf-8?B?Z3NadVBFQnRoMHZMRnUzNytURjhLQXRHK1UyL2twTVNoSE1IcU5zcHNkT2dn?=
 =?utf-8?B?YkxYaGYvYTZiVjYzc2Y4TU9pb0Z5YStHTFJKdnRSK2VaSTlXYk15ZTYvdUZT?=
 =?utf-8?Q?XqU5FwCBjDsyWd1msRLGcLFH9?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7f873cd2-fda4-4a1e-3a0e-08dacee8f72a
X-MS-Exchange-CrossTenant-AuthSource: BY5PR11MB4449.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Nov 2022 13:28:40.3633
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vcL5AYhKbnlmDcvk1PvmCbU2FqgX1BYwpNoA7uKrQvulxvGvcZvgG2fMnNMb+innQdFmaubi2FuUP6WM1eXG3Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7422
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/22/2022 9:48 PM, Jiri Olsa wrote:
> On Mon, Nov 21, 2022 at 11:32:43PM -0800, Chen Hu wrote:
>> With CONFIG_X86_KERNEL_IBT enabled, the test_verifier triggers the
>> following BUG:
>>
>>   traps: Missing ENDBR: bpf_kfunc_call_test_release+0x0/0x30
>>   ------------[ cut here ]------------
>>   kernel BUG at arch/x86/kernel/traps.c:254!
>>   invalid opcode: 0000 [#1] PREEMPT SMP
>>   <TASK>
>>    asm_exc_control_protection+0x26/0x50
>>   RIP: 0010:bpf_kfunc_call_test_release+0x0/0x30
>>   Code: 00 48 c7 c7 18 f2 e1 b4 e8 0d ca 8c ff 48 c7 c0 00 f2 e1 b4 c3
>> 	0f 1f 44 00 00 66 0f 1f 00 0f 1f 44 00 00 0f 0b 31 c0 c3 66 90
>>        <66> 0f 1f 00 0f 1f 44 00 00 48 85 ff 74 13 4c 8d 47 18 b8 ff ff ff
>>    bpf_map_free_kptrs+0x2e/0x70
>>    array_map_free+0x57/0x140
>>    process_one_work+0x194/0x3a0
>>    worker_thread+0x54/0x3a0
>>    ? rescuer_thread+0x390/0x390
>>    kthread+0xe9/0x110
>>    ? kthread_complete_and_exit+0x20/0x20
>>
>> This is because there are no compile-time references to the destructor
>> kfuncs, bpf_kfunc_call_test_release() for example. So objtool marked
>> them sealable and ENDBR in the functions were sealed (converted to NOP)
>> by apply_ibt_endbr().
>>
>> This fix creates dummy compile-time references to destructor kfuncs so
>> ENDBR stay there.
>>
>> Fixes: 05a945deefaa ("selftests/bpf: Add verifier tests for kptr")
>> Signed-off-by: Chen Hu <hu1.chen@intel.com>
>> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
>> ---
>> v2:
>> - Use generic macro name and place the macro after function body as
>> - suggested by Jiri Olsa
>>
>> v1: https://lore.kernel.org/all/20221121085113.611504-1-hu1.chen@intel.com/
>>
>>  include/linux/btf_ids.h | 7 +++++++
>>  net/bpf/test_run.c      | 4 ++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
>> index 2aea877d644f..db02691b506d 100644
>> --- a/include/linux/btf_ids.h
>> +++ b/include/linux/btf_ids.h
>> @@ -266,4 +266,11 @@ MAX_BTF_TRACING_TYPE,
>>  
>>  extern u32 btf_tracing_ids[];
>>  
>> +#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS)
>> +#define FUNC_IBT_NOSEAL(name)					\
>> +	asm(IBT_NOSEAL(#name));
>> +#else
>> +#define FUNC_IBT_NOSEAL(name)
>> +#endif /* CONFIG_X86_KERNEL_IBT */
> 
> hum, IBT_NOSEAL is x86 specific, so this will probably fail build
> on other archs.. I think we could ifdef it with CONFIG_X86, but
> it should go to some IBT related header? surely not to btf_ids.h
> 
> cc-ing Peter and Josh
> 
> thanks,
> jirka
>

The lkp reports build success because X86_KERNEL_IBT alredy depends on
X86_64.

Currently, arch/x86/include/asm/ibt.h which defines macro IBT_NOSEAL is
x86 specific. How about we just put asm at test_run.c directly (ugly?):

#if defined(CONFIG_X86_KERNEL_IBT) && !defined(__DISABLE_EXPORTS)
asm(IBT_NOSEAL("bpf_kfunc_call_test_release"));
asm(IBT_NOSEAL("bpf_kfunc_call_memb_release"));
#endif

thanks
Chen Hu

> 
>> +
>>  #endif
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index 13d578ce2a09..07263b7cc12d 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -597,10 +597,14 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
>>  	refcount_dec(&p->cnt);
>>  }
>>  
>> +FUNC_IBT_NOSEAL(bpf_kfunc_call_test_release)
>> +
>>  noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p)
>>  {
>>  }
>>  
>> +FUNC_IBT_NOSEAL(bpf_kfunc_call_memb_release)
>> +
>>  noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
>>  {
>>  	WARN_ON_ONCE(1);
>> -- 
>> 2.34.1
>>