Commit 4fb126cb authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'sockhash-fixes' 



John Fastabend says:

====================
First three patches resolve issues found while testing sockhash and
reviewing code. Syzbot also found them about the same time as I was
working on fixes. The main issue is in the sockhash path we reduced
the scope of sk_callback lock but this meant we could get update and
close running in parallel so fix that here.

Then testing sk_msg and sk_skb programs together found that skb->dev
is not always assigned and some of the helpers were depending on this
to lookup max mtu. Fix this by using SKB_MAX_ALLOC when no MTU is
available.

Finally, Martin spotted that the sockmap code was still using the
qdisc skb cb structure. But I was sure we had fixed this long ago.
Looks like we missed it in a merge conflict resolution and then by
chance data_end offset was the same in both structures so everything
sort of continued to work even though it could break at any moment
if the structs ever change. So redo the conversion and this time
also convert the helpers.

v2: fix '0 files changed' issue in patches
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 631da853 0ea488ff

include/net/tcp.h

+4 −0
Original line number Diff line number Diff line
@@ -828,6 +828,10 @@ struct tcp_skb_cb { 


#define TCP_SKB_CB(__skb)	((struct tcp_skb_cb *)&((__skb)->cb[0]))



static inline void bpf_compute_data_end_sk_skb(struct sk_buff *skb)

{

	TCP_SKB_CB(skb)->bpf.data_end = skb->data + skb_headlen(skb);

}



#if IS_ENABLED(CONFIG_IPV6)

/* This is the variant of inet6_iif() that must be used by TCP,

kernel/bpf/sockmap.c

+21 −3
Original line number Diff line number Diff line
@@ -312,10 +312,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout) 
	struct smap_psock *psock;

	struct sock *osk;



	lock_sock(sk);

	rcu_read_lock();

	psock = smap_psock_sk(sk);

	if (unlikely(!psock)) {

		rcu_read_unlock();

		release_sock(sk);

		return sk->sk_prot->close(sk, timeout);

	}
@@ -371,6 +373,7 @@ static void bpf_tcp_close(struct sock *sk, long timeout) 
		e = psock_map_pop(sk, psock);

	}

	rcu_read_unlock();

	release_sock(sk);

	close_fun(sk, timeout);

}
@@ -568,6 +571,7 @@ static int free_sg(struct sock *sk, int start, struct sk_msg_buff *md) 
	while (sg[i].length) {

		free += sg[i].length;

		sk_mem_uncharge(sk, sg[i].length);

		if (!md->skb)

			put_page(sg_page(&sg[i]));

		sg[i].length = 0;

		sg[i].page_link = 0;
@@ -577,6 +581,8 @@ static int free_sg(struct sock *sk, int start, struct sk_msg_buff *md) 
		if (i == MAX_SKB_FRAGS)

			i = 0;

	}

	if (md->skb)

		consume_skb(md->skb);



	return free;

}
@@ -1230,7 +1236,7 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb) 
	 */

	TCP_SKB_CB(skb)->bpf.sk_redir = NULL;

	skb->sk = psock->sock;

	bpf_compute_data_pointers(skb);

	bpf_compute_data_end_sk_skb(skb);

	preempt_disable();

	rc = (*prog->bpf_func)(skb, prog->insnsi);

	preempt_enable();
@@ -1485,7 +1491,7 @@ static int smap_parse_func_strparser(struct strparser *strp, 
	 * any socket yet.

	 */

	skb->sk = psock->sock;

	bpf_compute_data_pointers(skb);

	bpf_compute_data_end_sk_skb(skb);

	rc = (*prog->bpf_func)(skb, prog->insnsi);

	skb->sk = NULL;

	rcu_read_unlock();
@@ -2069,7 +2075,13 @@ static int sock_map_update_elem(struct bpf_map *map, 
		return -EOPNOTSUPP;

	}



	lock_sock(skops.sk);

	preempt_disable();

	rcu_read_lock();

	err = sock_map_ctx_update_elem(&skops, map, key, flags);

	rcu_read_unlock();

	preempt_enable();

	release_sock(skops.sk);

	fput(socket->file);

	return err;

}
@@ -2410,7 +2422,13 @@ static int sock_hash_update_elem(struct bpf_map *map, 
		return -EINVAL;

	}



	lock_sock(skops.sk);

	preempt_disable();

	rcu_read_lock();

	err = sock_hash_ctx_update_elem(&skops, map, key, flags);

	rcu_read_unlock();

	preempt_enable();

	release_sock(skops.sk);

	fput(socket->file);

	return err;

}

kernel/bpf/syscall.c

+3 −1
Original line number Diff line number Diff line
@@ -735,7 +735,9 @@ static int map_update_elem(union bpf_attr *attr) 
	if (bpf_map_is_dev_bound(map)) {

		err = bpf_map_offload_update_elem(map, key, value, attr->flags);

		goto out;

	} else if (map->map_type == BPF_MAP_TYPE_CPUMAP) {

	} else if (map->map_type == BPF_MAP_TYPE_CPUMAP ||

		   map->map_type == BPF_MAP_TYPE_SOCKHASH ||

		   map->map_type == BPF_MAP_TYPE_SOCKMAP) {

		err = map->ops->map_update_elem(map, key, value, attr->flags);

		goto out;

	}

net/core/filter.c

+93 −8
Original line number Diff line number Diff line
@@ -1762,6 +1762,37 @@ static const struct bpf_func_proto bpf_skb_pull_data_proto = { 
	.arg2_type	= ARG_ANYTHING,

};



static inline int sk_skb_try_make_writable(struct sk_buff *skb,

					   unsigned int write_len)

{

	int err = __bpf_try_make_writable(skb, write_len);



	bpf_compute_data_end_sk_skb(skb);

	return err;

}



BPF_CALL_2(sk_skb_pull_data, struct sk_buff *, skb, u32, len)

{

	/* Idea is the following: should the needed direct read/write

	 * test fail during runtime, we can pull in more data and redo

	 * again, since implicitly, we invalidate previous checks here.

	 *

	 * Or, since we know how much we need to make read/writeable,

	 * this can be done once at the program beginning for direct

	 * access case. By this we overcome limitations of only current

	 * headroom being accessible.

	 */

	return sk_skb_try_make_writable(skb, len ? : skb_headlen(skb));

}



static const struct bpf_func_proto sk_skb_pull_data_proto = {

	.func		= sk_skb_pull_data,

	.gpl_only	= false,

	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_PTR_TO_CTX,

	.arg2_type	= ARG_ANYTHING,

};



BPF_CALL_5(bpf_l3_csum_replace, struct sk_buff *, skb, u32, offset,

	   u64, from, u64, to, u64, flags)

{
@@ -2779,7 +2810,8 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 len_diff) 


static u32 __bpf_skb_max_len(const struct sk_buff *skb)

{

	return skb->dev->mtu + skb->dev->hard_header_len;

	return skb->dev ? skb->dev->mtu + skb->dev->hard_header_len :

			  SKB_MAX_ALLOC;

}



static int bpf_skb_adjust_net(struct sk_buff *skb, s32 len_diff)
@@ -2863,8 +2895,8 @@ static int bpf_skb_trim_rcsum(struct sk_buff *skb, unsigned int new_len) 
	return __skb_trim_rcsum(skb, new_len);

}



BPF_CALL_3(bpf_skb_change_tail, struct sk_buff *, skb, u32, new_len,

	   u64, flags)

static inline int __bpf_skb_change_tail(struct sk_buff *skb, u32 new_len,

					u64 flags)

{

	u32 max_len = __bpf_skb_max_len(skb);

	u32 min_len = __bpf_skb_min_len(skb);
@@ -2900,6 +2932,13 @@ BPF_CALL_3(bpf_skb_change_tail, struct sk_buff *, skb, u32, new_len, 
		if (!ret && skb_is_gso(skb))

			skb_gso_reset(skb);

	}

	return ret;

}



BPF_CALL_3(bpf_skb_change_tail, struct sk_buff *, skb, u32, new_len,

	   u64, flags)

{

	int ret = __bpf_skb_change_tail(skb, new_len, flags);



	bpf_compute_data_pointers(skb);

	return ret;
@@ -2914,8 +2953,26 @@ static const struct bpf_func_proto bpf_skb_change_tail_proto = { 
	.arg3_type	= ARG_ANYTHING,

};



BPF_CALL_3(bpf_skb_change_head, struct sk_buff *, skb, u32, head_room,

BPF_CALL_3(sk_skb_change_tail, struct sk_buff *, skb, u32, new_len,

	   u64, flags)

{

	int ret = __bpf_skb_change_tail(skb, new_len, flags);



	bpf_compute_data_end_sk_skb(skb);

	return ret;

}



static const struct bpf_func_proto sk_skb_change_tail_proto = {

	.func		= sk_skb_change_tail,

	.gpl_only	= false,

	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_PTR_TO_CTX,

	.arg2_type	= ARG_ANYTHING,

	.arg3_type	= ARG_ANYTHING,

};



static inline int __bpf_skb_change_head(struct sk_buff *skb, u32 head_room,

					u64 flags)

{

	u32 max_len = __bpf_skb_max_len(skb);

	u32 new_len = skb->len + head_room;
@@ -2941,8 +2998,16 @@ BPF_CALL_3(bpf_skb_change_head, struct sk_buff *, skb, u32, head_room, 
		skb_reset_mac_header(skb);

	}



	return ret;

}



BPF_CALL_3(bpf_skb_change_head, struct sk_buff *, skb, u32, head_room,

	   u64, flags)

{

	int ret = __bpf_skb_change_head(skb, head_room, flags);



	bpf_compute_data_pointers(skb);

	return 0;

	return ret;

}



static const struct bpf_func_proto bpf_skb_change_head_proto = {
@@ -2954,6 +3019,23 @@ static const struct bpf_func_proto bpf_skb_change_head_proto = { 
	.arg3_type	= ARG_ANYTHING,

};



BPF_CALL_3(sk_skb_change_head, struct sk_buff *, skb, u32, head_room,

	   u64, flags)

{

	int ret = __bpf_skb_change_head(skb, head_room, flags);



	bpf_compute_data_end_sk_skb(skb);

	return ret;

}



static const struct bpf_func_proto sk_skb_change_head_proto = {

	.func		= sk_skb_change_head,

	.gpl_only	= false,

	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_PTR_TO_CTX,

	.arg2_type	= ARG_ANYTHING,

	.arg3_type	= ARG_ANYTHING,

};

static unsigned long xdp_get_metalen(const struct xdp_buff *xdp)

{

	return xdp_data_meta_unsupported(xdp) ? 0 :
@@ -4617,9 +4699,12 @@ bool bpf_helper_changes_pkt_data(void *func) 
	    func == bpf_skb_store_bytes ||

	    func == bpf_skb_change_proto ||

	    func == bpf_skb_change_head ||

	    func == sk_skb_change_head ||

	    func == bpf_skb_change_tail ||

	    func == sk_skb_change_tail ||

	    func == bpf_skb_adjust_room ||

	    func == bpf_skb_pull_data ||

	    func == sk_skb_pull_data ||

	    func == bpf_clone_redirect ||

	    func == bpf_l3_csum_replace ||

	    func == bpf_l4_csum_replace ||
@@ -4871,11 +4956,11 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) 
	case BPF_FUNC_skb_load_bytes:

		return &bpf_skb_load_bytes_proto;

	case BPF_FUNC_skb_pull_data:

		return &bpf_skb_pull_data_proto;

		return &sk_skb_pull_data_proto;

	case BPF_FUNC_skb_change_tail:

		return &bpf_skb_change_tail_proto;

		return &sk_skb_change_tail_proto;

	case BPF_FUNC_skb_change_head:

		return &bpf_skb_change_head_proto;

		return &sk_skb_change_head_proto;

	case BPF_FUNC_get_socket_cookie:

		return &bpf_get_socket_cookie_proto;

	case BPF_FUNC_get_socket_uid:

CRA Git | Maintained and supported by SUSTech CRA and CCSE