[PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers

David Vernet posted 8 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
Posted by David Vernet 2 years, 3 months ago
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(-)

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
-- 
2.39.0
Re: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
Posted by kernel test robot 2 years, 3 months ago
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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Re: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
Posted by kernel test robot 2 years, 3 months ago
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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Re: [PATCH bpf-next 1/8] bpf: Enable annotating trusted nested pointers
Posted by David Vernet 2 years, 3 months ago
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