On 06/04/2019 04:54 PM, Daniel Borkmann wrote:

On 06/03/2019 06:38 PM, Jonathan Lemon wrote:

quoted

Currently, the AF_XDP code uses a separate map in order to
determine if an xsk is bound to a queue.  Instead of doing this,
have bpf_map_lookup_elem() return the queue_id, as a way of
indicating that there is a valid entry at the map index.

Rearrange some xdp_sock members to eliminate structure holes.

Signed-off-by: Jonathan Lemon <redacted>
Acked-by: Song Liu <redacted>
Acked-by: Björn Töpel <redacted>
---
 include/net/xdp_sock.h                            |  6 +++---
 kernel/bpf/verifier.c                             |  6 +++++-
 kernel/bpf/xskmap.c                               |  4 +++-
 .../selftests/bpf/verifier/prevent_map_lookup.c   | 15 ---------------
 4 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index d074b6d60f8a..7d84b1da43d2 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h

@@ -57,12 +57,12 @@ struct xdp_sock {
 	struct net_device *dev;
 	struct xdp_umem *umem;
 	struct list_head flush_node;
-	u16 queue_id;
-	struct xsk_queue *tx ____cacheline_aligned_in_smp;
-	struct list_head list;
+	u32 queue_id;
 	bool zc;
 	/* Protects multiple processes in the control path */
 	struct mutex mutex;
+	struct xsk_queue *tx ____cacheline_aligned_in_smp;
+	struct list_head list;
 	/* Mutual exclusion of NAPI TX thread and sendmsg error paths
 	 * in the SKB destructor callback.
 	 */

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2778417e6e0c..91c730f85e92 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c

@@ -2905,10 +2905,14 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
 	 * appear.
 	 */
 	case BPF_MAP_TYPE_CPUMAP:
-	case BPF_MAP_TYPE_XSKMAP:
 		if (func_id != BPF_FUNC_redirect_map)
 			goto error;
 		break;
+	case BPF_MAP_TYPE_XSKMAP:
+		if (func_id != BPF_FUNC_redirect_map &&
+		    func_id != BPF_FUNC_map_lookup_elem)
+			goto error;
+		break;
 	case BPF_MAP_TYPE_ARRAY_OF_MAPS:
 	case BPF_MAP_TYPE_HASH_OF_MAPS:
 		if (func_id != BPF_FUNC_map_lookup_elem)

diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index 686d244e798d..249b22089014 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c

@@ -154,7 +154,9 @@ void __xsk_map_flush(struct bpf_map *map)
 
 static void *xsk_map_lookup_elem(struct bpf_map *map, void *key)
 {
-	return ERR_PTR(-EOPNOTSUPP);
+	struct xdp_sock *xs = __xsk_map_lookup_elem(map, *(u32 *)key);
+
+	return xs ? &xs->queue_id : NULL;
 }

How do you guarantee that BPf programs don't mess around with the map values
e.g. overriding xs->queue_id from the lookup? This should be read-only map
from BPF program side.

(Or via per-cpu scratch var where you move xs->queue_id into and return from here.)