| Message ID | 20230119235833.2948341-2-void@manifault.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Enable cpumasks to be used as kptrs | expand |
Hi David,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20230119235833.2948341-2-void%40manifault.com
patch subject: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230120/202301200957.At49rpzu-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8f6df14342b1be3516f8e21037edf771df851427
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139
git checkout 8f6df14342b1be3516f8e21037edf771df851427
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash kernel/bpf/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/bpf/btf.c:533:5: warning: no previous prototype for 'bpf_find_btf_id' [-Wmissing-prototypes]
533 | s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p)
| ^~~~~~~~~~~~~~~
kernel/bpf/btf.c: In function 'btf_seq_show':
kernel/bpf/btf.c:6977:29: warning: function 'btf_seq_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
6977 | seq_vprintf((struct seq_file *)show->target, fmt, args);
| ^~~~~~~~
kernel/bpf/btf.c: In function 'btf_snprintf_show':
kernel/bpf/btf.c:7014:9: warning: function 'btf_snprintf_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
7014 | len = vsnprintf(show->target, ssnprintf->len_left, fmt, args);
| ^~~
vim +/bpf_find_btf_id +533 kernel/bpf/btf.c
532
> 533 s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p)
534 {
535 struct btf *btf;
536 s32 ret;
537 int id;
538
539 btf = bpf_get_btf_vmlinux();
540 if (IS_ERR(btf))
541 return PTR_ERR(btf);
542 if (!btf)
543 return -EINVAL;
544
545 ret = btf_find_by_name_kind(btf, name, kind);
546 /* ret is never zero, since btf_find_by_name_kind returns
547 * positive btf_id or negative error.
548 */
549 if (ret > 0) {
550 btf_get(btf);
551 *btf_p = btf;
552 return ret;
553 }
554
555 /* If name is not found in vmlinux's BTF then search in module's BTFs */
556 spin_lock_bh(&btf_idr_lock);
557 idr_for_each_entry(&btf_idr, btf, id) {
558 if (!btf_is_module(btf))
559 continue;
560 /* linear search could be slow hence unlock/lock
561 * the IDR to avoiding holding it for too long
562 */
563 btf_get(btf);
564 spin_unlock_bh(&btf_idr_lock);
565 ret = btf_find_by_name_kind(btf, name, kind);
566 if (ret > 0) {
567 *btf_p = btf;
568 return ret;
569 }
570 spin_lock_bh(&btf_idr_lock);
571 btf_put(btf);
572 }
573 spin_unlock_bh(&btf_idr_lock);
574 return ret;
575 }
576
On Fri, Jan 20, 2023 at 09:14:25AM +0800, kernel test robot wrote: > Hi David, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on bpf-next/master] > > url: https://github.com/intel-lab-lkp/linux/commits/David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139 > base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master > patch link: https://lore.kernel.org/r/20230119235833.2948341-2-void%40manifault.com > patch subject: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers > config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230120/202301200957.At49rpzu-lkp@intel.com/config) > compiler: ia64-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/intel-lab-lkp/linux/commit/8f6df14342b1be3516f8e21037edf771df851427 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139 > git checkout 8f6df14342b1be3516f8e21037edf771df851427 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash kernel/bpf/ > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > >> kernel/bpf/btf.c:533:5: warning: no previous prototype for 'bpf_find_btf_id' [-Wmissing-prototypes] > 533 | s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p) Silly mistake on my part. I removed static while debugging something in verifier.c and forgot to put it back. I'll put it back in v2. > | ^~~~~~~~~~~~~~~ > kernel/bpf/btf.c: In function 'btf_seq_show': > kernel/bpf/btf.c:6977:29: warning: function 'btf_seq_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format] > 6977 | seq_vprintf((struct seq_file *)show->target, fmt, args); > | ^~~~~~~~ > kernel/bpf/btf.c: In function 'btf_snprintf_show': > kernel/bpf/btf.c:7014:9: warning: function 'btf_snprintf_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format] > 7014 | len = vsnprintf(show->target, ssnprintf->len_left, fmt, args); > | ^~~ > > > vim +/bpf_find_btf_id +533 kernel/bpf/btf.c > > 532 > > 533 s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p) > 534 { > 535 struct btf *btf; > 536 s32 ret; > 537 int id; > 538 > 539 btf = bpf_get_btf_vmlinux(); > 540 if (IS_ERR(btf)) > 541 return PTR_ERR(btf); > 542 if (!btf) > 543 return -EINVAL; > 544 > 545 ret = btf_find_by_name_kind(btf, name, kind); > 546 /* ret is never zero, since btf_find_by_name_kind returns > 547 * positive btf_id or negative error. > 548 */ > 549 if (ret > 0) { > 550 btf_get(btf); > 551 *btf_p = btf; > 552 return ret; > 553 } > 554 > 555 /* If name is not found in vmlinux's BTF then search in module's BTFs */ > 556 spin_lock_bh(&btf_idr_lock); > 557 idr_for_each_entry(&btf_idr, btf, id) { > 558 if (!btf_is_module(btf)) > 559 continue; > 560 /* linear search could be slow hence unlock/lock > 561 * the IDR to avoiding holding it for too long > 562 */ > 563 btf_get(btf); > 564 spin_unlock_bh(&btf_idr_lock); > 565 ret = btf_find_by_name_kind(btf, name, kind); > 566 if (ret > 0) { > 567 *btf_p = btf; > 568 return ret; > 569 } > 570 spin_lock_bh(&btf_idr_lock); > 571 btf_put(btf); > 572 } > 573 spin_unlock_bh(&btf_idr_lock); > 574 return ret; > 575 } > 576 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests
Hi David,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20230119235833.2948341-2-void%40manifault.com
patch subject: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230120/202301201328.KZrjrJXf-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8f6df14342b1be3516f8e21037edf771df851427
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review David-Vernet/bpf-Enable-annotating-trusted-nested-pointers/20230120-080139
git checkout 8f6df14342b1be3516f8e21037edf771df851427
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/bpf/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/bpf/btf.c:533:5: warning: no previous prototype for function 'bpf_find_btf_id' [-Wmissing-prototypes]
s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p)
^
kernel/bpf/btf.c:533:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p)
^
static
1 warning generated.
vim +/bpf_find_btf_id +533 kernel/bpf/btf.c
532
> 533 s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p)
534 {
535 struct btf *btf;
536 s32 ret;
537 int id;
538
539 btf = bpf_get_btf_vmlinux();
540 if (IS_ERR(btf))
541 return PTR_ERR(btf);
542 if (!btf)
543 return -EINVAL;
544
545 ret = btf_find_by_name_kind(btf, name, kind);
546 /* ret is never zero, since btf_find_by_name_kind returns
547 * positive btf_id or negative error.
548 */
549 if (ret > 0) {
550 btf_get(btf);
551 *btf_p = btf;
552 return ret;
553 }
554
555 /* If name is not found in vmlinux's BTF then search in module's BTFs */
556 spin_lock_bh(&btf_idr_lock);
557 idr_for_each_entry(&btf_idr, btf, id) {
558 if (!btf_is_module(btf))
559 continue;
560 /* linear search could be slow hence unlock/lock
561 * the IDR to avoiding holding it for too long
562 */
563 btf_get(btf);
564 spin_unlock_bh(&btf_idr_lock);
565 ret = btf_find_by_name_kind(btf, name, kind);
566 if (ret > 0) {
567 *btf_p = btf;
568 return ret;
569 }
570 spin_lock_bh(&btf_idr_lock);
571 btf_put(btf);
572 }
573 spin_unlock_bh(&btf_idr_lock);
574 return ret;
575 }
576
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index ae7771c7d750..283e96e5b228 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2186,6 +2186,10 @@ struct bpf_core_ctx { const struct btf *btf; }; +bool btf_nested_type_is_trusted(struct bpf_verifier_log *log, + const struct bpf_reg_state *reg, + int off); + int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo, int relo_idx, void *insn); diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 4ba749fcce9d..fcd66edc22af 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -24,6 +24,7 @@ #include <linux/perf_event.h> #include <linux/bsearch.h> #include <linux/kobject.h> +#include <linux/stringify.h> #include <linux/sysfs.h> #include <net/sock.h> #include "../tools/lib/bpf/relo_core.h" @@ -529,7 +530,7 @@ s32 btf_find_by_name_kind(const struct btf *btf, const char *name, u8 kind) return -ENOENT; } -static s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p) +s32 bpf_find_btf_id(const char *name, u32 kind, struct btf **btf_p) { struct btf *btf; s32 ret; @@ -8227,3 +8228,64 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo, } return err; } + +bool btf_nested_type_is_trusted(struct bpf_verifier_log *log, + const struct bpf_reg_state *reg, + int off) +{ + struct btf *btf = reg->btf; + const struct btf_type *walk_type, *safe_type; + const char *tname; + char safe_tname[64]; + long ret, safe_id; + const struct btf_member *member, *m_walk = NULL; + u32 i; + const char *walk_name; + + walk_type = btf_type_by_id(btf, reg->btf_id); + if (!walk_type) + return false; + + tname = btf_name_by_offset(btf, walk_type->name_off); + + ret = snprintf(safe_tname, sizeof(safe_tname), "%s__safe_fields", tname); + if (ret < 0) + return false; + + safe_id = btf_find_by_name_kind(btf, safe_tname, BTF_INFO_KIND(walk_type->info)); + if (safe_id < 0) + return false; + + safe_type = btf_type_by_id(btf, safe_id); + if (!safe_type) + return false; + + for_each_member(i, walk_type, member) { + u32 moff; + + /* We're looking for the PTR_TO_BTF_ID member in the struct + * type we're walking which matches the specified offset. + * Below, we'll iterate over the fields in the safe variant of + * the struct and see if any of them has a matching type / + * name. + */ + moff = __btf_member_bit_offset(walk_type, member) / 8; + if (off == moff) { + m_walk = member; + break; + } + } + if (m_walk == NULL) + return false; + + walk_name = __btf_name_by_offset(btf, m_walk->name_off); + for_each_member(i, safe_type, member) { + const char *m_name = __btf_name_by_offset(btf, member->name_off); + + /* If we match on both type and name, the field is considered trusted. */ + if (m_walk->type == member->type && !strcmp(walk_name, m_name)) + return true; + } + + return false; +} diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ca7db2ce70b9..7f973847b58e 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4755,6 +4755,25 @@ static int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val) return 0; } +#define BTF_TYPE_SAFE_NESTED(__type) __PASTE(__type, __safe_fields) + +BTF_TYPE_SAFE_NESTED(struct task_struct) { + const cpumask_t *cpus_ptr; +}; + +static bool nested_ptr_is_trusted(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + int off) +{ + /* If its parent is not trusted, it can't regain its trusted status. */ + if (!is_trusted_reg(reg)) + return false; + + BTF_TYPE_EMIT(BTF_TYPE_SAFE_NESTED(struct task_struct)); + + return btf_nested_type_is_trusted(&env->log, reg, off); +} + static int check_ptr_to_btf_access(struct bpf_verifier_env *env, struct bpf_reg_state *regs, int regno, int off, int size, @@ -4843,10 +4862,17 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, if (type_flag(reg->type) & PTR_UNTRUSTED) flag |= PTR_UNTRUSTED; - /* By default any pointer obtained from walking a trusted pointer is - * no longer trusted except the rcu case below. + /* By default any pointer obtained from walking a trusted pointer is no + * longer trusted, unless the field being accessed has explicitly been + * marked as inheriting its parent's state of trust. + * + * An RCU-protected pointer can also be deemed trusted if we are in an + * RCU read region. This case is handled below. */ - flag &= ~PTR_TRUSTED; + if (nested_ptr_is_trusted(env, reg, off)) + flag |= PTR_TRUSTED; + else + flag &= ~PTR_TRUSTED; if (flag & MEM_RCU) { /* Mark value register as MEM_RCU only if it is protected by
In kfuncs, a "trusted" pointer is a pointer that the kfunc can assume is safe, and which the verifier will allow to be passed to a KF_TRUSTED_ARGS kfunc. Currently, a KF_TRUSTED_ARGS kfunc disallows any pointer to be passed at a nonzero offset, but sometimes this is in fact safe if the "nested" pointer's lifetime is inherited from its parent. For example, the const cpumask_t *cpus_ptr field in a struct task_struct will remain valid until the task itself is destroyed, and thus would also be safe to pass to a KF_TRUSTED_ARGS kfunc. While it would be conceptually simple to enable this by using BTF tags, gcc unfortunately does not yet support this. In the interim, this patch enables support for this by using a type-naming convention. A new BTF_TYPE_SAFE_NESTED macro is defined in verifier.c which allows a developer to specify the nested fields of a type which are considered trusted if its parent is also trusted. The verifier is also updated to account for this. A patch with selftests will be added in a follow-on change, along with documentation for this feature. Signed-off-by: David Vernet <void@manifault.com> --- include/linux/bpf.h | 4 +++ kernel/bpf/btf.c | 64 ++++++++++++++++++++++++++++++++++++++++++- kernel/bpf/verifier.c | 32 ++++++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-)