From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF5BC2EA151; Wed, 11 Feb 2026 19:35:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770838518; cv=none; b=V/MDHAxGZpICbafu4yOouEV4l+d0URaAC1Yw+YAjKXw6Mp/yR02R8zkN+9R8bpgDOj8Zuq+aOj76+cwQzrjOaSWOMXIUFGoK3K9MrRIj9xJvT9wz7HSXDxNZ5jTEHgaYxUzF5M1oBHJJ8B9s9exgYKyXMX/OC5UpSW/LmQedA1o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770838518; c=relaxed/simple; bh=GJMKJk6tNO9S0i/TxbGgoYQmEPPQ3hetsy1cZ4ScCC8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KiGwjqvEV9urFbxzEXc/K+IpgEg0A4gNKe4RQJlREoLFStxQvb2RwRja3UVtAAF/K/AHkvYJQ+AmO9QZ1EJNkTVvOzur1NO7rw1cecQbgpOJHU8an+NIGoisOXY3ovWdiUolZaMbZ65I23vF/9eIpcSmNqWncWnyJY394pD/Yrs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=bqCCdv59; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="bqCCdv59" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 83C01C4CEF7; Wed, 11 Feb 2026 19:35:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770838518; bh=GJMKJk6tNO9S0i/TxbGgoYQmEPPQ3hetsy1cZ4ScCC8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bqCCdv59hcRrGri7mFAPt+bg0qMTj5puuMYsN1BfBfuliLpa8WRP0WNT+jOfWTnGy XYhqC0AMgWv7FtBiZzE/p/OKYHYEnZpXvbHqJldvhYiUHlmdJQ9Nk5b3qynujUbwb+ mqfA9Gp1lC5NseLhmR5R6GP/wsymTRREf9/0KPSCGaBR6p6Bwvba8BgmzVUG7Oy8qm e+nMF4hmWPDqLmfmnlLOyOG1798n8TZUiQc2WQy4s2w5umbO3P3BB5IgYKvJg8yCZp KyVeOGe/m8uLnujt/zWzNGz1jZ9TlrsGrFIvl/GMKV3ESv+RnyW5Bk+Qxa2kf0BLbQ ddq39cNUkWz2g== Date: Wed, 11 Feb 2026 20:35:12 +0100 From: Alexey Gladkov To: Christian Brauner Cc: Dan Klishch , Al Viro , "Eric W . Biederman" , Kees Cook , containers@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 2/5] proc: subset=pid: Show /proc/self/net only for CAP_NET_ADMIN Message-ID: References: <20251213050639.735940-1-danilklishch@gmail.com> <20260204-bergung-abhilfe-073d732bc51f@brauner> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260204-bergung-abhilfe-073d732bc51f@brauner> On Wed, Feb 04, 2026 at 03:39:53PM +0100, Christian Brauner wrote: > On Tue, Jan 13, 2026 at 10:20:34AM +0100, Alexey Gladkov wrote: > > Cache the mounters credentials and allow access to the net directories > > contingent of the permissions of the mounter of proc. > > > > Do not show /proc/self/net when proc is mounted with subset=pid option > > and the mounter does not have CAP_NET_ADMIN. > > > > Signed-off-by: Alexey Gladkov > > --- > > fs/proc/proc_net.c | 8 ++++++++ > > fs/proc/root.c | 5 +++++ > > include/linux/proc_fs.h | 1 + > > 3 files changed, 14 insertions(+) > > > > diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c > > index 52f0b75cbce2..6e0ccef0169f 100644 > > --- a/fs/proc/proc_net.c > > +++ b/fs/proc/proc_net.c > > @@ -23,6 +23,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "internal.h" > > > > @@ -270,6 +271,7 @@ static struct net *get_proc_task_net(struct inode *dir) > > struct task_struct *task; > > struct nsproxy *ns; > > struct net *net = NULL; > > + struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb); > > > > rcu_read_lock(); > > task = pid_task(proc_pid(dir), PIDTYPE_PID); > > @@ -282,6 +284,12 @@ static struct net *get_proc_task_net(struct inode *dir) > > } > > rcu_read_unlock(); > > > > + if (net && (fs_info->pidonly == PROC_PIDONLY_ON) && > > + security_capable(fs_info->mounter_cred, net->user_ns, CAP_NET_ADMIN, CAP_OPT_NONE) < 0) { > > + put_net(net); > > + net = NULL; > > + } > > + > > return net; > > } > > > > diff --git a/fs/proc/root.c b/fs/proc/root.c > > index d8ca41d823e4..ed8a101d09d3 100644 > > --- a/fs/proc/root.c > > +++ b/fs/proc/root.c > > @@ -254,6 +254,7 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc) > > return -ENOMEM; > > > > fs_info->pid_ns = get_pid_ns(ctx->pid_ns); > > + fs_info->mounter_cred = get_cred(fc->cred); > > proc_apply_options(fs_info, fc, current_user_ns()); > > > > /* User space would break if executables or devices appear on proc */ > > @@ -303,6 +304,9 @@ static int proc_reconfigure(struct fs_context *fc) > > > > sync_filesystem(sb); > > > > + put_cred(fs_info->mounter_cred); > > + fs_info->mounter_cred = get_cred(fc->cred); > > Afaict, this races with get_proc_task_net(). You need a synchronization > mechanism here so that get_proc_task_net() doesn't risk accessing > invalid mounter creds while someone concurrently updates the creds. > Proposal how to fix that below. > > But I'm kinda torn here anyway whether we want that credential change on > remount. The problem is that someone might inadvertently allow access to > /proc//net as a side-effect simply because they remounted procfs. > But they never had a chance to prevent this. I think you're right, and there's no need to change credentials on remount. At least not now. > I think it's best if mounter_creds stays fixed just as they do for > overlayfs. So we don't allow them to change on reconfigure. That also > makes all of the code I hinted at below pointless. I'll just remove the mounter_cred update from proc_reconfigure. > If we ever want to change the credentials it's easier to add a mount > option to procfs like I did for overlayfs. > > _Untested_ patches: > > First, the preparatory patch diff (no functional changes intended): > > diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c > index 52f0b75cbce2..81825e5819b8 100644 > --- a/fs/proc/proc_net.c > +++ b/fs/proc/proc_net.c > @@ -268,19 +268,19 @@ EXPORT_SYMBOL_GPL(proc_create_net_single_write); > static struct net *get_proc_task_net(struct inode *dir) > { > struct task_struct *task; > - struct nsproxy *ns; > - struct net *net = NULL; > + struct net *net; > > - rcu_read_lock(); > + guard(rcu)(); > task = pid_task(proc_pid(dir), PIDTYPE_PID); > - if (task != NULL) { > - task_lock(task); > - ns = task->nsproxy; > - if (ns != NULL) > - net = get_net(ns->net_ns); > - task_unlock(task); > + if (!task) > + return NULL; > + > + scoped_guard(task_lock, task) { > + struct nsproxy *ns = task->nsproxy; > + if (!ns) > + return NULL; > + net = get_net(ns->net_ns); > } > - rcu_read_unlock(); > > return net; > } > > And then on top of it something like: > > diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c > index 81825e5819b8..47dc9806395c 100644 > --- a/fs/proc/proc_net.c > +++ b/fs/proc/proc_net.c > @@ -269,6 +269,8 @@ static struct net *get_proc_task_net(struct inode *dir) > { > struct task_struct *task; > struct net *net; > + struct proc_fs_info *fs_info; > + const struct cred *cred; > > guard(rcu)(); > task = pid_task(proc_pid(dir), PIDTYPE_PID); > @@ -282,6 +284,15 @@ static struct net *get_proc_task_net(struct inode *dir) > net = get_net(ns->net_ns); > } > > + fs_info = proc_sb_info(dir->i_sb); > + if (fs_info->pidonly != PROC_PIDONLY_ON) > + return net; > + > + cred = rcu_dereference(fs_info->mounter_cred); > + if (security_capable(cred, net->user_ns, CAP_NET_ADMIN, CAP_OPT_NONE) != 0) { > + put_net(net); > + return NULL; > + } > return net; > } > > diff --git a/fs/proc/root.c b/fs/proc/root.c > index d8ca41d823e4..68397900dab7 100644 > --- a/fs/proc/root.c > +++ b/fs/proc/root.c > @@ -300,11 +300,15 @@ static int proc_reconfigure(struct fs_context *fc) > { > struct super_block *sb = fc->root->d_sb; > struct proc_fs_info *fs_info = proc_sb_info(sb); > + const struct cred *cred; > > sync_filesystem(sb); > > - proc_apply_options(fs_info, fc, current_user_ns()); > - return 0; > + cred = rcu_replace_pointer(fs_info->mounter_cred, get_cred(fc->cred), > + lockdep_is_held(&sb->s_umount)); > + put_cred(cred); > + > + return proc_apply_options(sb, fc, current_user_ns()); > } > > static int proc_get_tree(struct fs_context *fc) > -- Rgrds, legion