From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-177.mta0.migadu.com (out-177.mta0.migadu.com [91.218.175.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3DD1B13790B for ; Tue, 22 Oct 2024 03:00:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729566004; cv=none; b=ISjRYYKAuMjqzHDwAGq9AkZTI3iY/gdyOSTJ/V+sGsDD4iM2aZoXGA912f4wfpOjVmgmBrL97ah/E870ZiAuW14i+oXDsbQeKHjgJlLX/UAciCFMjeWk+GjuNun7SfYwO1QMF/po5dUkLFA3brjchjACjQfjrjuIRQWlA24zvRM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729566004; c=relaxed/simple; bh=u92ZrMzCBtXnJXNwFtYQHj6Z6fMqCqq77dkJPFf52Ic=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=COJ/nZUdisuqMCkpe1Srn5vhwz5Aqcqs1L4nO+9C0ZHKYlcOWuhiBZlLq8UnFsAzGpx9uRO6E7GZQNaYQWwuAblbzq4RW5SojFd9Jo62b6qlEupG8CyeQLwAzrJvcqE6MWrvk82mG4V61RlaKdNylfM5Ku0+qTW+zOwpm2mM+qw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=ciFO88R4; arc=none smtp.client-ip=91.218.175.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="ciFO88R4" Message-ID: <969d8053-bb47-4339-9fdc-eb71b3e51cc7@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1729566000; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C2lC2BXpAUgtQMYVVcRhKZL5Xe+vJklNqGWAnz2w4IA=; b=ciFO88R40GkH7bQZXDdHQTrA/ZsYrsfLQoLs2NPWusr2ZCBvP8sbVmzDwdPtY6c09WfUCS v9kFdA4o2dMqrAP/IpgtTyeRywrtQjdTLr+S6ocnjYe/80PwLW8mg2t/eNEXFupHRLxJ3s sBV1tQGGIosvs1Wzr2eTCaQnTPIIJ84= Date: Mon, 21 Oct 2024 19:59:53 -0700 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH bpf-next v6 3/9] bpf: Support private stack for struct ops programs Content-Language: en-GB To: Alexei Starovoitov Cc: bpf , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Kernel Team , Martin KaFai Lau , Tejun Heo References: <20241020191341.2104841-1-yonghong.song@linux.dev> <20241020191400.2105605-1-yonghong.song@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yonghong Song In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On 10/21/24 6:34 PM, Alexei Starovoitov wrote: > On Sun, Oct 20, 2024 at 12:16 PM Yonghong Song wrote: >> To identify whether a st_ops program requests private stack or not, >> the st_ops stub function is checked. If the stub function has the >> following name >> ____priv_stack >> then the corresponding st_ops member func requests to use private >> stack. The information that the private stack is requested or not >> is encoded in struct bpf_struct_ops_func_info which will later be >> used by verifier. >> >> Signed-off-by: Yonghong Song >> --- >> include/linux/bpf.h | 2 ++ >> kernel/bpf/bpf_struct_ops.c | 35 +++++++++++++++++++++++++---------- >> kernel/bpf/verifier.c | 8 +++++++- >> 3 files changed, 34 insertions(+), 11 deletions(-) >> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >> index f3884ce2603d..376e43fc72b9 100644 >> --- a/include/linux/bpf.h >> +++ b/include/linux/bpf.h >> @@ -1491,6 +1491,7 @@ struct bpf_prog_aux { >> bool exception_boundary; >> bool is_extended; /* true if extended by freplace program */ >> bool priv_stack_eligible; >> + bool priv_stack_always; >> u64 prog_array_member_cnt; /* counts how many times as member of prog_array */ >> struct mutex ext_mutex; /* mutex for is_extended and prog_array_member_cnt */ >> struct bpf_arena *arena; >> @@ -1776,6 +1777,7 @@ struct bpf_struct_ops { >> struct bpf_struct_ops_func_info { >> struct bpf_ctx_arg_aux *info; >> u32 cnt; >> + bool priv_stack_always; >> }; >> >> struct bpf_struct_ops_desc { >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c >> index 8279b5a57798..2cd4bd086c7a 100644 >> --- a/kernel/bpf/bpf_struct_ops.c >> +++ b/kernel/bpf/bpf_struct_ops.c >> @@ -145,33 +145,44 @@ void bpf_struct_ops_image_free(void *image) >> } >> >> #define MAYBE_NULL_SUFFIX "__nullable" >> -#define MAX_STUB_NAME 128 >> +#define MAX_STUB_NAME 140 >> >> /* Return the type info of a stub function, if it exists. >> * >> - * The name of a stub function is made up of the name of the struct_ops and >> - * the name of the function pointer member, separated by "__". For example, >> - * if the struct_ops type is named "foo_ops" and the function pointer >> - * member is named "bar", the stub function name would be "foo_ops__bar". >> + * The name of a stub function is made up of the name of the struct_ops, >> + * the name of the function pointer member and optionally "priv_stack" >> + * suffix, separated by "__". For example, if the struct_ops type is named >> + * "foo_ops" and the function pointer member is named "bar", the stub >> + * function name would be "foo_ops__bar". If a suffix "priv_stack" exists, >> + * the stub function name would be "foo_ops__bar__priv_stack". >> */ >> static const struct btf_type * >> find_stub_func_proto(const struct btf *btf, const char *st_op_name, >> - const char *member_name) >> + const char *member_name, bool *priv_stack_always) >> { >> char stub_func_name[MAX_STUB_NAME]; >> const struct btf_type *func_type; >> s32 btf_id; >> int cp; >> >> - cp = snprintf(stub_func_name, MAX_STUB_NAME, "%s__%s", >> + cp = snprintf(stub_func_name, MAX_STUB_NAME, "%s__%s__priv_stack", >> st_op_name, member_name); > I don't think this approach fits. > pw-bot: cr Okay, I will use check_member() callback function then. It should avoid this hack. > > Also looking at original > commit 1611603537a4 ("bpf: Create argument information for nullable arguments.") > that added this %s__%s notation I'm not sure why we went > with that approach. > > Just to avoid adding __nullable suffix in the actual callback > and using cfi stub callback names with such suffixes as > a "proxy" for the real callback? > > Did we ever use this functionality for anything other than > bpf_testmod_ops__test_maybe_null selftest ? > > Martin ?