mbox series

[bpf,v2,0/2] Fix bpf_skb_load_bytes_relative for cgroup_skb/egress

Message ID cover.1591812755.git.zhuyifei@google.com
Headers show
Series Fix bpf_skb_load_bytes_relative for cgroup_skb/egress | expand

Message

YiFei Zhu June 10, 2020, 6:41 p.m. UTC 
When cgroup_skb/egress triggers the MAC header is not set. On the other hand,
load_bytes_relative unconditionally calls skb_mac_header which, when MC not
set, returns a pointer after the tail pointer, breaking the logic even if the
caller requested the NET header.

Fix is to conditionally use skb_mac_header or skb_network_header depending on
the requested header, -EFAULT when the header is not set. Added a test that
asserts during cgroup_skb/egress request for MAC header returns -EFAULT and
request for NET header succeeds.

Updates since v1:
* Reverted the bound condition check to account for bad offset parameter
  larger than data length.
* Add test asssertion for failure return code on the condition above.

YiFei Zhu (2):
  net/filter: Permit reading NET in load_bytes_relative when MAC not set
  selftests/bpf: Add cgroup_skb/egress test for load_bytes_relative

 net/core/filter.c                             | 16 +++--
 .../bpf/prog_tests/load_bytes_relative.c      | 71 +++++++++++++++++++
 .../selftests/bpf/progs/load_bytes_relative.c | 48 +++++++++++++
 3 files changed, 128 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/load_bytes_relative.c
 create mode 100644 tools/testing/selftests/bpf/progs/load_bytes_relative.c

--
2.27.0

Comments

Daniel Borkmann June 11, 2020, 2:25 p.m. UTC | #1 
On 6/10/20 8:41 PM, YiFei Zhu wrote:
> When cgroup_skb/egress triggers the MAC header is not set. On the other hand,
> load_bytes_relative unconditionally calls skb_mac_header which, when MC not
> set, returns a pointer after the tail pointer, breaking the logic even if the
> caller requested the NET header.
> 
> Fix is to conditionally use skb_mac_header or skb_network_header depending on
> the requested header, -EFAULT when the header is not set. Added a test that
> asserts during cgroup_skb/egress request for MAC header returns -EFAULT and
> request for NET header succeeds.
> 
> Updates since v1:
> * Reverted the bound condition check to account for bad offset parameter
>    larger than data length.
> * Add test asssertion for failure return code on the condition above.
> 
> YiFei Zhu (2):
>    net/filter: Permit reading NET in load_bytes_relative when MAC not set
>    selftests/bpf: Add cgroup_skb/egress test for load_bytes_relative
> 
>   net/core/filter.c                             | 16 +++--
>   .../bpf/prog_tests/load_bytes_relative.c      | 71 +++++++++++++++++++
>   .../selftests/bpf/progs/load_bytes_relative.c | 48 +++++++++++++
>   3 files changed, 128 insertions(+), 7 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/load_bytes_relative.c
>   create mode 100644 tools/testing/selftests/bpf/progs/load_bytes_relative.c
> 
> --
> 2.27.0
> 

Applied, thanks!