pid: use ns_capable_noaudit() when determining net sysctl permissions

[PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Christian Göttsche 3 weeks, 1 day ago

From: Christian Göttsche <cgzones@googlemail.com>

The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.

The denial audit message can either lead to the capability being
unnecessarily allowed in a security policy, or being silenced potentially
masking a legitimate capability check at a later point in time.

Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
determining net sysctl permissions")

Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
CC: Christian Brauner <brauner@kernel.org>
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 kernel/pid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/pid.c b/kernel/pid.c
index c45a28c16cd2..d94ce0250501 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -680,7 +680,7 @@ static int pid_table_root_permissions(struct ctl_table_header *head,
 		container_of(head->set, struct pid_namespace, set);
 	int mode = table->mode;
 
-	if (ns_capable(pidns->user_ns, CAP_SYS_ADMIN) ||
+	if (ns_capable_noaudit(pidns->user_ns, CAP_SYS_ADMIN) ||
 	    uid_eq(current_euid(), make_kuid(pidns->user_ns, 0)))
 		mode = (mode & S_IRWXU) >> 6;
 	else if (in_egroup_p(make_kgid(pidns->user_ns, 0)))
-- 
2.51.0

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Christian Brauner 1 week, 6 days ago

On Wed, 10 Sep 2025 21:26:05 +0200, Christian Göttsche wrote:
> The capability check should not be audited since it is only being used
> to determine the inode permissions. A failed check does not indicate a
> violation of security policy but, when an LSM is enabled, a denial audit
> message was being generated.
> 
> The denial audit message can either lead to the capability being
> unnecessarily allowed in a security policy, or being silenced potentially
> masking a legitimate capability check at a later point in time.
> 
> [...]

Applied to the vfs-6.18.misc branch of the vfs/vfs.git tree.
Patches in the vfs-6.18.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.18.misc

[1/1] pid: use ns_capable_noaudit() when determining net sysctl permissions
      https://git.kernel.org/vfs/vfs/c/b9cb7e59ac4a

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Paul Moore 2 weeks, 6 days ago

On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> 
> The capability check should not be audited since it is only being used
> to determine the inode permissions. A failed check does not indicate a
> violation of security policy but, when an LSM is enabled, a denial audit
> message was being generated.
> 
> The denial audit message can either lead to the capability being
> unnecessarily allowed in a security policy, or being silenced potentially
> masking a legitimate capability check at a later point in time.
> 
> Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> determining net sysctl permissions")
> 
> Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> CC: Christian Brauner <brauner@kernel.org>
> CC: linux-security-module@vger.kernel.org
> CC: selinux@vger.kernel.org
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  kernel/pid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Paul Moore <paul@paul-moore.com>

--
paul-moore.com

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Serge E. Hallyn 2 weeks, 6 days ago

On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > 
> > The capability check should not be audited since it is only being used
> > to determine the inode permissions. A failed check does not indicate a
> > violation of security policy but, when an LSM is enabled, a denial audit
> > message was being generated.
> > 
> > The denial audit message can either lead to the capability being
> > unnecessarily allowed in a security policy, or being silenced potentially
> > masking a legitimate capability check at a later point in time.
> > 
> > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > determining net sysctl permissions")
> > 
> > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > CC: Christian Brauner <brauner@kernel.org>
> > CC: linux-security-module@vger.kernel.org
> > CC: selinux@vger.kernel.org
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  kernel/pid.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Paul Moore <paul@paul-moore.com>

Thanks.

Acked-by: Serge Hallyn <serge@hallyn.com>

I'll queue this up in the capability tree, unless Christian wanted
it in his.

-serge

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Serge E. Hallyn 2 weeks, 6 days ago

On Thu, Sep 11, 2025 at 06:05:05PM -0500, Serge E. Hallyn wrote:
> On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> > On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > > 
> > > The capability check should not be audited since it is only being used
> > > to determine the inode permissions. A failed check does not indicate a
> > > violation of security policy but, when an LSM is enabled, a denial audit
> > > message was being generated.
> > > 
> > > The denial audit message can either lead to the capability being
> > > unnecessarily allowed in a security policy, or being silenced potentially
> > > masking a legitimate capability check at a later point in time.
> > > 
> > > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > > determining net sysctl permissions")
> > > 
> > > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > > CC: Christian Brauner <brauner@kernel.org>
> > > CC: linux-security-module@vger.kernel.org
> > > CC: selinux@vger.kernel.org
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > >  kernel/pid.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Reviewed-by: Paul Moore <paul@paul-moore.com>
> 
> Thanks.
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> I'll queue this up in the capability tree, unless Christian wanted
> it in his.
> 
> -serge

I've included this in git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git#caps-next

thanks,
-serge

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Christian Brauner 2 weeks, 3 days ago

On Fri, Sep 12, 2025 at 02:35:45PM -0500, Serge Hallyn wrote:
> On Thu, Sep 11, 2025 at 06:05:05PM -0500, Serge E. Hallyn wrote:
> > On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> > > On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > > > 
> > > > The capability check should not be audited since it is only being used
> > > > to determine the inode permissions. A failed check does not indicate a
> > > > violation of security policy but, when an LSM is enabled, a denial audit
> > > > message was being generated.
> > > > 
> > > > The denial audit message can either lead to the capability being
> > > > unnecessarily allowed in a security policy, or being silenced potentially
> > > > masking a legitimate capability check at a later point in time.
> > > > 
> > > > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > > > determining net sysctl permissions")
> > > > 
> > > > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > > > CC: Christian Brauner <brauner@kernel.org>
> > > > CC: linux-security-module@vger.kernel.org
> > > > CC: selinux@vger.kernel.org
> > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > ---
> > > >  kernel/pid.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> > 
> > Thanks.
> > 
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> > 
> > I'll queue this up in the capability tree, unless Christian wanted
> > it in his.
> > 
> > -serge
> 
> I've included this in git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git#caps-next

What's easier for you? Just want me to stuff this in the pidfs pile?

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Serge E. Hallyn 2 weeks, 3 days ago

On Mon, Sep 15, 2025 at 04:13:16PM +0200, Christian Brauner wrote:
> On Fri, Sep 12, 2025 at 02:35:45PM -0500, Serge Hallyn wrote:
> > On Thu, Sep 11, 2025 at 06:05:05PM -0500, Serge E. Hallyn wrote:
> > > On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> > > > On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > > > > 
> > > > > The capability check should not be audited since it is only being used
> > > > > to determine the inode permissions. A failed check does not indicate a
> > > > > violation of security policy but, when an LSM is enabled, a denial audit
> > > > > message was being generated.
> > > > > 
> > > > > The denial audit message can either lead to the capability being
> > > > > unnecessarily allowed in a security policy, or being silenced potentially
> > > > > masking a legitimate capability check at a later point in time.
> > > > > 
> > > > > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > > > > determining net sysctl permissions")
> > > > > 
> > > > > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > > > > CC: Christian Brauner <brauner@kernel.org>
> > > > > CC: linux-security-module@vger.kernel.org
> > > > > CC: selinux@vger.kernel.org
> > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > > ---
> > > > >  kernel/pid.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> > > 
> > > Thanks.
> > > 
> > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > 
> > > I'll queue this up in the capability tree, unless Christian wanted
> > > it in his.
> > > 
> > > -serge
> > 
> > I've included this in git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git#caps-next
> 
> What's easier for you? Just want me to stuff this in the pidfs pile?

Yes, please go ahead, thanks.

Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions

Posted by Serge E. Hallyn 2 weeks, 3 days ago

On Mon, Sep 15, 2025 at 04:13:16PM +0200, Christian Brauner wrote:
> On Fri, Sep 12, 2025 at 02:35:45PM -0500, Serge Hallyn wrote:
> > On Thu, Sep 11, 2025 at 06:05:05PM -0500, Serge E. Hallyn wrote:
> > > On Thu, Sep 11, 2025 at 04:46:20PM -0400, Paul Moore wrote:
> > > > On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
> > > > > 
> > > > > The capability check should not be audited since it is only being used
> > > > > to determine the inode permissions. A failed check does not indicate a
> > > > > violation of security policy but, when an LSM is enabled, a denial audit
> > > > > message was being generated.
> > > > > 
> > > > > The denial audit message can either lead to the capability being
> > > > > unnecessarily allowed in a security policy, or being silenced potentially
> > > > > masking a legitimate capability check at a later point in time.
> > > > > 
> > > > > Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> > > > > determining net sysctl permissions")
> > > > > 
> > > > > Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> > > > > CC: Christian Brauner <brauner@kernel.org>
> > > > > CC: linux-security-module@vger.kernel.org
> > > > > CC: selinux@vger.kernel.org
> > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > > > ---
> > > > >  kernel/pid.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> > > 
> > > Thanks.
> > > 
> > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > 
> > > I'll queue this up in the capability tree, unless Christian wanted
> > > it in his.
> > > 
> > > -serge
> > 
> > I've included this in git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux.git#caps-next
> 
> What's easier for you? Just want me to stuff this in the pidfs pile?

Oh, sure - I can drop it from my tree, since it's the only thing in there
right now.