All of lore.kernel.org
 help / color / mirror / Atom feed
From: kernel test robot <lkp@intel.com>
To: Amery Hung <ameryhung@gmail.com>, bpf@vger.kernel.org
Cc: oe-kbuild-all@lists.linux.dev, alexei.starovoitov@gmail.com,
	andrii@kernel.org, daniel@iogearbox.net, tj@kernel.org,
	martin.lau@kernel.org, ameryhung@gmail.com, kernel-team@meta.com
Subject: Re: [PATCH bpf-next v1 1/4] bpf: Save struct_ops instance pointer in bpf_prog_aux
Date: Wed, 11 Jun 2025 15:16:37 +0800	[thread overview]
Message-ID: <202506111427.2VNV9Biy-lkp@intel.com> (raw)
In-Reply-To: <20250609232746.1030044-1-ameryhung@gmail.com>

Hi Amery,

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/Amery-Hung/bpf-Allow-verifier-to-fixup-kernel-module-kfuncs/20250610-072957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250609232746.1030044-1-ameryhung%40gmail.com
patch subject: [PATCH bpf-next v1 1/4] bpf: Save struct_ops instance pointer in bpf_prog_aux
config: x86_64-randconfig-121-20250611 (https://download.01.org/0day-ci/archive/20250611/202506111427.2VNV9Biy-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250611/202506111427.2VNV9Biy-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/202506111427.2VNV9Biy-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/bpf/bpf_struct_ops.c:824:29: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void [noderef] __rcu *__new @@     got void *[assigned] kdata @@
   kernel/bpf/bpf_struct_ops.c:824:29: sparse:     expected void [noderef] __rcu *__new
   kernel/bpf/bpf_struct_ops.c:824:29: sparse:     got void *[assigned] kdata

vim +824 kernel/bpf/bpf_struct_ops.c

   686	
   687	static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
   688						   void *value, u64 flags)
   689	{
   690		struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
   691		const struct bpf_struct_ops_desc *st_ops_desc = st_map->st_ops_desc;
   692		const struct bpf_struct_ops *st_ops = st_ops_desc->st_ops;
   693		struct bpf_struct_ops_value *uvalue, *kvalue;
   694		const struct btf_type *module_type;
   695		const struct btf_member *member;
   696		const struct btf_type *t = st_ops_desc->type;
   697		struct bpf_tramp_links *tlinks;
   698		void *udata, *kdata;
   699		int prog_fd, err;
   700		u32 i, trampoline_start, image_off = 0;
   701		void *cur_image = NULL, *image = NULL;
   702		struct bpf_link **plink;
   703		struct bpf_ksym **pksym;
   704		const char *tname, *mname;
   705	
   706		if (flags)
   707			return -EINVAL;
   708	
   709		if (st_ops->flags & ~BPF_STRUCT_OPS_FLAG_MASK)
   710			return -EINVAL;
   711	
   712		if (*(u32 *)key != 0)
   713			return -E2BIG;
   714	
   715		err = check_zero_holes(st_map->btf, st_ops_desc->value_type, value);
   716		if (err)
   717			return err;
   718	
   719		uvalue = value;
   720		err = check_zero_holes(st_map->btf, t, uvalue->data);
   721		if (err)
   722			return err;
   723	
   724		if (uvalue->common.state || refcount_read(&uvalue->common.refcnt))
   725			return -EINVAL;
   726	
   727		tlinks = kcalloc(BPF_TRAMP_MAX, sizeof(*tlinks), GFP_KERNEL);
   728		if (!tlinks)
   729			return -ENOMEM;
   730	
   731		uvalue = (struct bpf_struct_ops_value *)st_map->uvalue;
   732		kvalue = (struct bpf_struct_ops_value *)&st_map->kvalue;
   733	
   734		mutex_lock(&st_map->lock);
   735	
   736		if (kvalue->common.state != BPF_STRUCT_OPS_STATE_INIT) {
   737			err = -EBUSY;
   738			goto unlock;
   739		}
   740	
   741		memcpy(uvalue, value, map->value_size);
   742	
   743		udata = &uvalue->data;
   744		kdata = &kvalue->data;
   745	
   746		plink = st_map->links;
   747		pksym = st_map->ksyms;
   748		tname = btf_name_by_offset(st_map->btf, t->name_off);
   749		module_type = btf_type_by_id(btf_vmlinux, st_ops_ids[IDX_MODULE_ID]);
   750		for_each_member(i, t, member) {
   751			const struct btf_type *mtype, *ptype;
   752			struct bpf_prog *prog;
   753			struct bpf_tramp_link *link;
   754			struct bpf_ksym *ksym;
   755			u32 moff;
   756	
   757			moff = __btf_member_bit_offset(t, member) / 8;
   758			mname = btf_name_by_offset(st_map->btf, member->name_off);
   759			ptype = btf_type_resolve_ptr(st_map->btf, member->type, NULL);
   760			if (ptype == module_type) {
   761				if (*(void **)(udata + moff))
   762					goto reset_unlock;
   763				*(void **)(kdata + moff) = BPF_MODULE_OWNER;
   764				continue;
   765			}
   766	
   767			err = st_ops->init_member(t, member, kdata, udata);
   768			if (err < 0)
   769				goto reset_unlock;
   770	
   771			/* The ->init_member() has handled this member */
   772			if (err > 0)
   773				continue;
   774	
   775			/* If st_ops->init_member does not handle it,
   776			 * we will only handle func ptrs and zero-ed members
   777			 * here.  Reject everything else.
   778			 */
   779	
   780			/* All non func ptr member must be 0 */
   781			if (!ptype || !btf_type_is_func_proto(ptype)) {
   782				u32 msize;
   783	
   784				mtype = btf_type_by_id(st_map->btf, member->type);
   785				mtype = btf_resolve_size(st_map->btf, mtype, &msize);
   786				if (IS_ERR(mtype)) {
   787					err = PTR_ERR(mtype);
   788					goto reset_unlock;
   789				}
   790	
   791				if (memchr_inv(udata + moff, 0, msize)) {
   792					err = -EINVAL;
   793					goto reset_unlock;
   794				}
   795	
   796				continue;
   797			}
   798	
   799			prog_fd = (int)(*(unsigned long *)(udata + moff));
   800			/* Similar check as the attr->attach_prog_fd */
   801			if (!prog_fd)
   802				continue;
   803	
   804			prog = bpf_prog_get(prog_fd);
   805			if (IS_ERR(prog)) {
   806				err = PTR_ERR(prog);
   807				goto reset_unlock;
   808			}
   809	
   810			if (prog->type != BPF_PROG_TYPE_STRUCT_OPS ||
   811			    prog->aux->attach_btf_id != st_ops_desc->type_id ||
   812			    prog->expected_attach_type != i) {
   813				bpf_prog_put(prog);
   814				err = -EINVAL;
   815				goto reset_unlock;
   816			}
   817	
   818			if (st_ops->flags & BPF_STRUCT_OPS_F_THIS_PTR) {
   819				/* Make sure a struct_ops map will not have programs with
   820				 * different this_st_ops. Once a program is associated with
   821				 * a struct_ops map, it cannot be used in another struct_ops
   822				 * map also with BPF_STRUCT_OPS_F_THIS_PTR
   823				 */
 > 824				if (cmpxchg(&prog->aux->this_st_ops, NULL, kdata)) {
   825					bpf_prog_put(prog);
   826					err = -EINVAL;
   827					goto reset_unlock;
   828				}
   829			}
   830	
   831			link = kzalloc(sizeof(*link), GFP_USER);
   832			if (!link) {
   833				bpf_prog_put(prog);
   834				err = -ENOMEM;
   835				goto reset_unlock;
   836			}
   837			bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
   838				      &bpf_struct_ops_link_lops, prog);
   839			*plink++ = &link->link;
   840	
   841			ksym = kzalloc(sizeof(*ksym), GFP_USER);
   842			if (!ksym) {
   843				err = -ENOMEM;
   844				goto reset_unlock;
   845			}
   846			*pksym++ = ksym;
   847	
   848			trampoline_start = image_off;
   849			err = bpf_struct_ops_prepare_trampoline(tlinks, link,
   850							&st_ops->func_models[i],
   851							*(void **)(st_ops->cfi_stubs + moff),
   852							&image, &image_off,
   853							st_map->image_pages_cnt < MAX_TRAMP_IMAGE_PAGES);
   854			if (err)
   855				goto reset_unlock;
   856	
   857			if (cur_image != image) {
   858				st_map->image_pages[st_map->image_pages_cnt++] = image;
   859				cur_image = image;
   860				trampoline_start = 0;
   861			}
   862	
   863			*(void **)(kdata + moff) = image + trampoline_start + cfi_get_offset();
   864	
   865			/* put prog_id to udata */
   866			*(unsigned long *)(udata + moff) = prog->aux->id;
   867	
   868			/* init ksym for this trampoline */
   869			bpf_struct_ops_ksym_init(tname, mname,
   870						 image + trampoline_start,
   871						 image_off - trampoline_start,
   872						 ksym);
   873		}
   874	
   875		if (st_ops->validate) {
   876			err = st_ops->validate(kdata);
   877			if (err)
   878				goto reset_unlock;
   879		}
   880		for (i = 0; i < st_map->image_pages_cnt; i++) {
   881			err = arch_protect_bpf_trampoline(st_map->image_pages[i],
   882							  PAGE_SIZE);
   883			if (err)
   884				goto reset_unlock;
   885		}
   886	
   887		if (st_map->map.map_flags & BPF_F_LINK) {
   888			err = 0;
   889			/* Let bpf_link handle registration & unregistration.
   890			 *
   891			 * Pair with smp_load_acquire() during lookup_elem().
   892			 */
   893			smp_store_release(&kvalue->common.state, BPF_STRUCT_OPS_STATE_READY);
   894			goto unlock;
   895		}
   896	
   897		err = st_ops->reg(kdata, NULL);
   898		if (likely(!err)) {
   899			/* This refcnt increment on the map here after
   900			 * 'st_ops->reg()' is secure since the state of the
   901			 * map must be set to INIT at this moment, and thus
   902			 * bpf_struct_ops_map_delete_elem() can't unregister
   903			 * or transition it to TOBEFREE concurrently.
   904			 */
   905			bpf_map_inc(map);
   906			/* Pair with smp_load_acquire() during lookup_elem().
   907			 * It ensures the above udata updates (e.g. prog->aux->id)
   908			 * can be seen once BPF_STRUCT_OPS_STATE_INUSE is set.
   909			 */
   910			smp_store_release(&kvalue->common.state, BPF_STRUCT_OPS_STATE_INUSE);
   911			goto unlock;
   912		}
   913	
   914		/* Error during st_ops->reg(). Can happen if this struct_ops needs to be
   915		 * verified as a whole, after all init_member() calls. Can also happen if
   916		 * there was a race in registering the struct_ops (under the same name) to
   917		 * a sub-system through different struct_ops's maps.
   918		 */
   919	
   920	reset_unlock:
   921		bpf_struct_ops_map_free_ksyms(st_map);
   922		bpf_struct_ops_map_free_image(st_map);
   923		bpf_struct_ops_map_put_progs(st_map);
   924		if (st_ops->flags & BPF_STRUCT_OPS_F_THIS_PTR)
   925			bpf_struct_ops_map_clear_this_ptr(st_map);
   926		memset(uvalue, 0, map->value_size);
   927		memset(kvalue, 0, map->value_size);
   928	unlock:
   929		kfree(tlinks);
   930		mutex_unlock(&st_map->lock);
   931		if (!err)
   932			bpf_struct_ops_map_add_ksyms(st_map);
   933		return err;
   934	}
   935	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

  parent reply	other threads:[~2025-06-11  7:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-09 23:27 [PATCH bpf-next v1 1/4] bpf: Save struct_ops instance pointer in bpf_prog_aux Amery Hung
2025-06-09 23:27 ` [PATCH bpf-next v1 2/4] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
2025-06-09 23:27 ` [PATCH bpf-next v1 3/4] selftests/bpf: Test accessing struct_ops this pointer Amery Hung
2025-06-09 23:27 ` [PATCH bpf-next v1 4/4] selftests/bpf: Test accessing struct_ops this pointer in timer callback Amery Hung
     [not found]   ` <8c4943cb40967d152abe032b4208a7cdd89b539da26783afaa61e37eb6663cbf@mail.kernel.org>
2025-06-10 16:45     ` Amery Hung
2025-06-10 22:15 ` [PATCH bpf-next v1 1/4] bpf: Save struct_ops instance pointer in bpf_prog_aux Tejun Heo
2025-06-11  7:16 ` kernel test robot [this message]
2025-06-12 23:08 ` Andrii Nakryiko
2025-06-18 22:18   ` Amery Hung
2025-06-24 15:38     ` Andrii Nakryiko
2025-06-24 15:59       ` Alexei Starovoitov
2025-06-24 19:41         ` Andrii Nakryiko

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=202506111427.2VNV9Biy-lkp@intel.com \
    --to=lkp@intel.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=martin.lau@kernel.org \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=tj@kernel.org \
    /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.