Re: [PATCH bpf v2 5/7] bpf: Check the validity of nr_words in bpf_iter_bits_new()

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Oct 21, 2024 at 09:40:02AM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> Check the validity of nr_words in bpf_iter_bits_new(). Without this
> check, when multiplication overflow occurs for nr_bits (e.g., when
> nr_words = 0x0400-0001, nr_bits becomes 64), stack corruption may occur
> due to bpf_probe_read_kernel_common(..., nr_bytes = 0x2000-0008).
> 
> Fix it by limiting the max value of nr_words to 512.

lgtm, nice catch .. it's actually stated in the comment,
but we did not force it

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> Fixes: 4665415975b0 ("bpf: Add bits iterator")
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  kernel/bpf/helpers.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 62349e206a29..c147f75e1b48 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2851,6 +2851,8 @@ struct bpf_iter_bits {
>  	__u64 __opaque[2];
>  } __aligned(8);
>  
> +#define BITS_ITER_NR_WORDS_MAX 512
> +
>  struct bpf_iter_bits_kern {
>  	union {
>  		unsigned long *bits;
> @@ -2892,6 +2894,8 @@ bpf_iter_bits_new(struct bpf_iter_bits *it, const u64 *unsafe_ptr__ign, u32 nr_w
>  
>  	if (!unsafe_ptr__ign || !nr_words)
>  		return -EINVAL;
> +	if (nr_words > BITS_ITER_NR_WORDS_MAX)
> +		return -E2BIG;
>  
>  	/* Optimization for u64 mask */
>  	if (nr_bits == 64) {
> -- 
> 2.29.2
>