diff mbox

[v10,11/12] bpf: enable direct packet data write for xdp progs

Message ID 1468955817-10604-12-git-send-email-bblanco@plumgrid.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Brenden Blanco July 19, 2016, 7:16 p.m. UTC
For forwarding to be effective, XDP programs should be allowed to
rewrite packet data.

This requires that the drivers supporting XDP must all map the packet
memory as TODEVICE or BIDIRECTIONAL before invoking the program.

Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>
---
 kernel/bpf/verifier.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Alexei Starovoitov July 19, 2016, 9:59 p.m. UTC | #1
On Tue, Jul 19, 2016 at 12:16:56PM -0700, Brenden Blanco wrote:
> For forwarding to be effective, XDP programs should be allowed to
> rewrite packet data.
> 
> This requires that the drivers supporting XDP must all map the packet
> memory as TODEVICE or BIDIRECTIONAL before invoking the program.
> 
> Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>
> ---
>  kernel/bpf/verifier.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a8d67d0..f72f23b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -653,6 +653,16 @@ static int check_map_access(struct verifier_env *env, u32 regno, int off,
>  
>  #define MAX_PACKET_OFF 0xffff
>  
> +static bool may_write_pkt_data(enum bpf_prog_type type)
> +{
> +	switch (type) {
> +	case BPF_PROG_TYPE_XDP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  static int check_packet_access(struct verifier_env *env, u32 regno, int off,
>  			       int size)
>  {
> @@ -806,10 +816,15 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
>  			err = check_stack_read(state, off, size, value_regno);
>  		}
>  	} else if (state->regs[regno].type == PTR_TO_PACKET) {
> -		if (t == BPF_WRITE) {
> +		if (t == BPF_WRITE && !may_write_pkt_data(env->prog->type)) {
>  			verbose("cannot write into packet\n");
>  			return -EACCES;
>  		}
> +		if (t == BPF_WRITE && value_regno >= 0 &&
> +		    is_pointer_value(env, value_regno)) {
> +			verbose("R%d leaks addr into packet\n", value_regno);
> +			return -EACCES;
> +		}

Like this extra security check :) though it's arguably overkill.

Acked-by: Alexei Starovoitov <ast@kernel.org>
diff mbox

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a8d67d0..f72f23b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -653,6 +653,16 @@  static int check_map_access(struct verifier_env *env, u32 regno, int off,
 
 #define MAX_PACKET_OFF 0xffff
 
+static bool may_write_pkt_data(enum bpf_prog_type type)
+{
+	switch (type) {
+	case BPF_PROG_TYPE_XDP:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int check_packet_access(struct verifier_env *env, u32 regno, int off,
 			       int size)
 {
@@ -806,10 +816,15 @@  static int check_mem_access(struct verifier_env *env, u32 regno, int off,
 			err = check_stack_read(state, off, size, value_regno);
 		}
 	} else if (state->regs[regno].type == PTR_TO_PACKET) {
-		if (t == BPF_WRITE) {
+		if (t == BPF_WRITE && !may_write_pkt_data(env->prog->type)) {
 			verbose("cannot write into packet\n");
 			return -EACCES;
 		}
+		if (t == BPF_WRITE && value_regno >= 0 &&
+		    is_pointer_value(env, value_regno)) {
+			verbose("R%d leaks addr into packet\n", value_regno);
+			return -EACCES;
+		}
 		err = check_packet_access(env, regno, off, size);
 		if (!err && t == BPF_READ && value_regno >= 0)
 			mark_reg_unknown_value(state->regs, value_regno);