Message ID | 20180124075802.1522053-5-brakmo@fb.com |
---|---|
State | Changes Requested, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | bpf: More sock_ops callbacks | expand |
On Tue, Jan 23, 2018 at 11:57 PM, Lawrence Brakmo <brakmo@fb.com> wrote: > Currently, a sock_ops BPF program can write the op field and all the > reply fields (reply and replylong). This is a bug. The op field should > not have been writeable and there is currently no way to use replylong > field for indices >= 1. This patch enforces that only the reply field > (which equals replylong[0]) is writeable. Would this patch be more suitable for -net ? > > Fixes: 40304b2a1567 ("bpf: BPF support for sock_ops") > Signed-off-by: Lawrence Brakmo <brakmo@fb.com> Acked-by: Yuchung Cheng <ycheng@google.com> > --- > net/core/filter.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 0cf170f..c356ec0 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -3845,8 +3845,7 @@ static bool sock_ops_is_valid_access(int off, int size, > { > if (type == BPF_WRITE) { > switch (off) { > - case offsetof(struct bpf_sock_ops, op) ... > - offsetof(struct bpf_sock_ops, replylong[3]): > + case offsetof(struct bpf_sock_ops, reply): > break; > default: > return false; > -- > 2.9.5 >
On 1/24/18 11:58 AM, Yuchung Cheng wrote: > On Tue, Jan 23, 2018 at 11:57 PM, Lawrence Brakmo <brakmo@fb.com> wrote: >> Currently, a sock_ops BPF program can write the op field and all the >> reply fields (reply and replylong). This is a bug. The op field should >> not have been writeable and there is currently no way to use replylong >> field for indices >= 1. This patch enforces that only the reply field >> (which equals replylong[0]) is writeable. > Would this patch be more suitable for -net ? yes. we will send it to 4.15 and 4.14 as soon as it's released. See discussion with Eric. >> >> Fixes: 40304b2a1567 ("bpf: BPF support for sock_ops") >> Signed-off-by: Lawrence Brakmo <brakmo@fb.com> > Acked-by: Yuchung Cheng <ycheng@google.com> > >> --- >> net/core/filter.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/net/core/filter.c b/net/core/filter.c >> index 0cf170f..c356ec0 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -3845,8 +3845,7 @@ static bool sock_ops_is_valid_access(int off, int size, >> { >> if (type == BPF_WRITE) { >> switch (off) { >> - case offsetof(struct bpf_sock_ops, op) ... >> - offsetof(struct bpf_sock_ops, replylong[3]): >> + case offsetof(struct bpf_sock_ops, reply): >> break; >> default: >> return false; >> -- >> 2.9.5 >>
diff --git a/net/core/filter.c b/net/core/filter.c index 0cf170f..c356ec0 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3845,8 +3845,7 @@ static bool sock_ops_is_valid_access(int off, int size, { if (type == BPF_WRITE) { switch (off) { - case offsetof(struct bpf_sock_ops, op) ... - offsetof(struct bpf_sock_ops, replylong[3]): + case offsetof(struct bpf_sock_ops, reply): break; default: return false;
Currently, a sock_ops BPF program can write the op field and all the reply fields (reply and replylong). This is a bug. The op field should not have been writeable and there is currently no way to use replylong field for indices >= 1. This patch enforces that only the reply field (which equals replylong[0]) is writeable. Fixes: 40304b2a1567 ("bpf: BPF support for sock_ops") Signed-off-by: Lawrence Brakmo <brakmo@fb.com> --- net/core/filter.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)