From: Yonghong Song <yonghong.song@linux.dev>
To: kernel test robot <lkp@intel.com>, bpf@vger.kernel.org
Cc: oe-kbuild-all@lists.linux.dev,
Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH bpf-next 4/5] bpf: Limit up to 512 bytes for bpf_global_percpu_ma allocation
Date: Wed, 13 Dec 2023 09:20:59 -0800 [thread overview]
Message-ID: <7aaee542-78e9-4b6b-a2ee-1f47d431f8e3@linux.dev> (raw)
In-Reply-To: <202312131731.Yh7iYbJG-lkp@intel.com>
On 12/13/23 2:15 AM, kernel test robot wrote:
> Hi Yonghong,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on bpf-next/master]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Yonghong-Song/bpf-Refactor-to-have-a-memalloc-cache-destroying-function/20231213-063401
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/20231212223100.2138537-1-yonghong.song%40linux.dev
> patch subject: [PATCH bpf-next 4/5] bpf: Limit up to 512 bytes for bpf_global_percpu_ma allocation
> config: m68k-defconfig (https://download.01.org/0day-ci/archive/20231213/202312131731.Yh7iYbJG-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312131731.Yh7iYbJG-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312131731.Yh7iYbJG-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> kernel/bpf/verifier.c: In function 'check_kfunc_call':
>>> kernel/bpf/verifier.c:12082:115: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
> 12082 | verbose(env, "bpf_percpu_obj_new type size (%d) is greater than %lu\n",
> | ~~^
> | |
> | long unsigned int
> | %u
>
>
> vim +12082 kernel/bpf/verifier.c
Okay, seems '%lu' not portable. Will fix with '%zu' if the code roughly stay the same in the next revision.
>
> 11885
> 11886 static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> 11887 int *insn_idx_p)
> 11888 {
> 11889 const struct btf_type *t, *ptr_type;
> 11890 u32 i, nargs, ptr_type_id, release_ref_obj_id;
> 11891 struct bpf_reg_state *regs = cur_regs(env);
> 11892 const char *func_name, *ptr_type_name;
> 11893 bool sleepable, rcu_lock, rcu_unlock;
> 11894 struct bpf_kfunc_call_arg_meta meta;
> 11895 struct bpf_insn_aux_data *insn_aux;
> 11896 int err, insn_idx = *insn_idx_p;
> 11897 const struct btf_param *args;
> 11898 const struct btf_type *ret_t;
> 11899 struct btf *desc_btf;
> 11900
> 11901 /* skip for now, but return error when we find this in fixup_kfunc_call */
> 11902 if (!insn->imm)
> 11903 return 0;
> 11904
> 11905 err = fetch_kfunc_meta(env, insn, &meta, &func_name);
> 11906 if (err == -EACCES && func_name)
> 11907 verbose(env, "calling kernel function %s is not allowed\n", func_name);
> 11908 if (err)
> 11909 return err;
> 11910 desc_btf = meta.btf;
> 11911 insn_aux = &env->insn_aux_data[insn_idx];
> 11912
> 11913 insn_aux->is_iter_next = is_iter_next_kfunc(&meta);
> 11914
> 11915 if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {
> 11916 verbose(env, "destructive kfunc calls require CAP_SYS_BOOT capability\n");
> 11917 return -EACCES;
> 11918 }
> 11919
> 11920 sleepable = is_kfunc_sleepable(&meta);
> 11921 if (sleepable && !env->prog->aux->sleepable) {
> 11922 verbose(env, "program must be sleepable to call sleepable kfunc %s\n", func_name);
> 11923 return -EACCES;
> 11924 }
> 11925
> 11926 /* Check the arguments */
> 11927 err = check_kfunc_args(env, &meta, insn_idx);
> 11928 if (err < 0)
> 11929 return err;
> 11930
> 11931 if (meta.func_id == special_kfunc_list[KF_bpf_rbtree_add_impl]) {
> 11932 err = push_callback_call(env, insn, insn_idx, meta.subprogno,
> 11933 set_rbtree_add_callback_state);
> 11934 if (err) {
> 11935 verbose(env, "kfunc %s#%d failed callback verification\n",
> 11936 func_name, meta.func_id);
> 11937 return err;
> 11938 }
> 11939 }
> 11940
> 11941 rcu_lock = is_kfunc_bpf_rcu_read_lock(&meta);
> 11942 rcu_unlock = is_kfunc_bpf_rcu_read_unlock(&meta);
> 11943
> 11944 if (env->cur_state->active_rcu_lock) {
> 11945 struct bpf_func_state *state;
> 11946 struct bpf_reg_state *reg;
> 11947 u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
> 11948
> 11949 if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
> 11950 verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
> 11951 return -EACCES;
> 11952 }
> 11953
> 11954 if (rcu_lock) {
> 11955 verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
> 11956 return -EINVAL;
> 11957 } else if (rcu_unlock) {
> 11958 bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
> 11959 if (reg->type & MEM_RCU) {
> 11960 reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
> 11961 reg->type |= PTR_UNTRUSTED;
> 11962 }
> 11963 }));
> 11964 env->cur_state->active_rcu_lock = false;
> 11965 } else if (sleepable) {
> 11966 verbose(env, "kernel func %s is sleepable within rcu_read_lock region\n", func_name);
> 11967 return -EACCES;
> 11968 }
> 11969 } else if (rcu_lock) {
> 11970 env->cur_state->active_rcu_lock = true;
> 11971 } else if (rcu_unlock) {
> 11972 verbose(env, "unmatched rcu read unlock (kernel function %s)\n", func_name);
> 11973 return -EINVAL;
> 11974 }
> 11975
> 11976 /* In case of release function, we get register number of refcounted
> 11977 * PTR_TO_BTF_ID in bpf_kfunc_arg_meta, do the release now.
> 11978 */
> 11979 if (meta.release_regno) {
> 11980 err = release_reference(env, regs[meta.release_regno].ref_obj_id);
> 11981 if (err) {
> 11982 verbose(env, "kfunc %s#%d reference has not been acquired before\n",
> 11983 func_name, meta.func_id);
> 11984 return err;
> 11985 }
> 11986 }
> 11987
> 11988 if (meta.func_id == special_kfunc_list[KF_bpf_list_push_front_impl] ||
> 11989 meta.func_id == special_kfunc_list[KF_bpf_list_push_back_impl] ||
> 11990 meta.func_id == special_kfunc_list[KF_bpf_rbtree_add_impl]) {
> 11991 release_ref_obj_id = regs[BPF_REG_2].ref_obj_id;
> 11992 insn_aux->insert_off = regs[BPF_REG_2].off;
> 11993 insn_aux->kptr_struct_meta = btf_find_struct_meta(meta.arg_btf, meta.arg_btf_id);
> 11994 err = ref_convert_owning_non_owning(env, release_ref_obj_id);
> 11995 if (err) {
> 11996 verbose(env, "kfunc %s#%d conversion of owning ref to non-owning failed\n",
> 11997 func_name, meta.func_id);
> 11998 return err;
> 11999 }
> 12000
> 12001 err = release_reference(env, release_ref_obj_id);
> 12002 if (err) {
> 12003 verbose(env, "kfunc %s#%d reference has not been acquired before\n",
> 12004 func_name, meta.func_id);
> 12005 return err;
> 12006 }
> 12007 }
> 12008
> 12009 if (meta.func_id == special_kfunc_list[KF_bpf_throw]) {
> 12010 if (!bpf_jit_supports_exceptions()) {
> 12011 verbose(env, "JIT does not support calling kfunc %s#%d\n",
> 12012 func_name, meta.func_id);
> 12013 return -ENOTSUPP;
> 12014 }
> 12015 env->seen_exception = true;
> 12016
> 12017 /* In the case of the default callback, the cookie value passed
> 12018 * to bpf_throw becomes the return value of the program.
> 12019 */
> 12020 if (!env->exception_callback_subprog) {
> 12021 err = check_return_code(env, BPF_REG_1, "R1");
> 12022 if (err < 0)
> 12023 return err;
> 12024 }
> 12025 }
> 12026
> 12027 for (i = 0; i < CALLER_SAVED_REGS; i++)
> 12028 mark_reg_not_init(env, regs, caller_saved[i]);
> 12029
> 12030 /* Check return type */
> 12031 t = btf_type_skip_modifiers(desc_btf, meta.func_proto->type, NULL);
> 12032
> 12033 if (is_kfunc_acquire(&meta) && !btf_type_is_struct_ptr(meta.btf, t)) {
> 12034 /* Only exception is bpf_obj_new_impl */
> 12035 if (meta.btf != btf_vmlinux ||
> 12036 (meta.func_id != special_kfunc_list[KF_bpf_obj_new_impl] &&
> 12037 meta.func_id != special_kfunc_list[KF_bpf_percpu_obj_new_impl] &&
> 12038 meta.func_id != special_kfunc_list[KF_bpf_refcount_acquire_impl])) {
> 12039 verbose(env, "acquire kernel function does not return PTR_TO_BTF_ID\n");
> 12040 return -EINVAL;
> 12041 }
> 12042 }
> 12043
> 12044 if (btf_type_is_scalar(t)) {
> 12045 mark_reg_unknown(env, regs, BPF_REG_0);
> 12046 mark_btf_func_reg_size(env, BPF_REG_0, t->size);
> 12047 } else if (btf_type_is_ptr(t)) {
> 12048 ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id);
> 12049
> 12050 if (meta.btf == btf_vmlinux && btf_id_set_contains(&special_kfunc_set, meta.func_id)) {
> 12051 if (meta.func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
> 12052 meta.func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
> 12053 struct btf_struct_meta *struct_meta;
> 12054 struct btf *ret_btf;
> 12055 u32 ret_btf_id;
> 12056
> 12057 if (meta.func_id == special_kfunc_list[KF_bpf_obj_new_impl] && !bpf_global_ma_set)
> 12058 return -ENOMEM;
> 12059
> 12060 if (((u64)(u32)meta.arg_constant.value) != meta.arg_constant.value) {
> 12061 verbose(env, "local type ID argument must be in range [0, U32_MAX]\n");
> 12062 return -EINVAL;
> 12063 }
> 12064
> 12065 ret_btf = env->prog->aux->btf;
> 12066 ret_btf_id = meta.arg_constant.value;
> 12067
> 12068 /* This may be NULL due to user not supplying a BTF */
> 12069 if (!ret_btf) {
> 12070 verbose(env, "bpf_obj_new/bpf_percpu_obj_new requires prog BTF\n");
> 12071 return -EINVAL;
> 12072 }
> 12073
> 12074 ret_t = btf_type_by_id(ret_btf, ret_btf_id);
> 12075 if (!ret_t || !__btf_type_is_struct(ret_t)) {
> 12076 verbose(env, "bpf_obj_new/bpf_percpu_obj_new type ID argument must be of a struct\n");
> 12077 return -EINVAL;
> 12078 }
> 12079
> 12080 if (meta.func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
> 12081 if (ret_t->size > BPF_GLOBAL_PERCPU_MA_MAX_SIZE) {
> 12082 verbose(env, "bpf_percpu_obj_new type size (%d) is greater than %lu\n",
> 12083 ret_t->size, BPF_GLOBAL_PERCPU_MA_MAX_SIZE);
> 12084 return -EINVAL;
> 12085 }
> 12086 mutex_lock(&bpf_percpu_ma_lock);
> 12087 err = bpf_mem_alloc_percpu_unit_init(&bpf_global_percpu_ma, ret_t->size);
> 12088 mutex_unlock(&bpf_percpu_ma_lock);
> 12089 if (err)
> 12090 return err;
> 12091 }
> 12092
[...]
next prev parent reply other threads:[~2023-12-13 17:21 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-12 22:30 [PATCH bpf-next 0/5] bpf: Reduce memory usage for bpf_global_percpu_ma Yonghong Song
2023-12-12 22:30 ` [PATCH bpf-next 1/5] bpf: Refactor to have a memalloc cache destroying function Yonghong Song
2023-12-13 11:05 ` Hou Tao
2023-12-12 22:30 ` [PATCH bpf-next 2/5] bpf: Allow per unit prefill for non-fix-size percpu memory allocator Yonghong Song
2023-12-13 11:03 ` Hou Tao
2023-12-13 17:25 ` Yonghong Song
2023-12-12 22:30 ` [PATCH bpf-next 3/5] bpf: Refill only one percpu element in memalloc Yonghong Song
2023-12-13 11:05 ` Hou Tao
2023-12-13 17:26 ` Yonghong Song
2023-12-12 22:31 ` [PATCH bpf-next 4/5] bpf: Limit up to 512 bytes for bpf_global_percpu_ma allocation Yonghong Song
2023-12-13 10:15 ` kernel test robot
2023-12-13 17:20 ` Yonghong Song [this message]
2023-12-13 11:09 ` Hou Tao
2023-12-13 17:28 ` Yonghong Song
2023-12-13 14:13 ` kernel test robot
2023-12-12 22:31 ` [PATCH bpf-next 5/5] selftests/bpf: Cope with 512 bytes limit with bpf_global_percpu_ma Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7aaee542-78e9-4b6b-a2ee-1f47d431f8e3@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=lkp@intel.com \
--cc=martin.lau@kernel.org \
--cc=oe-kbuild-all@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.