On Mon, Aug 03, 2020 at 12:36:30AM +0300, Dmitry Yakunin wrote:
> Now it's impossible to test all branches of cgroup_skb bpf program which
> accesses skb->family and skb->{local,remote}_ip{4,6} fields because they
> are zeroed during socket allocation. This commit fills socket family and
> addresses from related fields in constructed skb.
>
> v2:
> - fix build without CONFIG_IPV6 (kernel test robot <lkp@xxxxxxxxx>)
>
> v3:
> - check skb length before access to inet headers (Eric Dumazet)
>
> Signed-off-by: Dmitry Yakunin <zeil@xxxxxxxxxxxxxx>
> ---
> net/bpf/test_run.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index b03c469..8d69295 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -449,6 +449,27 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
> skb_reset_network_header(skb);
>
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + sk->sk_family = AF_INET;
> + if (pskb_may_pull(skb, sizeof(struct iphdr))) {
skb was just inited with __skb_put(skb, size);
Looking at pskb_may_pull() messes with my brain too much,
since it should never go into __pskb_pull_tail path.
Can you open code the skb->len check instead?
if (sizeof(struct iphdr) <= skb_headlen(skb)) {
> + sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> + sk->sk_daddr = ip_hdr(skb)->daddr;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case htons(ETH_P_IPV6):
> + sk->sk_family = AF_INET6;
> + if (pskb_may_pull(skb, sizeof(struct ipv6hdr))) {
> + sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> + sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
> + }
> + break;
> +#endif
> + default:
> + break;
> + }
> +
> if (is_l2)
> __skb_push(skb, hh_len);
> if (is_direct_pkt_access)
> --
> 2.7.4
>
