Re: [PATCH bpf-next v2 1/5] bpf: add bpf_memcmp kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf_memcmp bypasses verifier memory bounds checking by using __ign arguments, creating an arbitrary kernel memory read oracle.
--

commit 29136173f7ef78c9e1fa53cea458359e9ef94899
Author: Longjun Tang <tanglongjun@kylinos.cn>

bpf: add bpf_memcmp kfunc

This commit adds a new bpf_memcmp kfunc for binary memory comparison in BPF
programs. Unlike bpf_strcmp, null bytes do not terminate the comparison,
making it suitable for comparing fixed-size binary data like MAC addresses
and UUIDs.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 8e196c9b7c50f..897e8d71eae45 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> @@ -4070,6 +4070,48 @@ __bpf_kfunc int bpf_strcspn(const char *s__ign, const char *reject__ign)
>  	return -EFAULT;
>  }
>  
> +/**
> + * bpf_memcmp - Compare two memory regions byte by byte
> + * @ptr1__ign: First memory region
> + * @ptr2__ign: Second memory region
> + * @size: Number of bytes to compare
> + *
> + * Compares the first @size bytes of the memory areas @ptr1__ign and
> + * @ptr2__ign. Unlike bpf_strcmp, null bytes do not terminate the comparison.
> + *
> + * Return:
> + * * %0       - The memory regions are equal
> + * * %-1      - @ptr1__ign is less than @ptr2__ign
> + * * %1       - @ptr1__ign is greater than @ptr2__ign
> + * * %-EFAULT - Cannot read one of the memory regions
> + * * %-E2BIG  - @size exceeds the maximum allowed size
> + * * %-ERANGE - One of the memory regions is outside of kernel address space
> + */
> +__bpf_kfunc int bpf_memcmp(const void *ptr1__ign, const void *ptr2__ign,
> +			   size_t size)

[Severity: Critical]
Does the __ign suffix on the pointer arguments bypass the verifier's memory
bounds checking?

If the verifier skips type and bounds checking for ptr1__ign and ptr2__ign,
could this regression allow passing arbitrary scalars as pointers?

Since the function reads from these addresses using __get_kernel_nofault
and returns a byte-by-byte comparison result, can this be used as an
arbitrary kernel memory read oracle via binary search?

Would it be safer to use KF_ARG_PTR_TO_MEM (for example, by using a __sz
suffix) to enforce static verifier bounds checking instead of relying on
runtime nofault reads?

> +{
> +	const unsigned char *p1 = ptr1__ign, *p2 = ptr2__ign;
> +	unsigned char c1, c2;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624093638.117062-1-lange_tang@163.com?part=1