[PATCH bpf-next v4 0/8] Fixes for bad PTR_TO_BTF_ID offset

[PATCH bpf-next v4 0/8] Fixes for bad PTR_TO_BTF_ID offset

[Kumar Kartikeya Dwivedi]

This set fixes a bug related to bad var_off being permitted for kfunc call in
case of PTR_TO_BTF_ID, consolidates offset checks for all register types allowed
as helper or kfunc arguments into a common shared helper, and introduces a
couple of other checks to harden the kfunc release logic and prevent future
bugs. Some selftests are also included that fail in absence of these fixes,
serving as demonstration of the issues being fixed.

Changelog:
----------
v3 -> v4:
v3: https://lore.kernel.org/bpf/20220304000508.2904128-1-memxor@gmail.com

 * Update commit message for __diag patch to say clang instead of LLVM (Nathan)
 * Address nits for check_func_arg_reg_off (Martin)
 * Add comment for fixed_off_ok case, remove is_kfunc check (Martin)

v2 -> v3:
v2: https://lore.kernel.org/bpf/20220303045029.2645297-1-memxor@gmail.com

 * Add my SoB to __diag for clang patch (Nathan)

v1 -> v2:
v1: https://lore.kernel.org/bpf/20220301065745.1634848-1-memxor@gmail.com

 * Put reg->off check for release kfunc inside check_func_arg_reg_off,
   make the check a bit more readable
 * Squash verifier selftests errstr update into patch 3 for bisect (Alexei)
 * Include fix from Nathan for clang warning about missing prototypes
 * Add unified __diag_ingore_all that works for both GCC/LLVM (Alexei)

Older discussion:
Link: https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com

Kumar Kartikeya Dwivedi (7):
  bpf: Add check_func_arg_reg_off function
  bpf: Fix PTR_TO_BTF_ID var_off check
  bpf: Disallow negative offset in check_ptr_off_reg
  bpf: Harden register offset checks for release helpers and kfuncs
  compiler_types.h: Add unified __diag_ignore_all for GCC/LLVM
  bpf: Replace __diag_ignore with unified __diag_ignore_all
  selftests/bpf: Add tests for kfunc register offset checks

Nathan Chancellor (1):
  compiler-clang.h: Add __diag infrastructure for clang

 include/linux/bpf_verifier.h                  |  4 +
 include/linux/compiler-clang.h                | 25 +++++
 include/linux/compiler-gcc.h                  |  3 +
 include/linux/compiler_types.h                |  4 +
 kernel/bpf/btf.c                              | 40 ++++----
 kernel/bpf/verifier.c                         | 94 +++++++++++++------
 net/bpf/test_run.c                            | 15 ++-
 net/netfilter/nf_conntrack_bpf.c              |  5 +-
 .../selftests/bpf/verifier/bounds_deduction.c |  2 +-
 tools/testing/selftests/bpf/verifier/calls.c  | 83 ++++++++++++++++
 tools/testing/selftests/bpf/verifier/ctx.c    |  8 +-
 11 files changed, 230 insertions(+), 53 deletions(-)

-- 
2.35.1

[PATCH bpf-next v4 1/8] bpf: Add check_func_arg_reg_off function

[Kumar Kartikeya Dwivedi]

Lift the list of register types allowed for having fixed and variable
offsets when passed as helper function arguments into a common helper,
so that they can be reused for kfunc checks in later commits. Keeping a
common helper aids maintainability and allows us to follow the same
consistent rules across helpers and kfuncs. Also, convert check_func_arg
to use this function.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpf_verifier.h |  3 ++
 kernel/bpf/verifier.c        | 69 +++++++++++++++++++++---------------
 2 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 7a7be8c057f2..38b24ee8d8c2 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -521,6 +521,9 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);
 
 int check_ptr_off_reg(struct bpf_verifier_env *env,
 		      const struct bpf_reg_state *reg, int regno);
+int check_func_arg_reg_off(struct bpf_verifier_env *env,
+			   const struct bpf_reg_state *reg, int regno,
+			   enum bpf_arg_type arg_type);
 int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 			     u32 regno);
 int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a57db4b2803c..e37eb6020253 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5359,6 +5359,44 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 	return 0;
 }
 
+int check_func_arg_reg_off(struct bpf_verifier_env *env,
+			   const struct bpf_reg_state *reg, int regno,
+			   enum bpf_arg_type arg_type)
+{
+	enum bpf_reg_type type = reg->type;
+	bool fixed_off_ok = false;
+
+	switch ((u32)type) {
+	case SCALAR_VALUE:
+	/* Pointer types where reg offset is explicitly allowed: */
+	case PTR_TO_PACKET:
+	case PTR_TO_PACKET_META:
+	case PTR_TO_MAP_KEY:
+	case PTR_TO_MAP_VALUE:
+	case PTR_TO_MEM:
+	case PTR_TO_MEM | MEM_RDONLY:
+	case PTR_TO_MEM | MEM_ALLOC:
+	case PTR_TO_BUF:
+	case PTR_TO_BUF | MEM_RDONLY:
+	case PTR_TO_STACK:
+		/* Some of the argument types nevertheless require a
+		 * zero register offset.
+		 */
+		if (arg_type != ARG_PTR_TO_ALLOC_MEM)
+			return 0;
+		break;
+	/* All the rest must be rejected, except PTR_TO_BTF_ID which allows
+	 * fixed offset.
+	 */
+	case PTR_TO_BTF_ID:
+		fixed_off_ok = true;
+		break;
+	default:
+		break;
+	}
+	return __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
+}
+
 static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			  struct bpf_call_arg_meta *meta,
 			  const struct bpf_func_proto *fn)
@@ -5408,34 +5446,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 	if (err)
 		return err;
 
-	switch ((u32)type) {
-	case SCALAR_VALUE:
-	/* Pointer types where reg offset is explicitly allowed: */
-	case PTR_TO_PACKET:
-	case PTR_TO_PACKET_META:
-	case PTR_TO_MAP_KEY:
-	case PTR_TO_MAP_VALUE:
-	case PTR_TO_MEM:
-	case PTR_TO_MEM | MEM_RDONLY:
-	case PTR_TO_MEM | MEM_ALLOC:
-	case PTR_TO_BUF:
-	case PTR_TO_BUF | MEM_RDONLY:
-	case PTR_TO_STACK:
-		/* Some of the argument types nevertheless require a
-		 * zero register offset.
-		 */
-		if (arg_type == ARG_PTR_TO_ALLOC_MEM)
-			goto force_off_check;
-		break;
-	/* All the rest must be rejected: */
-	default:
-force_off_check:
-		err = __check_ptr_off_reg(env, reg, regno,
-					  type == PTR_TO_BTF_ID);
-		if (err < 0)
-			return err;
-		break;
-	}
+	err = check_func_arg_reg_off(env, reg, regno, arg_type);
+	if (err)
+		return err;
 
 skip_type_check:
 	if (reg->ref_obj_id) {
-- 
2.35.1

[PATCH bpf-next v4 2/8] bpf: Fix PTR_TO_BTF_ID var_off check

[Kumar Kartikeya Dwivedi]

When kfunc support was added, check_ctx_reg was called for PTR_TO_CTX
register, but no offset checks were made for PTR_TO_BTF_ID. Only
reg->off was taken into account by btf_struct_ids_match, which protected
against type mismatch due to non-zero reg->off, but when reg->off was
zero, a user could set the variable offset of the register and allow it
to be passed to kfunc, leading to bad pointer being passed into the
kernel.

Fix this by reusing the extracted helper check_func_arg_reg_off from
previous commit, and make one call before checking all supported
register types. Since the list is maintained, any future changes will be
taken into account by updating check_func_arg_reg_off. This function
prevents non-zero var_off to be set for PTR_TO_BTF_ID, but still allows
a fixed non-zero reg->off, which is needed for type matching to work
correctly when using pointer arithmetic.

ARG_DONTCARE is passed as arg_type, since kfunc doesn't support
accepting a ARG_PTR_TO_ALLOC_MEM without relying on size of parameter
type from BTF (in case of pointer), or using a mem, len pair. The
forcing of offset check for ARG_PTR_TO_ALLOC_MEM is done because ringbuf
helpers obtain the size from the header located at the beginning of the
memory region, hence any changes to the original pointer shouldn't be
allowed. In case of kfunc, size is always known, either at verification
time, or using the length parameter, hence this forcing is not required.

Since this check will happen once already for PTR_TO_CTX, remove the
check_ptr_off_reg call inside its block.

Cc: Martin KaFai Lau <kafai@fb.com>
Fixes: e6ac2450d6de ("bpf: Support bpf program calling kernel function")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/btf.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index b472cf0c8fdb..7f6a0ae5028b 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5726,7 +5726,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 	const char *func_name, *ref_tname;
 	const struct btf_type *t, *ref_t;
 	const struct btf_param *args;
-	int ref_regno = 0;
+	int ref_regno = 0, ret;
 	bool rel = false;
 
 	t = btf_type_by_id(btf, func_id);
@@ -5776,6 +5776,11 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 
 		ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
 		ref_tname = btf_name_by_offset(btf, ref_t->name_off);
+
+		ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
+		if (ret < 0)
+			return ret;
+
 		if (btf_get_prog_ctx_type(log, btf, t,
 					  env->prog->type, i)) {
 			/* If function expects ctx type in BTF check that caller
@@ -5787,8 +5792,6 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 					i, btf_type_str(t));
 				return -EINVAL;
 			}
-			if (check_ptr_off_reg(env, reg, regno))
-				return -EINVAL;
 		} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID ||
 			   (reg2btf_ids[base_type(reg->type)] && !type_flag(reg->type)))) {
 			const struct btf_type *reg_ref_t;
-- 
2.35.1

[PATCH bpf-next v4 3/8] bpf: Disallow negative offset in check_ptr_off_reg

[Kumar Kartikeya Dwivedi]

check_ptr_off_reg only allows fixed offset to be set for PTR_TO_BTF_ID,
where reg->off < 0 doesn't make sense. This would shift the pointer
backwards, and fails later in btf_struct_ids_match or btf_struct_walk
due to out of bounds access (since offset is interpreted as unsigned).

Improve the verifier by rejecting this case by using a better error
message for BPF helpers and kfunc, by putting a check inside the
check_func_arg_reg_off function.

Also, update existing verifier selftests to work with new error string.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c                                   | 6 ++++++
 tools/testing/selftests/bpf/verifier/bounds_deduction.c | 2 +-
 tools/testing/selftests/bpf/verifier/ctx.c              | 8 ++++----
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e37eb6020253..455b4ab69e47 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3990,6 +3990,12 @@ static int __check_ptr_off_reg(struct bpf_verifier_env *env,
 	 * is only allowed in its original, unmodified form.
 	 */
 
+	if (reg->off < 0) {
+		verbose(env, "negative offset %s ptr R%d off=%d disallowed\n",
+			reg_type_str(env, reg->type), regno, reg->off);
+		return -EACCES;
+	}
+
 	if (!fixed_off_ok && reg->off) {
 		verbose(env, "dereference of modified %s ptr R%d off=%d disallowed\n",
 			reg_type_str(env, reg->type), regno, reg->off);
diff --git a/tools/testing/selftests/bpf/verifier/bounds_deduction.c b/tools/testing/selftests/bpf/verifier/bounds_deduction.c
index 91869aea6d64..3931c481e30c 100644
--- a/tools/testing/selftests/bpf/verifier/bounds_deduction.c
+++ b/tools/testing/selftests/bpf/verifier/bounds_deduction.c
@@ -105,7 +105,7 @@
 		BPF_EXIT_INSN(),
 	},
 	.errstr_unpriv = "R1 has pointer with unsupported alu operation",
-	.errstr = "dereference of modified ctx ptr",
+	.errstr = "negative offset ctx ptr R1 off=-1 disallowed",
 	.result = REJECT,
 	.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
 },
diff --git a/tools/testing/selftests/bpf/verifier/ctx.c b/tools/testing/selftests/bpf/verifier/ctx.c
index 60f6fbe03f19..c8eaf0536c24 100644
--- a/tools/testing/selftests/bpf/verifier/ctx.c
+++ b/tools/testing/selftests/bpf/verifier/ctx.c
@@ -58,7 +58,7 @@
 	},
 	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 	.result = REJECT,
-	.errstr = "dereference of modified ctx ptr",
+	.errstr = "negative offset ctx ptr R1 off=-612 disallowed",
 },
 {
 	"pass modified ctx pointer to helper, 2",
@@ -71,8 +71,8 @@
 	},
 	.result_unpriv = REJECT,
 	.result = REJECT,
-	.errstr_unpriv = "dereference of modified ctx ptr",
-	.errstr = "dereference of modified ctx ptr",
+	.errstr_unpriv = "negative offset ctx ptr R1 off=-612 disallowed",
+	.errstr = "negative offset ctx ptr R1 off=-612 disallowed",
 },
 {
 	"pass modified ctx pointer to helper, 3",
@@ -141,7 +141,7 @@
 	.prog_type = BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
 	.expected_attach_type = BPF_CGROUP_UDP6_SENDMSG,
 	.result = REJECT,
-	.errstr = "dereference of modified ctx ptr",
+	.errstr = "negative offset ctx ptr R1 off=-612 disallowed",
 },
 {
 	"pass ctx or null check, 5: null (connect)",
-- 
2.35.1

[PATCH bpf-next v4 4/8] bpf: Harden register offset checks for release helpers and kfuncs

[Kumar Kartikeya Dwivedi]

Let's ensure that the PTR_TO_BTF_ID reg being passed in to release BPF
helpers and kfuncs always has its offset set to 0. While not a real
problem now, there's a very real possibility this will become a problem
when more and more kfuncs are exposed, and more BPF helpers are added
which can release PTR_TO_BTF_ID.

Previous commits already protected against non-zero var_off. One of the
case we are concerned about now is when we have a type that can be
returned by e.g. an acquire kfunc:

struct foo {
	int a;
	int b;
	struct bar b;
};

... and struct bar is also a type that can be returned by another
acquire kfunc.

Then, doing the following sequence:

	struct foo *f = bpf_get_foo(); // acquire kfunc
	if (!f)
		return 0;
	bpf_put_bar(&f->b); // release kfunc

... would work with the current code, since the btf_struct_ids_match
takes reg->off into account for matching pointer type with release kfunc
argument type, but would obviously be incorrect, and most likely lead to
a kernel crash. A test has been included later to prevent regressions in
this area.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpf_verifier.h |  3 ++-
 kernel/bpf/btf.c             | 33 +++++++++++++++++++--------------
 kernel/bpf/verifier.c        | 25 ++++++++++++++++++++++---
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 38b24ee8d8c2..c1fc4af47f69 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -523,7 +523,8 @@ int check_ptr_off_reg(struct bpf_verifier_env *env,
 		      const struct bpf_reg_state *reg, int regno);
 int check_func_arg_reg_off(struct bpf_verifier_env *env,
 			   const struct bpf_reg_state *reg, int regno,
-			   enum bpf_arg_type arg_type);
+			   enum bpf_arg_type arg_type,
+			   bool is_release_func);
 int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 			     u32 regno);
 int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 7f6a0ae5028b..162807e3b4a5 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5753,6 +5753,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	/* Only kfunc can be release func */
+	if (is_kfunc)
+		rel = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
+						BTF_KFUNC_TYPE_RELEASE, func_id);
 	/* check that BTF function arguments match actual types that the
 	 * verifier sees.
 	 */
@@ -5777,7 +5781,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
 		ref_tname = btf_name_by_offset(btf, ref_t->name_off);
 
-		ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
+		ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE, rel);
 		if (ret < 0)
 			return ret;
 
@@ -5809,7 +5813,11 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 			if (reg->type == PTR_TO_BTF_ID) {
 				reg_btf = reg->btf;
 				reg_ref_id = reg->btf_id;
-				/* Ensure only one argument is referenced PTR_TO_BTF_ID */
+				/* Ensure only one argument is referenced
+				 * PTR_TO_BTF_ID, check_func_arg_reg_off relies
+				 * on only one referenced register being allowed
+				 * for kfuncs.
+				 */
 				if (reg->ref_obj_id) {
 					if (ref_obj_id) {
 						bpf_log(log, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n",
@@ -5891,18 +5899,15 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 
 	/* Either both are set, or neither */
 	WARN_ON_ONCE((ref_obj_id && !ref_regno) || (!ref_obj_id && ref_regno));
-	if (is_kfunc) {
-		rel = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
-						BTF_KFUNC_TYPE_RELEASE, func_id);
-		/* We already made sure ref_obj_id is set only for one argument */
-		if (rel && !ref_obj_id) {
-			bpf_log(log, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n",
-				func_name);
-			return -EINVAL;
-		}
-		/* Allow (!rel && ref_obj_id), so that passing such referenced PTR_TO_BTF_ID to
-		 * other kfuncs works
-		 */
+	/* We already made sure ref_obj_id is set only for one argument. We do
+	 * allow (!rel && ref_obj_id), so that passing such referenced
+	 * PTR_TO_BTF_ID to other kfuncs works. Note that rel is only true when
+	 * is_kfunc is true.
+	 */
+	if (rel && !ref_obj_id) {
+		bpf_log(log, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n",
+			func_name);
+		return -EINVAL;
 	}
 	/* returns argument register number > 0 in case of reference release kfunc */
 	return rel ? ref_regno : 0;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 455b4ab69e47..fe9a513e2314 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5367,10 +5367,11 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 
 int check_func_arg_reg_off(struct bpf_verifier_env *env,
 			   const struct bpf_reg_state *reg, int regno,
-			   enum bpf_arg_type arg_type)
+			   enum bpf_arg_type arg_type,
+			   bool is_release_func)
 {
+	bool fixed_off_ok = false, release_reg;
 	enum bpf_reg_type type = reg->type;
-	bool fixed_off_ok = false;
 
 	switch ((u32)type) {
 	case SCALAR_VALUE:
@@ -5395,6 +5396,21 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
 	 * fixed offset.
 	 */
 	case PTR_TO_BTF_ID:
+		/* When referenced PTR_TO_BTF_ID is passed to release function,
+		 * it's fixed offset must be 0. We rely on the property that
+		 * only one referenced register can be passed to BPF helpers and
+		 * kfuncs. In the other cases, fixed offset can be non-zero.
+		 */
+		release_reg = is_release_func && reg->ref_obj_id;
+		if (release_reg && reg->off) {
+			verbose(env, "R%d must have zero offset when passed to release func\n",
+				regno);
+			return -EINVAL;
+		}
+		/* For release_reg == true, fixed_off_ok must be false, but we
+		 * already checked and rejected reg->off != 0 above, so set to
+		 * true to allow fixed offset for all other cases.
+		 */
 		fixed_off_ok = true;
 		break;
 	default:
@@ -5452,11 +5468,14 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 	if (err)
 		return err;
 
-	err = check_func_arg_reg_off(env, reg, regno, arg_type);
+	err = check_func_arg_reg_off(env, reg, regno, arg_type, is_release_function(meta->func_id));
 	if (err)
 		return err;
 
 skip_type_check:
+	/* check_func_arg_reg_off relies on only one referenced register being
+	 * allowed for BPF helpers.
+	 */
 	if (reg->ref_obj_id) {
 		if (meta->ref_obj_id) {
 			verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n",
-- 
2.35.1

[PATCH bpf-next v4 5/8] compiler-clang.h: Add __diag infrastructure for clang

[Kumar Kartikeya Dwivedi]

From: Nathan Chancellor <nathan@kernel.org>

Add __diag macros similar to those in compiler-gcc.h, so that warnings
that need to be adjusted for specific cases but not globally can be
ignored when building with clang.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: llvm@lists.linux.dev
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
[ Kartikeya: wrote commit message ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/compiler-clang.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 3c4de9b6c6e3..f1aa41d520bd 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -68,3 +68,25 @@
 
 #define __nocfi		__attribute__((__no_sanitize__("cfi")))
 #define __cficanonical	__attribute__((__cfi_canonical_jump_table__))
+
+/*
+ * Turn individual warnings and errors on and off locally, depending
+ * on version.
+ */
+#define __diag_clang(version, severity, s) \
+	__diag_clang_ ## version(__diag_clang_ ## severity s)
+
+/* Severity used in pragma directives */
+#define __diag_clang_ignore	ignored
+#define __diag_clang_warn	warning
+#define __diag_clang_error	error
+
+#define __diag_str1(s)		#s
+#define __diag_str(s)		__diag_str1(s)
+#define __diag(s)		_Pragma(__diag_str(clang diagnostic s))
+
+#if CONFIG_CLANG_VERSION >= 110000
+#define __diag_clang_11(s)	__diag(s)
+#else
+#define __diag_clang_11(s)
+#endif
-- 
2.35.1

[PATCH bpf-next v4 6/8] compiler_types.h: Add unified __diag_ignore_all for GCC/LLVM

[Kumar Kartikeya Dwivedi]

Add a __diag_ignore_all macro, to ignore warnings for both GCC and LLVM,
without having to specify the compiler type and version. By default, GCC
8 and clang 11 are used. This will be used by bpf subsystem to ignore
-Wmissing-prototypes warning for functions that are meant to be global
functions so that they are in vmlinux BTF, but don't have a prototype.

Cc: linux-kernel@vger.kernel.org
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/compiler-clang.h | 3 +++
 include/linux/compiler-gcc.h   | 3 +++
 include/linux/compiler_types.h | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index f1aa41d520bd..babb1347148c 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -90,3 +90,6 @@
 #else
 #define __diag_clang_11(s)
 #endif
+
+#define __diag_ignore_all(option, comment) \
+	__diag_clang(11, ignore, option)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index ccbbd31b3aae..d364c98a4a80 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -151,6 +151,9 @@
 #define __diag_GCC_8(s)
 #endif
 
+#define __diag_ignore_all(option, comment) \
+	__diag_GCC(8, ignore, option)
+
 /*
  * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
  * attribute) do not work, and must be disabled.
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3f31ff400432..8e5d2f50f951 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -371,4 +371,8 @@ struct ftrace_likely_data {
 #define __diag_error(compiler, version, option, comment) \
 	__diag_ ## compiler(version, error, option)
 
+#ifndef __diag_ignore_all
+#define __diag_ignore_all(option, comment)
+#endif
+
 #endif /* __LINUX_COMPILER_TYPES_H */
-- 
2.35.1

[PATCH bpf-next v4 7/8] bpf: Replace __diag_ignore with unified __diag_ignore_all

[Kumar Kartikeya Dwivedi]

Currently, -Wmissing-prototypes warning is ignored for GCC, but not
clang. This leads to clang build warning in W=1 mode. Since the flag
used by both compilers is same, we can use the unified __diag_ignore_all
macro that works for all supported versions and compilers which have
__diag macro support (currently GCC >= 8.0, and Clang >= 11.0).

Also add nf_conntrack_bpf.h include to prevent missing prototype warning
for register_nf_conntrack_bpf.

Cc: netfilter-devel@vger.kernel.org
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 net/bpf/test_run.c               | 4 ++--
 net/netfilter/nf_conntrack_bpf.c | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index eb129e48f90b..fcc83017cd03 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -201,8 +201,8 @@ static int bpf_test_finish(const union bpf_attr *kattr,
  * future.
  */
 __diag_push();
-__diag_ignore(GCC, 8, "-Wmissing-prototypes",
-	      "Global functions as their definitions will be in vmlinux BTF");
+__diag_ignore_all("-Wmissing-prototypes",
+		  "Global functions as their definitions will be in vmlinux BTF");
 int noinline bpf_fentry_test1(int a)
 {
 	return a + 1;
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index 8ad3f52579f3..fe98673dd5ac 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -12,6 +12,7 @@
 #include <linux/btf_ids.h>
 #include <linux/net_namespace.h>
 #include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_bpf.h>
 #include <net/netfilter/nf_conntrack_core.h>
 
 /* bpf_ct_opts - Options for CT lookup helpers
@@ -102,8 +103,8 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 }
 
 __diag_push();
-__diag_ignore(GCC, 8, "-Wmissing-prototypes",
-	      "Global functions as their definitions will be in nf_conntrack BTF");
+__diag_ignore_all("-Wmissing-prototypes",
+		  "Global functions as their definitions will be in nf_conntrack BTF");
 
 /* bpf_xdp_ct_lookup - Lookup CT entry for the given tuple, and acquire a
  *		       reference to it
-- 
2.35.1

[PATCH bpf-next v4 8/8] selftests/bpf: Add tests for kfunc register offset checks

[Kumar Kartikeya Dwivedi]

Include a few verifier selftests that test against the problems being
fixed by previous commits, i.e. release kfunc always require
PTR_TO_BTF_ID fixed and var_off to be 0, and negative offset is not
permitted and returns a helpful error message.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 net/bpf/test_run.c                           | 11 +++
 tools/testing/selftests/bpf/verifier/calls.c | 83 ++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index fcc83017cd03..ba410b069824 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -270,9 +270,14 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
 	return sk;
 }
 
+struct prog_test_member {
+	u64 c;
+};
+
 struct prog_test_ref_kfunc {
 	int a;
 	int b;
+	struct prog_test_member memb;
 	struct prog_test_ref_kfunc *next;
 };
 
@@ -295,6 +300,10 @@ noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
 {
 }
 
+noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p)
+{
+}
+
 struct prog_test_pass1 {
 	int x0;
 	struct {
@@ -379,6 +388,7 @@ BTF_ID(func, bpf_kfunc_call_test2)
 BTF_ID(func, bpf_kfunc_call_test3)
 BTF_ID(func, bpf_kfunc_call_test_acquire)
 BTF_ID(func, bpf_kfunc_call_test_release)
+BTF_ID(func, bpf_kfunc_call_memb_release)
 BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
 BTF_ID(func, bpf_kfunc_call_test_pass1)
 BTF_ID(func, bpf_kfunc_call_test_pass2)
@@ -396,6 +406,7 @@ BTF_SET_END(test_sk_acquire_kfunc_ids)
 
 BTF_SET_START(test_sk_release_kfunc_ids)
 BTF_ID(func, bpf_kfunc_call_test_release)
+BTF_ID(func, bpf_kfunc_call_memb_release)
 BTF_SET_END(test_sk_release_kfunc_ids)
 
 BTF_SET_START(test_sk_ret_null_kfunc_ids)
diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index f890333259ad..2e03decb11b6 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -115,6 +115,89 @@
 		{ "bpf_kfunc_call_test_release", 5 },
 	},
 },
+{
+	"calls: invalid kfunc call: reg->off must be zero when passed to release kfunc",
+	.insns = {
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+	BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "R1 must have zero offset when passed to release func",
+	.fixup_kfunc_btf_id = {
+		{ "bpf_kfunc_call_test_acquire", 3 },
+		{ "bpf_kfunc_call_memb_release", 8 },
+	},
+},
+{
+	"calls: invalid kfunc call: PTR_TO_BTF_ID with negative offset",
+	.insns = {
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+	BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 16),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -4),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.fixup_kfunc_btf_id = {
+		{ "bpf_kfunc_call_test_acquire", 3 },
+		{ "bpf_kfunc_call_test_release", 9 },
+	},
+	.result_unpriv = REJECT,
+	.result = REJECT,
+	.errstr = "negative offset ptr_ ptr R1 off=-4 disallowed",
+},
+{
+	"calls: invalid kfunc call: PTR_TO_BTF_ID with variable offset",
+	.insns = {
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+	BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_0, 4),
+	BPF_JMP_IMM(BPF_JLE, BPF_REG_2, 4, 3),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_JMP_IMM(BPF_JGE, BPF_REG_2, 0, 3),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_2),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.fixup_kfunc_btf_id = {
+		{ "bpf_kfunc_call_test_acquire", 3 },
+		{ "bpf_kfunc_call_test_release", 9 },
+		{ "bpf_kfunc_call_test_release", 13 },
+		{ "bpf_kfunc_call_test_release", 17 },
+	},
+	.result_unpriv = REJECT,
+	.result = REJECT,
+	.errstr = "variable ptr_ access var_off=(0x0; 0x7) disallowed",
+},
 {
 	"calls: basic sanity",
 	.insns = {
-- 
2.35.1

Re: [PATCH bpf-next v4 0/8] Fixes for bad PTR_TO_BTF_ID offset

[Martin KaFai Lau]

On Sat, Mar 05, 2022 at 04:16:37AM +0530, Kumar Kartikeya Dwivedi wrote:
> This set fixes a bug related to bad var_off being permitted for kfunc call in
> case of PTR_TO_BTF_ID, consolidates offset checks for all register types allowed
> as helper or kfunc arguments into a common shared helper, and introduces a
> couple of other checks to harden the kfunc release logic and prevent future
> bugs. Some selftests are also included that fail in absence of these fixes,
> serving as demonstration of the issues being fixed.
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next v4 0/8] Fixes for bad PTR_TO_BTF_ID offset

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sat,  5 Mar 2022 04:16:37 +0530 you wrote:
> This set fixes a bug related to bad var_off being permitted for kfunc call in
> case of PTR_TO_BTF_ID, consolidates offset checks for all register types allowed
> as helper or kfunc arguments into a common shared helper, and introduces a
> couple of other checks to harden the kfunc release logic and prevent future
> bugs. Some selftests are also included that fail in absence of these fixes,
> serving as demonstration of the issues being fixed.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v4,1/8] bpf: Add check_func_arg_reg_off function
    https://git.kernel.org/bpf/bpf-next/c/25b35dd28138
  - [bpf-next,v4,2/8] bpf: Fix PTR_TO_BTF_ID var_off check
    https://git.kernel.org/bpf/bpf-next/c/655efe5089f0
  - [bpf-next,v4,3/8] bpf: Disallow negative offset in check_ptr_off_reg
    https://git.kernel.org/bpf/bpf-next/c/e1fad0ff46b3
  - [bpf-next,v4,4/8] bpf: Harden register offset checks for release helpers and kfuncs
    https://git.kernel.org/bpf/bpf-next/c/24d5bb806c7e
  - [bpf-next,v4,5/8] compiler-clang.h: Add __diag infrastructure for clang
    https://git.kernel.org/bpf/bpf-next/c/f014a00bbeb0
  - [bpf-next,v4,6/8] compiler_types.h: Add unified __diag_ignore_all for GCC/LLVM
    https://git.kernel.org/bpf/bpf-next/c/4d1ea705d797
  - [bpf-next,v4,7/8] bpf: Replace __diag_ignore with unified __diag_ignore_all
    https://git.kernel.org/bpf/bpf-next/c/0b206c6d1066
  - [bpf-next,v4,8/8] selftests/bpf: Add tests for kfunc register offset checks
    https://git.kernel.org/bpf/bpf-next/c/8218ccb5bd68

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next v4 5/8] compiler-clang.h: Add __diag infrastructure for clang

[Nick Desaulniers]

On Fri, Mar 4, 2022 at 2:47 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> From: Nathan Chancellor <nathan@kernel.org>
>
> Add __diag macros similar to those in compiler-gcc.h, so that warnings
> that need to be adjusted for specific cases but not globally can be
> ignored when building with clang.
>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: llvm@lists.linux.dev
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> [ Kartikeya: wrote commit message ]
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  include/linux/compiler-clang.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 3c4de9b6c6e3..f1aa41d520bd 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h

The equivalent functionality for GCC has
357 #ifndef __diag_GCC
358 #define __diag_GCC(version, severity, string)
359 #endif
in include/linux/compiler_types.h. Should this patch as well? (at
least #define __diag_clang`)?

> @@ -68,3 +68,25 @@
>
>  #define __nocfi                __attribute__((__no_sanitize__("cfi")))
>  #define __cficanonical __attribute__((__cfi_canonical_jump_table__))
> +
> +/*
> + * Turn individual warnings and errors on and off locally, depending
> + * on version.
> + */
> +#define __diag_clang(version, severity, s) \
> +       __diag_clang_ ## version(__diag_clang_ ## severity s)
> +
> +/* Severity used in pragma directives */
> +#define __diag_clang_ignore    ignored
> +#define __diag_clang_warn      warning
> +#define __diag_clang_error     error

These severities match GCC. I wonder if rather than copy+pasting these
over, we could rework __diag_ignore, __diag_warn, and __diag_error to
not invoke a compiler-suffixed macro and rather pass the compiler
along (or make it implicit since we know CONFIG_CC_IS_CLANG vs
CONFIG_CC_IS_GCC)?  We can probably land this than follow up on better
code-reuse between compilers for diagnostics.

> +
> +#define __diag_str1(s)         #s
> +#define __diag_str(s)          __diag_str1(s)
> +#define __diag(s)              _Pragma(__diag_str(clang diagnostic s))
> +
> +#if CONFIG_CLANG_VERSION >= 110000
> +#define __diag_clang_11(s)     __diag(s)
> +#else
> +#define __diag_clang_11(s)
> +#endif
> --
> 2.35.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH bpf-next v4 5/8] compiler-clang.h: Add __diag infrastructure for clang

[Nick Desaulniers]

Also, please cc me and the llvm mailing list on all changes to
include/linux/compiler-clang.h. I see now on lore there's further
patches here than just this single patch I was cc'ed on.

On Mon, Mar 7, 2022 at 11:25 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Mar 4, 2022 at 2:47 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
> >
> > From: Nathan Chancellor <nathan@kernel.org>
> >
> > Add __diag macros similar to those in compiler-gcc.h, so that warnings
> > that need to be adjusted for specific cases but not globally can be
> > ignored when building with clang.
> >
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: llvm@lists.linux.dev
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > [ Kartikeya: wrote commit message ]
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  include/linux/compiler-clang.h | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index 3c4de9b6c6e3..f1aa41d520bd 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
>
> The equivalent functionality for GCC has
> 357 #ifndef __diag_GCC
> 358 #define __diag_GCC(version, severity, string)
> 359 #endif
> in include/linux/compiler_types.h. Should this patch as well? (at
> least #define __diag_clang`)?
>
> > @@ -68,3 +68,25 @@
> >
> >  #define __nocfi                __attribute__((__no_sanitize__("cfi")))
> >  #define __cficanonical __attribute__((__cfi_canonical_jump_table__))
> > +
> > +/*
> > + * Turn individual warnings and errors on and off locally, depending
> > + * on version.
> > + */
> > +#define __diag_clang(version, severity, s) \
> > +       __diag_clang_ ## version(__diag_clang_ ## severity s)
> > +
> > +/* Severity used in pragma directives */
> > +#define __diag_clang_ignore    ignored
> > +#define __diag_clang_warn      warning
> > +#define __diag_clang_error     error
>
> These severities match GCC. I wonder if rather than copy+pasting these
> over, we could rework __diag_ignore, __diag_warn, and __diag_error to
> not invoke a compiler-suffixed macro and rather pass the compiler
> along (or make it implicit since we know CONFIG_CC_IS_CLANG vs
> CONFIG_CC_IS_GCC)?  We can probably land this than follow up on better
> code-reuse between compilers for diagnostics.
>
> > +
> > +#define __diag_str1(s)         #s
> > +#define __diag_str(s)          __diag_str1(s)
> > +#define __diag(s)              _Pragma(__diag_str(clang diagnostic s))
> > +
> > +#if CONFIG_CLANG_VERSION >= 110000
> > +#define __diag_clang_11(s)     __diag(s)
> > +#else
> > +#define __diag_clang_11(s)
> > +#endif
> > --
> > 2.35.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH bpf-next v4 6/8] compiler_types.h: Add unified __diag_ignore_all for GCC/LLVM

[Nick Desaulniers]

On Sat, Mar 05, 2022 at 04:16:43AM +0530, Kumar Kartikeya Dwivedi wrote:
> Add a __diag_ignore_all macro, to ignore warnings for both GCC and LLVM,
> without having to specify the compiler type and version. By default, GCC
> 8 and clang 11 are used. This will be used by bpf subsystem to ignore
> -Wmissing-prototypes warning for functions that are meant to be global
> functions so that they are in vmlinux BTF, but don't have a prototype.
> 
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  include/linux/compiler-clang.h | 3 +++
>  include/linux/compiler-gcc.h   | 3 +++
>  include/linux/compiler_types.h | 4 ++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index f1aa41d520bd..babb1347148c 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -90,3 +90,6 @@
>  #else
>  #define __diag_clang_11(s)
>  #endif
> +
> +#define __diag_ignore_all(option, comment) \
> +	__diag_clang(11, ignore, option)
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index ccbbd31b3aae..d364c98a4a80 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -151,6 +151,9 @@
>  #define __diag_GCC_8(s)
>  #endif
>  
> +#define __diag_ignore_all(option, comment) \
> +	__diag_GCC(8, ignore, option)

While this approach will work for clang, it doesn't seem scalable for
GCC. Documentation/process/changes.rst documents that we support gcc
5.1+. This approach will only disable diagnostics for gcc 8+.

> +
>  /*
>   * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
>   * attribute) do not work, and must be disabled.
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 3f31ff400432..8e5d2f50f951 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -371,4 +371,8 @@ struct ftrace_likely_data {
>  #define __diag_error(compiler, version, option, comment) \
>  	__diag_ ## compiler(version, error, option)
>  
> +#ifndef __diag_ignore_all
> +#define __diag_ignore_all(option, comment)
> +#endif
> +
>  #endif /* __LINUX_COMPILER_TYPES_H */
> -- 
> 2.35.1
> 

Re: [PATCH bpf-next v4 5/8] compiler-clang.h: Add __diag infrastructure for clang

[Kumar Kartikeya Dwivedi]

On Tue, Mar 08, 2022 at 12:57:20AM IST, Nick Desaulniers wrote:
> Also, please cc me and the llvm mailing list on all changes to
> include/linux/compiler-clang.h. I see now on lore there's further
> patches here than just this single patch I was cc'ed on.
>

Mea culpa, sorry about that.

> On Mon, Mar 7, 2022 at 11:25 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Fri, Mar 4, 2022 at 2:47 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
> > >
> > > From: Nathan Chancellor <nathan@kernel.org>
> > >
> > > Add __diag macros similar to those in compiler-gcc.h, so that warnings
> > > that need to be adjusted for specific cases but not globally can be
> > > ignored when building with clang.
> > >
> > > Cc: Nathan Chancellor <nathan@kernel.org>
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > Cc: llvm@lists.linux.dev
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > [ Kartikeya: wrote commit message ]
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > ---
> > >  include/linux/compiler-clang.h | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > > index 3c4de9b6c6e3..f1aa41d520bd 100644
> > > --- a/include/linux/compiler-clang.h
> > > +++ b/include/linux/compiler-clang.h
> >
> > The equivalent functionality for GCC has
> > 357 #ifndef __diag_GCC
> > 358 #define __diag_GCC(version, severity, string)
> > 359 #endif
> > in include/linux/compiler_types.h. Should this patch as well? (at
> > least #define __diag_clang`)?
> >

I left it out because there are no users for it, no code is doing
__diag_ignore(clang, ...), all of the current ones are hardcoding e.g.
__diag_ignore(GCC, 8, ...).

> > > @@ -68,3 +68,25 @@
> > >
> > >  #define __nocfi                __attribute__((__no_sanitize__("cfi")))
> > >  #define __cficanonical __attribute__((__cfi_canonical_jump_table__))
> > > +
> > > +/*
> > > + * Turn individual warnings and errors on and off locally, depending
> > > + * on version.
> > > + */
> > > +#define __diag_clang(version, severity, s) \
> > > +       __diag_clang_ ## version(__diag_clang_ ## severity s)
> > > +
> > > +/* Severity used in pragma directives */
> > > +#define __diag_clang_ignore    ignored
> > > +#define __diag_clang_warn      warning
> > > +#define __diag_clang_error     error
> >
> > These severities match GCC. I wonder if rather than copy+pasting these
> > over, we could rework __diag_ignore, __diag_warn, and __diag_error to
> > not invoke a compiler-suffixed macro and rather pass the compiler
> > along (or make it implicit since we know CONFIG_CC_IS_CLANG vs
> > CONFIG_CC_IS_GCC)?  We can probably land this than follow up on better
> > code-reuse between compilers for diagnostics.
> >

That was the idea with __diag_ignore_all in the next patch. FYI this series has
already been applied.

> > > +
> > > +#define __diag_str1(s)         #s
> > > +#define __diag_str(s)          __diag_str1(s)
> > > +#define __diag(s)              _Pragma(__diag_str(clang diagnostic s))
> > > +
> > > +#if CONFIG_CLANG_VERSION >= 110000
> > > +#define __diag_clang_11(s)     __diag(s)
> > > +#else
> > > +#define __diag_clang_11(s)
> > > +#endif
> > > --
> > > 2.35.1
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

--
Kartikeya

Re: [PATCH bpf-next v4 6/8] compiler_types.h: Add unified __diag_ignore_all for GCC/LLVM

[Kumar Kartikeya Dwivedi]

On Tue, Mar 08, 2022 at 01:00:18AM IST, Nick Desaulniers wrote:
> On Sat, Mar 05, 2022 at 04:16:43AM +0530, Kumar Kartikeya Dwivedi wrote:
> > Add a __diag_ignore_all macro, to ignore warnings for both GCC and LLVM,
> > without having to specify the compiler type and version. By default, GCC
> > 8 and clang 11 are used. This will be used by bpf subsystem to ignore
> > -Wmissing-prototypes warning for functions that are meant to be global
> > functions so that they are in vmlinux BTF, but don't have a prototype.
> >
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  include/linux/compiler-clang.h | 3 +++
> >  include/linux/compiler-gcc.h   | 3 +++
> >  include/linux/compiler_types.h | 4 ++++
> >  3 files changed, 10 insertions(+)
> >
> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index f1aa41d520bd..babb1347148c 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
> > @@ -90,3 +90,6 @@
> >  #else
> >  #define __diag_clang_11(s)
> >  #endif
> > +
> > +#define __diag_ignore_all(option, comment) \
> > +	__diag_clang(11, ignore, option)
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index ccbbd31b3aae..d364c98a4a80 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -151,6 +151,9 @@
> >  #define __diag_GCC_8(s)
> >  #endif
> >
> > +#define __diag_ignore_all(option, comment) \
> > +	__diag_GCC(8, ignore, option)
>
> While this approach will work for clang, it doesn't seem scalable for
> GCC. Documentation/process/changes.rst documents that we support gcc
> 5.1+. This approach will only disable diagnostics for gcc 8+.
>

ISTM the original commit adding these macros only defined them for GCC 8+, so
coverage for previous versions is already not there, unrelated to this change.
I am not sure what the reason for that was, though.

> > +
> >  /*
> >   * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
> >   * attribute) do not work, and must be disabled.
> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index 3f31ff400432..8e5d2f50f951 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -371,4 +371,8 @@ struct ftrace_likely_data {
> >  #define __diag_error(compiler, version, option, comment) \
> >  	__diag_ ## compiler(version, error, option)
> >
> > +#ifndef __diag_ignore_all
> > +#define __diag_ignore_all(option, comment)
> > +#endif
> > +
> >  #endif /* __LINUX_COMPILER_TYPES_H */
> > --
> > 2.35.1
> >

--
Kartikeya