Commit d8fe449a authored by Stanislav Fomichev's avatar Stanislav Fomichev Committed by Alexei Starovoitov
Browse files

bpf: Don't return EINVAL from {get,set}sockopt when optlen > PAGE_SIZE 



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: 0d01da6a ("bpf: implement getsockopt and setsockopt hooks")
Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Cc: David Laight <David.Laight@ACULAB.COM>
Link: https://lore.kernel.org/bpf/20200617010416.93086-1-sdf@google.com
parent 99c51064

kernel/bpf/cgroup.c

+33 −20
Original line number Diff line number Diff line
@@ -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)
@@ -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;

	}
@@ -1353,9 +1360,15 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level, 
		/* export any potential modifications */

		*level = ctx.level;

		*optname = ctx.optname;



		/* optlen == 0 from BPF indicates that we should

		 * use original userspace data.

		 */

		if (ctx.optlen != 0) {

			*optlen = ctx.optlen;

			*kernel_optval = ctx.optval;

		}

	}



out:

	if (ret)
@@ -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
@@ -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;

		}
@@ -1436,11 +1447,13 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level, 
		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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE