Received: by 2002:a25:e7d8:0:0:0:0:0 with SMTP id e207csp3607565ybh; Tue, 17 Mar 2020 03:18:41 -0700 (PDT) X-Google-Smtp-Source: ADFU+vsovK2ONYXAGo31kWJanFRdND0KfgfOOzUxMh097FUIjXToDFoOKhB2C10HiqmuP6tbKTMq X-Received: by 2002:a05:6830:1605:: with SMTP id g5mr3177982otr.268.1584440321636; Tue, 17 Mar 2020 03:18:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1584440321; cv=none; d=google.com; s=arc-20160816; b=EbEP9trR1q6EgK3xULwoWo/O/VnAjAn6TazWAsgph3PRxaykejQS0UNtt0A3Zu1tct 8ZSKnywxMWFDY+C65b6z/J9g0+vhYF8TJNpaEpXjDddOoqqMN+qMOCZ9BRS2tqcPWy2G rLsJtGpL2MkXcVlrgZSfS+3tVGoui6cn8sPy8k+hKS8j4AcZiEOieKL6Gwychzu1U748 qBmZznAIZF2m67qoXOvBu7ZQxzZTsO819B1BUcYL2I+g6FK5AcMGCWLBoplmp71RaARl J6oj1UeG153pbWKd3urOHwVIEtMcjpSG2L0rGxOQoe4JJxuVhiqC+TusKBShjLh6ETLq HvEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=oDa8VM8GdllhFAvMRWp4hpupCPaz/QUO5MsZDtEiqs0=; b=telJS6z+wEYfvsdRoFXIjKk+KY/RT3AbJl6WV2PwgM6BBZ1z1SBk9dL/QyASdPo/gY Kob1L+x2IdbCJK7RnqDhJYEoGVMb6B+XaFVL+HBilVMJhtgd28DEKpc8GNcIECIgcXvX 1dRUbRw4I2SCwMaUNoAnRqs83Vn41wCMcoGTni1KX/kssiJLS7A+brCJTxmzQyESAsx0 sFkkFrfbG8EATUO9xCBG4nLIVKIhK/gK2fxNs5bZF+XXocPkqJnzdv6nKtZ2OB+vN6QB PBG2GsLonnZcXBTN34OSDL1MLT6ST0OEqTEoy2/C8Id9QNdX5OROHqTBLAGv/kALDg5H Q+6w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@cloudflare.com header.s=google header.b=ltYysL5W; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=cloudflare.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j17si1380777otl.278.2020.03.17.03.18.29; Tue, 17 Mar 2020 03:18:41 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@cloudflare.com header.s=google header.b=ltYysL5W; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=cloudflare.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726272AbgCQKRv (ORCPT + 99 others); Tue, 17 Mar 2020 06:17:51 -0400 Received: from mail-ot1-f66.google.com ([209.85.210.66]:36447 "EHLO mail-ot1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725730AbgCQKRu (ORCPT ); Tue, 17 Mar 2020 06:17:50 -0400 Received: by mail-ot1-f66.google.com with SMTP id 39so5614855otu.3 for ; Tue, 17 Mar 2020 03:17:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=oDa8VM8GdllhFAvMRWp4hpupCPaz/QUO5MsZDtEiqs0=; b=ltYysL5WIC7dpKo4GMaDU8Ml3B6mi7mjaGVG65T2DEiDiGYOgzRyMLCrxrRWChP5HS yTQFrSduvcsM0xNp1YBTAIiflKZ/sMN8pGWZae/ArhXNCikTtXesCEck8fUXUCEp/w/C Iw+LoUythKxvdhc9mhmIz2zs2mGoFetgsadTo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=oDa8VM8GdllhFAvMRWp4hpupCPaz/QUO5MsZDtEiqs0=; b=dI25JgDjW0KEd4oFROKG4WW/WkBB2HAlrYDECtF0fpT4NdyP4JINuU5lbZxDbo5dM8 3QBN1s4IM6SVv/lYrjXYS4qsOV9uBnBHeoUFSQ6JO+emGg9ja4A/K8b1m9R/80iLSzmk klyqbvnMb+txSUOK2uLghjCZD/jNxBw14M1n4xRbXD7yGlDkOKwyWjqLPEaDq3iPsBKF LLKTGVSm2dzzhH66lldTukGvN6K6v7Sr24MyT/8rxJHs7Jh6j7UvkD8MyCUAh/GlYtMm OV5RjTTjqkiKPR4Ps+Sm3MQ8zGhX7pTYeIdeJnj46lgm3bmaKm8RLeOiY/J3KM3T0Q0a q73w== X-Gm-Message-State: ANhLgQ2n7W/NX9yg61DHihQF24zWk1U+fYt0ic8ByyPkvSI3OoA7rckv KPcl/Y3r3hPYsDXMow5+QBXIFFtuZ6ISQyWenFZC0g== X-Received: by 2002:a9d:30c7:: with SMTP id r7mr2911068otg.289.1584440268438; Tue, 17 Mar 2020 03:17:48 -0700 (PDT) MIME-Version: 1.0 References: <20200310174711.7490-1-lmb@cloudflare.com> <20200310174711.7490-5-lmb@cloudflare.com> <5e6973ed90f8d_20552ab9153405b4ca@john-XPS-13-9370.notmuch> In-Reply-To: <5e6973ed90f8d_20552ab9153405b4ca@john-XPS-13-9370.notmuch> From: Lorenz Bauer Date: Tue, 17 Mar 2020 10:17:37 +0000 Message-ID: Subject: Re: [PATCH 4/5] bpf: sockmap, sockhash: return file descriptors from privileged lookup To: John Fastabend Cc: Daniel Borkmann , Jakub Sitnicki , "David S. Miller" , Jakub Kicinski , Alexei Starovoitov , kernel-team , Networking , bpf , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 11 Mar 2020 at 23:27, John Fastabend wrote: > > Lorenz Bauer wrote: > > 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 > > Signed-off-by: Lorenz Bauer > > --- > > 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