bpf: don't use MAX_SELECTORS_MASK for args

Currently, the max number of selectors is the same as the max number of
configured args, so we don't have problems due to the fact that we use
the MAX_SELECTORS_MASK on arg indexes.

This change makes arg index masking use MAX_POSSIBLE_ARGS_INDEX for
clarity and to decouple these independent concepts.

EVENT_CONFIG_MAX_ARG is replaced everywhere with MAX_POSSIBLE_ARGS
because these two values cannot deviate. There are places where we mask
an index that is used for both event config and kprobe messages.  This
consolidation helps in those cases.

Convenience masks are added for the usdt and reg arg masks, because
these arrays can/do have different sizes than the original arg arrays.

There is a limit on the number of args that a user can config. There is
also an unrelated limit about which args can be reached within a
tracepoint/function signature. Both of these limits are currently 5, but
they are logically unrelated limits. However, we use the same arg index
mask for both.  This change decouples these two concepts from a masking
perspective so that we could increase the number of arguments that can
be reached from a function signature perspective without increasing the
allowed number of configured arguments.

Signed-off-by: Andy Strohman <astrohma@isovalent.com>

Author: andrewstrohman

bpf/lib/generic.h

@@ -69,6 +69,10 @@ struct msg_generic_kprobe {
 	__u64 user_stack_id; // User Stack trace ID
 	/* anything above is shared with the userspace so it should match structs MsgGenericKprobe and MsgGenericTracepoint in Go */
 	char args[24000];
+#define MAX_ACCESSIBLE_ARGS 5
+/* convenience mask for verifier appeasing*/
+#define MAX_ACCESSIBLE_ARGS_MASK 0x7
+	_Static_assert(MAX_ACCESSIBLE_ARGS - 1 <= MAX_ACCESSIBLE_ARGS_MASK, "Need to update MAX_ACCESSIBLE_ARGS_MASK");
 	unsigned long a0, a1, a2, a3, a4;
 	long argsoff[MAX_POSSIBLE_ARGS];
 	arg_status_t arg_status[MAX_POSSIBLE_ARGS];

bpf/process/event_config.h

@@ -46,16 +46,20 @@ struct extract_arg_data {
 };
 
 #define MAX_BTF_ARG_DEPTH	  10
-#define EVENT_CONFIG_MAX_ARG	  5
 #define EVENT_CONFIG_MAX_USDT_ARG 8
 #define EVENT_CONFIG_MAX_REG_ARG  8
+/* convenience masks for verifier appeasing*/
+#define EVENT_CONFIG_MAX_USDT_ARG_MASK 0x7
+_Static_assert(EVENT_CONFIG_MAX_USDT_ARG - 1 <= EVENT_CONFIG_MAX_USDT_ARG_MASK, "Need to update EVENT_CONFIG_MAX_USDT_ARG_MASK");
+#define EVENT_CONFIG_MAX_REG_ARG_MASK 0x7
+_Static_assert(EVENT_CONFIG_MAX_REG_ARG - 1 <= EVENT_CONFIG_MAX_REG_ARG_MASK, "Need to update EVENT_CONFIG_MAX_REG_ARG_MASK");
 
 struct event_config {
 	__u32 func_id;
-	__s32 arg[EVENT_CONFIG_MAX_ARG];
-	__u32 arm[EVENT_CONFIG_MAX_ARG];
-	__u32 off[EVENT_CONFIG_MAX_ARG];
-	__s32 idx[EVENT_CONFIG_MAX_ARG];
+	__s32 arg[MAX_POSSIBLE_ARGS];
+	__u32 arm[MAX_POSSIBLE_ARGS];
+	__u32 off[MAX_POSSIBLE_ARGS];
+	__s32 idx[MAX_POSSIBLE_ARGS];
 	__u32 syscall;
 	__s32 argreturncopy;
 	__s32 argreturn;
@@ -70,7 +74,7 @@ struct event_config {
 	__u32 policy_id;
 	__u32 flags;
 	__u32 pad;
-	struct config_btf_arg btf_arg[EVENT_CONFIG_MAX_ARG][MAX_BTF_ARG_DEPTH];
+	struct config_btf_arg btf_arg[MAX_POSSIBLE_ARGS][MAX_BTF_ARG_DEPTH];
 	struct config_usdt_arg usdt_arg[EVENT_CONFIG_MAX_USDT_ARG];
 	struct config_reg_arg reg_arg[EVENT_CONFIG_MAX_REG_ARG];
 } __attribute__((packed));
@@ -91,7 +95,7 @@ FUNC_INLINE int arg_idx(int index)
 
 	asm volatile("%[index] &= %1 ;\n"
 		     : [index] "+r"(index)
-		     : "i"(MAX_SELECTORS_MASK));
+		     : "i"(MAX_POSSIBLE_ARGS_MASK));
 	return config->idx[index];
 }
 

bpf/process/generic_calls.h

@@ -508,9 +508,9 @@ FUNC_INLINE void extract_arg(struct event_config *config, int index, unsigned lo
 
 	asm volatile("%[index] &= %1 ;\n"
 		     : [index] "+r"(index)
-		     : "i"(MAX_SELECTORS_MASK));
+		     : "i"(MAX_POSSIBLE_ARGS_MASK));
 
-	if (index >= EVENT_CONFIG_MAX_ARG)
+	if (index >= MAX_POSSIBLE_ARGS)
 		return;
 
 	btf_config = config->btf_arg[index];
@@ -572,7 +572,7 @@ FUNC_INLINE long get_pt_regs_arg(struct pt_regs *ctx, struct event_config *confi
 
 	asm volatile("%[index] &= %1 ;\n"
 		     : [index] "+r"(index)
-		     : "i"(MAX_SELECTORS_MASK));
+		     : "i"(EVENT_CONFIG_MAX_REG_ARG_MASK));
 	reg = &config->reg_arg[index];
 	shift = 64 - reg->size * 8;
 
@@ -629,9 +629,17 @@ FUNC_INLINE long generic_read_arg(void *ctx, int index, long off, struct bpf_map
 	if (!config)
 		return 0;
 
+#if defined(GENERIC_TRACEPOINT) || defined(GENERIC_USDT)
+	asm volatile("%[index] &= %1 ;\n"
+		     : [index] "+r"(index)
+		     : "i"(MAX_ACCESSIBLE_ARGS_MASK));
+
+	a = (&e->a0)[index];
+#endif
+
 	asm volatile("%[index] &= %1 ;\n"
 		     : [index] "+r"(index)
-		     : "i"(MAX_SELECTORS_MASK));
+		     : "i"(MAX_POSSIBLE_ARGS_MASK));
 	ty = config->arg[index];
 	am = config->arm[index];
 
@@ -645,14 +653,9 @@ FUNC_INLINE long generic_read_arg(void *ctx, int index, long off, struct bpf_map
 	e->arg_status[index] = 0;
 
 #if defined(GENERIC_TRACEPOINT) || defined(GENERIC_USDT)
-	a = (&e->a0)[index];
 	extract_arg(config, index, &a, &e->arg_status[index]);
 #else
 	arg_index = config->idx[index];
-	asm volatile("%[arg_index] &= %1 ;\n"
-		     : [arg_index] "+r"(arg_index)
-		     : "i"(MAX_SELECTORS_MASK));
-
 	/* Getting argument data based on the source attribute, which is encoded
 	 * in argument meta data, so far it's either:
 	 *
@@ -661,14 +664,21 @@ FUNC_INLINE long generic_read_arg(void *ctx, int index, long off, struct bpf_map
 	 *   - current task object
 	 *   - real argument value
 	 */
-	if (am & ARGM_PT_REGS_PRELOAD)
+	if (am & ARGM_PT_REGS_PRELOAD) {
 		a = get_pt_regs_preload_arg(ctx, ty);
-	else if (am & ARGM_PT_REGS)
+	} else if (am & ARGM_PT_REGS) {
+		asm volatile("%[arg_index] &= %1 ;\n"
+			     : [arg_index] "+r"(arg_index)
+			     : "i"(EVENT_CONFIG_MAX_REG_ARG_MASK));
 		a = get_pt_regs_arg(ctx, config, arg_index);
-	else if (am & ARGM_CURRENT_TASK)
+	} else if (am & ARGM_CURRENT_TASK) {
 		a = get_current_task();
-	else
+	} else {
+		asm volatile("%[arg_index] &= %1 ;\n"
+			     : [arg_index] "+r"(arg_index)
+			     : "i"(MAX_ACCESSIBLE_ARGS_MASK));
 		a = (&e->a0)[arg_index];
+	}
 
 	extract_arg(config, index, &a, &e->arg_status[index]);
 
@@ -753,7 +763,7 @@ read_usdt_arg(struct pt_regs *ctx, struct event_config *config, int index)
 	unsigned long val, off, idx;
 	int err;
 
-	index &= 7;
+	index &= EVENT_CONFIG_MAX_USDT_ARG_MASK;
 	arg = &config->usdt_arg[index];
 
 	if (arg->type == USDT_ARG_TYPE_NONE)
@@ -1009,7 +1019,7 @@ do_set_action(void *ctx, struct msg_generic_kprobe *e, __u32 arg_idx, __u32 arg_
 	if (!config)
 		return;
 
-	arg_idx &= 7;
+	arg_idx &= EVENT_CONFIG_MAX_USDT_ARG_MASK;
 	arg = &config->usdt_arg[arg_idx];
 
 	switch (arg->type) {

bpf/process/uprobe_preload.h

@@ -59,7 +59,7 @@ preload_pt_regs_arg(struct pt_regs *ctx, struct event_config *config, int index)
 
 	asm volatile("%[index] &= %1 ;\n"
 		     : [index] "+r"(index)
-		     : "i"(MAX_SELECTORS_MASK));
+		     : "i"(EVENT_CONFIG_MAX_REG_ARG_MASK));
 
 	reg = &config->reg_arg[index];
 	shift = 64 - reg->size * 8;