Skip to content

Commit

Permalink
bpf: Don't return EINVAL from {get,set}sockopt when optlen > PAGE_SIZE
Browse files Browse the repository at this point in the history
Attaching to these hooks can break iptables because its optval is
usually quite big, or at least bigger than the current PAGE_SIZE limit.
David also mentioned some SCTP options can be big (around 256k).

For such optvals we expose only the first PAGE_SIZE bytes to
the BPF program. BPF program has two options:
1. Set ctx->optlen to 0 to indicate that the BPF's optval
   should be ignored and the kernel should use original userspace
   value.
2. Set ctx->optlen to something that's smaller than the PAGE_SIZE.

v5:
* use ctx->optlen == 0 with trimmed buffer (Alexei Starovoitov)
* update the docs accordingly

v4:
* use temporary buffer to avoid optval == optval_end == NULL;
  this removes the corner case in the verifier that might assume
  non-zero PTR_TO_PACKET/PTR_TO_PACKET_END.

v3:
* don't increase the limit, bypass the argument

v2:
* proper comments formatting (Jakub Kicinski)

Fixes: 0d01da6 ("bpf: implement getsockopt and setsockopt hooks")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Cc: David Laight <David.Laight@ACULAB.COM>
Link: https://lore.kernel.org/bpf/20200617010416.93086-1-sdf@google.com
  • Loading branch information
Stanislav Fomichev authored and Alexei Starovoitov committed Jun 17, 2020
1 parent 99c5106 commit d8fe449
Showing 1 changed file with 33 additions and 20 deletions.
53 changes: 33 additions & 20 deletions kernel/bpf/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1276,16 +1276,23 @@ 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)
if (unlikely(max_optlen < 0))
return -EINVAL;

if (unlikely(max_optlen > PAGE_SIZE)) {
/* We don't expose optvals that are greater than PAGE_SIZE
* to the BPF program.
*/
max_optlen = PAGE_SIZE;
}

ctx->optval = kzalloc(max_optlen, GFP_USER);
if (!ctx->optval)
return -ENOMEM;

ctx->optval_end = ctx->optval + max_optlen;

return 0;
return max_optlen;
}

static void sockopt_free_buf(struct bpf_sockopt_kern *ctx)
Expand Down Expand Up @@ -1319,13 +1326,13 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
*/
max_optlen = max_t(int, 16, *optlen);

ret = sockopt_alloc_buf(&ctx, max_optlen);
if (ret)
return ret;
max_optlen = sockopt_alloc_buf(&ctx, max_optlen);
if (max_optlen < 0)
return max_optlen;

ctx.optlen = *optlen;

if (copy_from_user(ctx.optval, optval, *optlen) != 0) {
if (copy_from_user(ctx.optval, optval, min(*optlen, max_optlen)) != 0) {
ret = -EFAULT;
goto out;
}
Expand Down Expand Up @@ -1353,8 +1360,14 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
/* export any potential modifications */
*level = ctx.level;
*optname = ctx.optname;
*optlen = ctx.optlen;
*kernel_optval = ctx.optval;

/* optlen == 0 from BPF indicates that we should
* use original userspace data.
*/
if (ctx.optlen != 0) {
*optlen = ctx.optlen;
*kernel_optval = ctx.optval;
}
}

out:
Expand Down Expand Up @@ -1385,12 +1398,12 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
__cgroup_bpf_prog_array_is_empty(cgrp, BPF_CGROUP_GETSOCKOPT))
return retval;

ret = sockopt_alloc_buf(&ctx, max_optlen);
if (ret)
return ret;

ctx.optlen = max_optlen;

max_optlen = sockopt_alloc_buf(&ctx, max_optlen);
if (max_optlen < 0)
return max_optlen;

if (!retval) {
/* If kernel getsockopt finished successfully,
* copy whatever was returned to the user back
Expand All @@ -1404,10 +1417,8 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
goto out;
}

if (ctx.optlen > max_optlen)
ctx.optlen = max_optlen;

if (copy_from_user(ctx.optval, optval, ctx.optlen) != 0) {
if (copy_from_user(ctx.optval, optval,
min(ctx.optlen, max_optlen)) != 0) {
ret = -EFAULT;
goto out;
}
Expand Down Expand Up @@ -1436,10 +1447,12 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
goto out;
}

if (copy_to_user(optval, ctx.optval, ctx.optlen) ||
put_user(ctx.optlen, optlen)) {
ret = -EFAULT;
goto out;
if (ctx.optlen != 0) {
if (copy_to_user(optval, ctx.optval, ctx.optlen) ||
put_user(ctx.optlen, optlen)) {
ret = -EFAULT;
goto out;
}
}

ret = ctx.retval;
Expand Down

0 comments on commit d8fe449

Please sign in to comment.