Re: [PATCH bpf-next v4] bpf: Lift hashtab key_size limit

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

 



On Wed, Oct 28, 2020 at 3:21 PM Florian Lehner <dev@xxxxxxxxxxx> wrote:
>
> Currently key_size of hashtab is limited to MAX_BPF_STACK.
> As the key of hashtab can also be a value from a per cpu map it can be
> larger than MAX_BPF_STACK.
>
> The use-case for this patch originates to implement allow/disallow
> lists for files and file paths. The maximum length of file paths is
> defined by PATH_MAX with 4096 chars including nul.
> This limit exceeds MAX_BPF_STACK.
>
> Changelog:
>
> v4:
>  - Utilize BPF skeleton in tests
>  - Rebase
>
> v3:
>  - Rebase
>
> v2:
>  - Add a test for bpf side
>
> Signed-off-by: Florian Lehner <dev@xxxxxxxxxxx>
> Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
> ---
>  kernel/bpf/hashtab.c                          | 16 ++-----
>  .../selftests/bpf/prog_tests/hash_large_key.c | 43 +++++++++++++++++
>  .../selftests/bpf/progs/test_hash_large_key.c | 46 +++++++++++++++++++
>  tools/testing/selftests/bpf/test_maps.c       |  3 +-
>  4 files changed, 96 insertions(+), 12 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/hash_large_key.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_hash_large_key.c
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 1815e97d4c9c..fff7cd05b9e3 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -390,17 +390,11 @@ static int htab_map_alloc_check(union bpf_attr *attr)
>             attr->value_size == 0)
>                 return -EINVAL;
>
> -       if (attr->key_size > MAX_BPF_STACK)
> -               /* eBPF programs initialize keys on stack, so they cannot be
> -                * larger than max stack size
> -                */
> -               return -E2BIG;
> -
> -       if (attr->value_size >= KMALLOC_MAX_SIZE -
> -           MAX_BPF_STACK - sizeof(struct htab_elem))
> -               /* if value_size is bigger, the user space won't be able to
> -                * access the elements via bpf syscall. This check also makes
> -                * sure that the elem_size doesn't overflow and it's
> +       if ((u64)(attr->key_size + attr->value_size) >= KMALLOC_MAX_SIZE -

this will add both as u32, then will cast overflown result to u64.
Instead just do:

if ((u64)attr->key_size + attr->value_size >= ....)

> +          sizeof(struct htab_elem))
> +               /* if key_size + value_size is bigger, the user space won't be
> +                * able to access the elements via bpf syscall. This check
> +                * also makes sure that the elem_size doesn't overflow and it's
>                  * kmalloc-able later in htab_map_update_elem()
>                  */
>                 return -E2BIG;

[...]

> +
> +SEC("raw_tracepoint/sys_enter")
> +int bpf_hash_large_key_test(void *ctx)
> +{
> +       int zero = 0, err = 1, value = 42;
> +       struct bigelement *key;
> +
> +       key = bpf_map_lookup_elem(&key_map, &zero);
> +       if (!key)
> +               goto err;
> +
> +       key->c = 1;
> +       if (bpf_map_update_elem(&hash_map, key, &value, BPF_ANY))
> +               goto err;
> +
> +       err = 0;
> +err:
> +       return err;

return value from raw_tracepoint doesn't really communicate error, you
might as well just return 0 always

> +}
> +
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index 0d92ebcb335d..0ad3e6305ff0 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -1223,9 +1223,10 @@ static void test_map_in_map(void)
>
>  static void test_map_large(void)
>  {
> +
>         struct bigkey {
>                 int a;
> -               char b[116];
> +               char b[4096];
>                 long long c;
>         } key;
>         int fd, i, value;
> --
> 2.26.2
>



[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