All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/3] bpf: btf: check name validity for various types
@ 2018-11-27  5:17 Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 1/3] " Yonghong Song
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yonghong Song @ 2018-11-27  5:17 UTC (permalink / raw)
  To: ast, daniel, netdev, kafai; +Cc: kernel-team

Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
and BTF_KIND_FUNC_PROTO") checked the name validity
for BTF_KIND_FUNC/FUNC_PROTO types.

This patch added name checking for PTR, ARRAY, VOLATILE, TYPEDEF,
CONST, RESTRICT, STRUCT, UNION, ENUM and FWD types. Such a strict
name checking makes BTF more sound in the kernel and future
BTF-to-header-file converesion ([1]) less fragile.

[1]: http://vger.kernel.org/lpc-bpf2018.html#session-2

Yonghong Song (3):
  bpf: btf: check name validity for various types
  tools/bpf: fix two test_btf unit test cases
  tools/bpf: add addition type tests to test_btf

 kernel/bpf/btf.c                       |  57 ++++
 tools/testing/selftests/bpf/test_btf.c | 380 ++++++++++++++++++++++++-
 2 files changed, 428 insertions(+), 9 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 1/3] bpf: btf: check name validity for various types
  2018-11-27  5:17 [PATCH bpf-next 0/3] bpf: btf: check name validity for various types Yonghong Song
@ 2018-11-27  5:17 ` Yonghong Song
  2018-11-27 19:02   ` Alexei Starovoitov
  2018-11-27  5:17 ` [PATCH bpf-next 2/3] tools/bpf: fix two test_btf unit test cases Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 3/3] tools/bpf: add addition type tests to test_btf Yonghong Song
  2 siblings, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2018-11-27  5:17 UTC (permalink / raw)
  To: ast, daniel, netdev, kafai; +Cc: kernel-team

Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
and BTF_KIND_FUNC_PROTO") checked the name validity
for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that:
 . BTF_KIND_FUNC must have a valid identifier name
 . BTF_KIND_PROTO must have a null name
 . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO,
   if not null, must be a valid identifier.

This patch added name checking for the following types:
 . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
   BTF_KIND_CONST, BTF_KIND_RESTRICT:
     the name must be null
 . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
     is either null or a valid identifier
 . BTF_KIND_ENUM: the enum type name is either null or a valid
     identifier; the enumerator name must be a valid identifier.
 . BTF_KIND_FWD: the name must be a valid identifier
 . BTF_KIND_TYPEDEF: the name must be a valid identifier

For those places a valid name is required, the name must be
a valid C identifier. This can be relaxed later if we found
use cases for a different (non-C) frontend.

Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/btf.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index a09b2f94ab25..793acba40b4c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1195,6 +1195,22 @@ static int btf_ref_type_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* typedef type must have a valid name, and other ref types,
+	 * volatile, const, restrict, should have a null name.
+	 */
+	if (BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF) {
+		if (!t->name_off ||
+		    !btf_name_valid_identifier(env->btf, t->name_off)) {
+			btf_verifier_log_type(env, t, "Invalid name");
+			return -EINVAL;
+		}
+	} else {
+		if (t->name_off) {
+			btf_verifier_log_type(env, t, "Invalid name");
+			return -EINVAL;
+		}
+	}
+
 	btf_verifier_log_type(env, t, NULL);
 
 	return 0;
@@ -1353,6 +1369,13 @@ static s32 btf_fwd_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* fwd type must have a valid name */
+	if (!t->name_off ||
+	    !btf_name_valid_identifier(env->btf, t->name_off)) {
+		btf_verifier_log_type(env, t, "Invalid name");
+		return -EINVAL;
+	}
+
 	btf_verifier_log_type(env, t, NULL);
 
 	return 0;
@@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* array type should not have a name */
+	if (t->name_off) {
+		btf_verifier_log_type(env, t, "Invalid name");
+		return -EINVAL;
+	}
+
 	if (btf_type_vlen(t)) {
 		btf_verifier_log_type(env, t, "vlen != 0");
 		return -EINVAL;
@@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* struct type either no name or a valid one */
+	if (t->name_off &&
+	    !btf_name_valid_identifier(env->btf, t->name_off)) {
+		btf_verifier_log_type(env, t, "Invalid name");
+		return -EINVAL;
+	}
+
 	btf_verifier_log_type(env, t, NULL);
 
 	last_offset = 0;
@@ -1596,6 +1632,12 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
 			return -EINVAL;
 		}
 
+		/* struct member either no name or a valid one */
+		if (member->name_off &&
+		    !btf_name_valid_identifier(btf, member->name_off)) {
+			btf_verifier_log_member(env, t, member, "Invalid name");
+			return -EINVAL;
+		}
 		/* A member cannot be in type void */
 		if (!member->type || !BTF_TYPE_ID_VALID(member->type)) {
 			btf_verifier_log_member(env, t, member,
@@ -1783,6 +1825,13 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* enum type either no name or a valid one */
+	if (t->name_off &&
+	    !btf_name_valid_identifier(env->btf, t->name_off)) {
+		btf_verifier_log_type(env, t, "Invalid name");
+		return -EINVAL;
+	}
+
 	btf_verifier_log_type(env, t, NULL);
 
 	for (i = 0; i < nr_enums; i++) {
@@ -1792,6 +1841,14 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env,
 			return -EINVAL;
 		}
 
+		/* enum member must have a valid name */
+		if (!enums[i].name_off ||
+		    !btf_name_valid_identifier(btf, enums[i].name_off)) {
+			btf_verifier_log_type(env, t, "Invalid name");
+			return -EINVAL;
+		}
+
+
 		btf_verifier_log(env, "\t%s val=%d\n",
 				 btf_name_by_offset(btf, enums[i].name_off),
 				 enums[i].val);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 2/3] tools/bpf: fix two test_btf unit test cases
  2018-11-27  5:17 [PATCH bpf-next 0/3] bpf: btf: check name validity for various types Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 1/3] " Yonghong Song
@ 2018-11-27  5:17 ` Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 3/3] tools/bpf: add addition type tests to test_btf Yonghong Song
  2 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2018-11-27  5:17 UTC (permalink / raw)
  To: ast, daniel, netdev, kafai; +Cc: kernel-team

From: Martin KaFai Lau <kafai@fb.com>

There are two unit test cases, which should encode
TYPEDEF type, but instead encode PTR type.
The error is flagged out after enforcing name
checking in the previous patch.

Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/test_btf.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index bae7308b7ec5..99a92923e3f9 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -447,13 +447,13 @@ static struct btf_raw_test raw_tests[] = {
 		/* const void */	/* [2] */
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
 		/* const void* */	/* [3] */
-		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
+		BTF_PTR_ENC(2),
 		/* typedef const void * const_void_ptr */
-		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
-		/* struct A { */	/* [4] */
+		BTF_TYPEDEF_ENC(NAME_TBD, 3),	/* [4] */
+		/* struct A { */	/* [5] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), sizeof(void *)),
 		/* const_void_ptr m; */
-		BTF_MEMBER_ENC(NAME_TBD, 3, 0),
+		BTF_MEMBER_ENC(NAME_TBD, 4, 0),
 		/* } */
 		BTF_END_RAW,
 	},
@@ -510,11 +510,11 @@ static struct btf_raw_test raw_tests[] = {
 		/* const void */	/* [2] */
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
 		/* const void* */	/* [3] */
-		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
-		/* typedef const void * const_void_ptr */	/* [4] */
-		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
-		/* const_void_ptr[4] */	/* [5] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 4),
+		BTF_PTR_ENC(2),
+		/* typedef const void * const_void_ptr */
+		BTF_TYPEDEF_ENC(NAME_TBD, 3),	/* [4] */
+		/* const_void_ptr[4] */
+		BTF_TYPE_ARRAY_ENC(4, 1, 4),	/* [5] */
 		BTF_END_RAW,
 	},
 	.str_sec = "\0const_void_ptr",
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 3/3] tools/bpf: add addition type tests to test_btf
  2018-11-27  5:17 [PATCH bpf-next 0/3] bpf: btf: check name validity for various types Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 1/3] " Yonghong Song
  2018-11-27  5:17 ` [PATCH bpf-next 2/3] tools/bpf: fix two test_btf unit test cases Yonghong Song
@ 2018-11-27  5:17 ` Yonghong Song
  2 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2018-11-27  5:17 UTC (permalink / raw)
  To: ast, daniel, netdev, kafai; +Cc: kernel-team

The following additional unit testcases are added to test_btf:
  BTF raw test[65] (typedef (invalid name, name_off = 0)): OK
  BTF raw test[66] (typedef (invalid name, invalid identifier)): OK
  BTF raw test[67] (ptr type (invalid name, name_off <> 0)): OK
  BTF raw test[68] (volatile type (invalid name, name_off <> 0)): OK
  BTF raw test[69] (const type (invalid name, name_off <> 0)): OK
  BTF raw test[70] (restrict type (invalid name, name_off <> 0)): OK
  BTF raw test[71] (fwd type (invalid name, name_off = 0)): OK
  BTF raw test[72] (fwd type (invalid name, invalid identifier)): OK
  BTF raw test[73] (array type (invalid name, name_off <> 0)): OK
  BTF raw test[74] (struct type (name_off = 0)): OK
  BTF raw test[75] (struct type (invalid name, invalid identifier)): OK
  BTF raw test[76] (struct member (name_off = 0)): OK
  BTF raw test[77] (struct member (invalid name, invalid identifier)): OK
  BTF raw test[78] (enum type (name_off = 0)): OK
  BTF raw test[79] (enum type (invalid name, invalid identifier)): OK
  BTF raw test[80] (enum member (invalid name, name_off = 0)): OK
  BTF raw test[81] (enum member (invalid name, invalid identifier)): OK

Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/test_btf.c | 362 +++++++++++++++++++++++++
 1 file changed, 362 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index 99a92923e3f9..a7267815311f 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1849,6 +1849,368 @@ static struct btf_raw_test raw_tests[] = {
 	.err_str = "Invalid type_id",
 },
 
+{
+	.descr = "typedef (invalid name, name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPEDEF_ENC(0, 1),				/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__int",
+	.str_sec_size = sizeof("\0__int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "typedef_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "typedef (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPEDEF_ENC(NAME_TBD, 1),			/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__!int",
+	.str_sec_size = sizeof("\0__!int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "typedef_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "ptr type (invalid name, name_off <> 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 1),	/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__int",
+	.str_sec_size = sizeof("\0__int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "ptr_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "volatile type (invalid name, name_off <> 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_VOLATILE, 0, 0), 1),	/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__int",
+	.str_sec_size = sizeof("\0__int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "volatile_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "const type (invalid name, name_off <> 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 1),	/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__int",
+	.str_sec_size = sizeof("\0__int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "const_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "restrict type (invalid name, name_off <> 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 1),	/* [2] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_RESTRICT, 0, 0), 2),	/* [3] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__int",
+	.str_sec_size = sizeof("\0__int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "restrict_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "fwd type (invalid name, name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FWD, 0, 0), 0),	/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__skb",
+	.str_sec_size = sizeof("\0__skb"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "fwd_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "fwd type (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_FWD, 0, 0), 0),	/* [2] */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__!skb",
+	.str_sec_size = sizeof("\0__!skb"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "fwd_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "array type (invalid name, name_off <> 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_ARRAY, 0, 0), 0),	/* [2] */
+		BTF_ARRAY_ENC(1, 1, 4),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0__skb",
+	.str_sec_size = sizeof("\0__skb"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "struct type (name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0,
+			     BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),	/* [2] */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A",
+	.str_sec_size = sizeof("\0A"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "struct_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "struct type (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),	/* [2] */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A!\0B",
+	.str_sec_size = sizeof("\0A!\0B"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "struct_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "struct member (name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0,
+			     BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),	/* [2] */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A",
+	.str_sec_size = sizeof("\0A"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "struct_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "struct member (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),	/* [2] */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A\0B*",
+	.str_sec_size = sizeof("\0A\0B*"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "struct_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "enum type (name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0,
+			     BTF_INFO_ENC(BTF_KIND_ENUM, 0, 1),
+			     sizeof(int)),				/* [2] */
+		BTF_ENUM_ENC(NAME_TBD, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A\0B",
+	.str_sec_size = sizeof("\0A\0B"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "enum_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "enum type (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(NAME_TBD,
+			     BTF_INFO_ENC(BTF_KIND_ENUM, 0, 1),
+			     sizeof(int)),				/* [2] */
+		BTF_ENUM_ENC(NAME_TBD, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A!\0B",
+	.str_sec_size = sizeof("\0A!\0B"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "enum_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "enum member (invalid name, name_off = 0)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0,
+			     BTF_INFO_ENC(BTF_KIND_ENUM, 0, 1),
+			     sizeof(int)),				/* [2] */
+		BTF_ENUM_ENC(0, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "enum_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
+{
+	.descr = "enum member (invalid name, invalid identifier)",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),		/* [1] */
+		BTF_TYPE_ENC(0,
+			     BTF_INFO_ENC(BTF_KIND_ENUM, 0, 1),
+			     sizeof(int)),				/* [2] */
+		BTF_ENUM_ENC(NAME_TBD, 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0A!",
+	.str_sec_size = sizeof("\0A!"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "enum_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid name",
+},
+
 }; /* struct btf_raw_test raw_tests[] */
 
 static const char *get_next_str(const char *start, const char *end)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 1/3] bpf: btf: check name validity for various types
  2018-11-27  5:17 ` [PATCH bpf-next 1/3] " Yonghong Song
@ 2018-11-27 19:02   ` Alexei Starovoitov
  2018-11-27 19:57     ` Yonghong Song
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2018-11-27 19:02 UTC (permalink / raw)
  To: Yonghong Song; +Cc: ast, daniel, netdev, kafai, kernel-team

On Mon, Nov 26, 2018 at 09:17:13PM -0800, Yonghong Song wrote:
> Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
> and BTF_KIND_FUNC_PROTO") checked the name validity
> for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that:
>  . BTF_KIND_FUNC must have a valid identifier name
>  . BTF_KIND_PROTO must have a null name
>  . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO,
>    if not null, must be a valid identifier.
> 
> This patch added name checking for the following types:
>  . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
>    BTF_KIND_CONST, BTF_KIND_RESTRICT:
>      the name must be null
>  . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
>      is either null or a valid identifier
>  . BTF_KIND_ENUM: the enum type name is either null or a valid
>      identifier; the enumerator name must be a valid identifier.
>  . BTF_KIND_FWD: the name must be a valid identifier
>  . BTF_KIND_TYPEDEF: the name must be a valid identifier
> 
> For those places a valid name is required, the name must be
> a valid C identifier. This can be relaxed later if we found
> use cases for a different (non-C) frontend.
> 
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
...
>  	return 0;
> @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>  
> +	/* array type should not have a name */
> +	if (t->name_off) {
> +		btf_verifier_log_type(env, t, "Invalid name");
> +		return -EINVAL;
> +	}
> +
>  	if (btf_type_vlen(t)) {
>  		btf_verifier_log_type(env, t, "vlen != 0");
>  		return -EINVAL;
> @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>  
> +	/* struct type either no name or a valid one */
> +	if (t->name_off &&
> +	    !btf_name_valid_identifier(env->btf, t->name_off)) {

Looks like some of these changes need to go into bpf tree.
please split it up and let's try to minimize the conflicts between bpf and bpf-next
Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 1/3] bpf: btf: check name validity for various types
  2018-11-27 19:02   ` Alexei Starovoitov
@ 2018-11-27 19:57     ` Yonghong Song
  0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2018-11-27 19:57 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, daniel, netdev, Martin Lau, Kernel Team



On 11/27/18 11:02 AM, Alexei Starovoitov wrote:
> On Mon, Nov 26, 2018 at 09:17:13PM -0800, Yonghong Song wrote:
>> Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
>> and BTF_KIND_FUNC_PROTO") checked the name validity
>> for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that:
>>   . BTF_KIND_FUNC must have a valid identifier name
>>   . BTF_KIND_PROTO must have a null name
>>   . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO,
>>     if not null, must be a valid identifier.
>>
>> This patch added name checking for the following types:
>>   . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
>>     BTF_KIND_CONST, BTF_KIND_RESTRICT:
>>       the name must be null
>>   . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
>>       is either null or a valid identifier
>>   . BTF_KIND_ENUM: the enum type name is either null or a valid
>>       identifier; the enumerator name must be a valid identifier.
>>   . BTF_KIND_FWD: the name must be a valid identifier
>>   . BTF_KIND_TYPEDEF: the name must be a valid identifier
>>
>> For those places a valid name is required, the name must be
>> a valid C identifier. This can be relaxed later if we found
>> use cases for a different (non-C) frontend.
>>
>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> ...
>>   	return 0;
>> @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* array type should not have a name */
>> +	if (t->name_off) {
>> +		btf_verifier_log_type(env, t, "Invalid name");
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (btf_type_vlen(t)) {
>>   		btf_verifier_log_type(env, t, "vlen != 0");
>>   		return -EINVAL;
>> @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* struct type either no name or a valid one */
>> +	if (t->name_off &&
>> +	    !btf_name_valid_identifier(env->btf, t->name_off)) {
> 
> Looks like some of these changes need to go into bpf tree.
> please split it up and let's try to minimize the conflicts between bpf and bpf-next

Make sense. Will restructure and resubmit for bpf in the next version.

> Thanks!
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-28  6:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27  5:17 [PATCH bpf-next 0/3] bpf: btf: check name validity for various types Yonghong Song
2018-11-27  5:17 ` [PATCH bpf-next 1/3] " Yonghong Song
2018-11-27 19:02   ` Alexei Starovoitov
2018-11-27 19:57     ` Yonghong Song
2018-11-27  5:17 ` [PATCH bpf-next 2/3] tools/bpf: fix two test_btf unit test cases Yonghong Song
2018-11-27  5:17 ` [PATCH bpf-next 3/3] tools/bpf: add addition type tests to test_btf Yonghong Song

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.