On Wed, 11 Mar 2020 at 23:27, John Fastabend [off-list ref] wrote:

Lorenz Bauer wrote:

quoted

Allow callers with CAP_NET_ADMIN to retrieve file descriptors from a
sockmap and sockhash. O_CLOEXEC is enforced on all fds.

Without this, it's difficult to resize or otherwise rebuild existing
sockmap or sockhashes.

Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Lorenz Bauer <redacted>
---
 net/core/sock_map.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 03e04426cd21..3228936aa31e 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c

@@ -347,12 +347,31 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
 static int __sock_map_copy_value(struct bpf_map *map, struct sock *sk,
                               void *value)
 {
+     struct file *file;
+     int fd;
+
      switch (map->value_size) {
      case sizeof(u64):
              sock_gen_cookie(sk);
              *(u64 *)value = atomic64_read(&sk->sk_cookie);
              return 0;

+     case sizeof(u32):
+             if (!capable(CAP_NET_ADMIN))
+                     return -EPERM;
+
+             fd = get_unused_fd_flags(O_CLOEXEC);
+             if (unlikely(fd < 0))
+                     return fd;
+
+             read_lock_bh(&sk->sk_callback_lock);
+             file = get_file(sk->sk_socket->file);
+             read_unlock_bh(&sk->sk_callback_lock);
+
+             fd_install(fd, file);
+             *(u32 *)value = fd;
+             return 0;
+

Hi Lorenz, Can you say something about what happens if the sk
is deleted from the map or the sock is closed/unhashed ideally
in the commit message so we have it for later reference. I guess
because we are in an rcu block here the sk will be OK and psock
reference will exist until after the rcu block at least because
of call_rcu(). If the psock is destroyed from another path then
the fd will still point at the sock. correct?

This is how I understand it:
* sk is protected by rcu_read_lock (as you point out)
* sk->sk_callback_lock protects against sk->sk_socket being
  modified by sock_orphan, sock_graft, etc. via sk_set_socket
* get_file increments the refcount on the file

I'm not sure how the psock figures into this, maybe you can
elaborate a little?

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com