[RFC bpf-next] guard against access_size overflow?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Looking at commit ecc6a2101840 ("bpf: Protect against int overflow for
stack access size") and the associated syzbot report (linked below), it
seems that the underlying issue is that access_size argument
check_helper_mem_access() can be overflowed.

E.g. with a bloom filter where the value size is INT_MAX+2

  4: type 30  flags 0x0
          key 0B  value 2147483649B  max_entries 255  memlock 720B

The ARG_PTR_TO_MAP_VALUE case in check_func_arg() could overflow
access_size. (Potentially in the ARG_PTR_TO_MAP_KEY case as well)

  case ARG_PTR_TO_MAP_VALUE:
  	/* value_size is u32, access_size is int  */
  	err = check_helper_mem_access(env, regno,
  				      meta->map_ptr->value_size, false,
  				      meta);

Should we guard against such overflow? The easiest way seems to be in the
beginning of check_helper_mem_access(). But I'm not sure if there's
anywhere it is expected that access_size is negative?

AFAICT the main reason we have access_size defined as int is to match the
return type of bpf_size_to_bytes().

Link: https://lore.kernel.org/bpf/CAADnVQLORV5PT0iTAhRER+iLBTkByCYNBYyvBSgjN1T31K+gOw@xxxxxxxxxxxxxx/
Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@xxxxxxxx>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 77da1f438bec..96f2c4014937 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7284,6 +7284,9 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
 	struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
 	u32 *max_access;
 
+	if (access_size < 0)
+		return -EINVAL;
+
 	switch (base_type(reg->type)) {
 	case PTR_TO_PACKET:
 	case PTR_TO_PACKET_META:
@@ -8778,6 +8781,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			return -EACCES;
 		}
 		meta->raw_mode = arg_type & MEM_UNINIT;
+		/* This can overflow int access_size */
 		err = check_helper_mem_access(env, regno,
 					      meta->map_ptr->value_size, false,
 					      meta);

base-commit: a87f34e742d279d54d529e4bc4763fdaab32a466
-- 
2.45.1





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux