On Thu, Jun 04, 2020 at 05:21:55PM -0700, Stanislav Fomichev wrote:
> Attaching to these hooks can break iptables because its optval is
> usually quite big, or at least bigger than the current PAGE_SIZE limit.
>
> There are two possible ways to fix it:
> 1. Increase the limit to match iptables max optval.
> 2. Implement some way to bypass the value if it's too big and trigger
> BPF only with level/optname so BPF can still decide whether
> to allow/deny big sockopts.
>
> I went with #1 which means we are potentially increasing the
> amount of data we copy from the userspace from PAGE_SIZE to 512M.
>
> v2:
> * proper comments formatting (Jakub Kicinski)
>
> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> ---
> kernel/bpf/cgroup.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index fdf7836750a3..fb786b0f0f88 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -1276,7 +1276,14 @@ static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
>
> static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen)
> {
> - if (unlikely(max_optlen > PAGE_SIZE) || max_optlen < 0)
> + /* The user with the largest known setsockopt optvals is iptables.
> + * Allocate enough space to accommodate it.
> + *
> + * See XT_MAX_TABLE_SIZE and sizeof(struct ipt_replace).
> + */
> + const int max_supported_optlen = 512 * 1024 * 1024 + 128;
looks like arbitrary number. Why did you pick this one?
Also it won't work with kzalloc() below.
May be trim it to some number instead of hard failing ?
bpf prog cannot really examine more than few kbytes.
> +
> + if (unlikely(max_optlen > max_supported_optlen) || max_optlen < 0)
> return -EINVAL;
>
> ctx->optval = kzalloc(max_optlen, GFP_USER);
> --
> 2.27.0.278.ge193c7cf3a9-goog
>
