public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipc: don't audit capability check in ipc_permissions()
@ 2026-01-22 14:13 Ondrej Mosnacek
  2026-01-22 14:37 ` Alexey Gladkov
  2026-01-26 22:50 ` Paul Moore
  0 siblings, 2 replies; 6+ messages in thread
From: Ondrej Mosnacek @ 2026-01-22 14:13 UTC (permalink / raw)
  To: Andrew Morton, Eric W . Biederman
  Cc: Alexey Gladkov, linux-kernel, linux-security-module, selinux

The IPC sysctls implement the ctl_table_root::permissions hook and
they override the file access mode based on the CAP_CHECKPOINT_RESTORE
capability, which is being checked regardless of whether any access is
actually denied or not, so if an LSM denies the capability, an audit
record may be logged even when access is in fact granted.

It wouldn't be viable to restructure the sysctl permission logic to only
check the capability when the access would be actually denied if it's
not granted. Thus, do the same as in net_ctl_permissions()
(net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
so that the check never emits an audit record.

Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 include/linux/capability.h | 6 ++++++
 ipc/ipc_sysctl.c           | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 1fb08922552c7..37db92b3d6f89 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -203,6 +203,12 @@ static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns)
 		ns_capable(ns, CAP_SYS_ADMIN);
 }
 
+static inline bool checkpoint_restore_ns_capable_noaudit(struct user_namespace *ns)
+{
+	return ns_capable_noaudit(ns, CAP_CHECKPOINT_RESTORE) ||
+		ns_capable_noaudit(ns, CAP_SYS_ADMIN);
+}
+
 /* audit system wants to get cap info from files as well */
 int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
 			   const struct dentry *dentry,
diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 15b17e86e198c..9b087ebeb643b 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -214,7 +214,7 @@ static int ipc_permissions(struct ctl_table_header *head, const struct ctl_table
 	if (((table->data == &ns->ids[IPC_SEM_IDS].next_id) ||
 	     (table->data == &ns->ids[IPC_MSG_IDS].next_id) ||
 	     (table->data == &ns->ids[IPC_SHM_IDS].next_id)) &&
-	    checkpoint_restore_ns_capable(ns->user_ns))
+	    checkpoint_restore_ns_capable_noaudit(ns->user_ns))
 		mode = 0666;
 	else
 #endif
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
  2026-01-22 14:13 [PATCH] ipc: don't audit capability check in ipc_permissions() Ondrej Mosnacek
@ 2026-01-22 14:37 ` Alexey Gladkov
  2026-01-26 22:50 ` Paul Moore
  1 sibling, 0 replies; 6+ messages in thread
From: Alexey Gladkov @ 2026-01-22 14:37 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Andrew Morton, Eric W . Biederman, linux-kernel,
	linux-security-module, selinux

On Thu, Jan 22, 2026 at 03:13:03PM +0100, Ondrej Mosnacek wrote:
> The IPC sysctls implement the ctl_table_root::permissions hook and
> they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> capability, which is being checked regardless of whether any access is
> actually denied or not, so if an LSM denies the capability, an audit
> record may be logged even when access is in fact granted.
> 
> It wouldn't be viable to restructure the sysctl permission logic to only
> check the capability when the access would be actually denied if it's
> not granted. Thus, do the same as in net_ctl_permissions()
> (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> so that the check never emits an audit record.
> 
> Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Some permission hooks already use ns_capable_noaudit.
For example, pid_table_root_permissions.
I think it makes sense here also.

Acked-by: Alexey Gladkov <legion@kernel.org>

> ---
>  include/linux/capability.h | 6 ++++++
>  ipc/ipc_sysctl.c           | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 1fb08922552c7..37db92b3d6f89 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -203,6 +203,12 @@ static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns)
>  		ns_capable(ns, CAP_SYS_ADMIN);
>  }
>  
> +static inline bool checkpoint_restore_ns_capable_noaudit(struct user_namespace *ns)
> +{
> +	return ns_capable_noaudit(ns, CAP_CHECKPOINT_RESTORE) ||
> +		ns_capable_noaudit(ns, CAP_SYS_ADMIN);
> +}
> +
>  /* audit system wants to get cap info from files as well */
>  int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
>  			   const struct dentry *dentry,
> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> index 15b17e86e198c..9b087ebeb643b 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -214,7 +214,7 @@ static int ipc_permissions(struct ctl_table_header *head, const struct ctl_table
>  	if (((table->data == &ns->ids[IPC_SEM_IDS].next_id) ||
>  	     (table->data == &ns->ids[IPC_MSG_IDS].next_id) ||
>  	     (table->data == &ns->ids[IPC_SHM_IDS].next_id)) &&
> -	    checkpoint_restore_ns_capable(ns->user_ns))
> +	    checkpoint_restore_ns_capable_noaudit(ns->user_ns))
>  		mode = 0666;
>  	else
>  #endif
> -- 
> 2.52.0
> 

-- 
Rgrds, legion


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
  2026-01-22 14:13 [PATCH] ipc: don't audit capability check in ipc_permissions() Ondrej Mosnacek
  2026-01-22 14:37 ` Alexey Gladkov
@ 2026-01-26 22:50 ` Paul Moore
  2026-01-27  2:01   ` Serge E. Hallyn
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Moore @ 2026-01-26 22:50 UTC (permalink / raw)
  To: Ondrej Mosnacek, Serge Hallyn
  Cc: Andrew Morton, Eric W . Biederman, Alexey Gladkov, linux-kernel,
	linux-security-module, selinux

On Thu, Jan 22, 2026 at 9:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> The IPC sysctls implement the ctl_table_root::permissions hook and
> they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> capability, which is being checked regardless of whether any access is
> actually denied or not, so if an LSM denies the capability, an audit
> record may be logged even when access is in fact granted.
>
> It wouldn't be viable to restructure the sysctl permission logic to only
> check the capability when the access would be actually denied if it's
> not granted. Thus, do the same as in net_ctl_permissions()
> (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> so that the check never emits an audit record.
>
> Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  include/linux/capability.h | 6 ++++++
>  ipc/ipc_sysctl.c           | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)

This change seems reasonable to me, but I would make sure Serge has a
chance to review/ACK this patch as it has a capability impact.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
  2026-01-26 22:50 ` Paul Moore
@ 2026-01-27  2:01   ` Serge E. Hallyn
  2026-01-27 22:06     ` Paul Moore
  0 siblings, 1 reply; 6+ messages in thread
From: Serge E. Hallyn @ 2026-01-27  2:01 UTC (permalink / raw)
  To: Paul Moore
  Cc: Ondrej Mosnacek, Serge Hallyn, Andrew Morton, Eric W . Biederman,
	Alexey Gladkov, linux-kernel, linux-security-module, selinux

On Mon, Jan 26, 2026 at 05:50:12PM -0500, Paul Moore wrote:
> On Thu, Jan 22, 2026 at 9:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > The IPC sysctls implement the ctl_table_root::permissions hook and
> > they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> > capability, which is being checked regardless of whether any access is
> > actually denied or not, so if an LSM denies the capability, an audit
> > record may be logged even when access is in fact granted.
> >
> > It wouldn't be viable to restructure the sysctl permission logic to only
> > check the capability when the access would be actually denied if it's
> > not granted. Thus, do the same as in net_ctl_permissions()
> > (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> > so that the check never emits an audit record.
> >
> > Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  include/linux/capability.h | 6 ++++++
> >  ipc/ipc_sysctl.c           | 2 +-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> This change seems reasonable to me, but I would make sure Serge has a
> chance to review/ACK this patch as it has a capability impact.

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

Thanks - looks good to me.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
  2026-01-27  2:01   ` Serge E. Hallyn
@ 2026-01-27 22:06     ` Paul Moore
  2026-01-28  3:26       ` Serge E. Hallyn
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Moore @ 2026-01-27 22:06 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Ondrej Mosnacek, Andrew Morton, Eric W . Biederman,
	Alexey Gladkov, linux-kernel, linux-security-module, selinux

On Mon, Jan 26, 2026 at 9:01 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Mon, Jan 26, 2026 at 05:50:12PM -0500, Paul Moore wrote:
> > On Thu, Jan 22, 2026 at 9:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > >
> > > The IPC sysctls implement the ctl_table_root::permissions hook and
> > > they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> > > capability, which is being checked regardless of whether any access is
> > > actually denied or not, so if an LSM denies the capability, an audit
> > > record may be logged even when access is in fact granted.
> > >
> > > It wouldn't be viable to restructure the sysctl permission logic to only
> > > check the capability when the access would be actually denied if it's
> > > not granted. Thus, do the same as in net_ctl_permissions()
> > > (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> > > so that the check never emits an audit record.
> > >
> > > Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > ---
> > >  include/linux/capability.h | 6 ++++++
> > >  ipc/ipc_sysctl.c           | 2 +-
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > This change seems reasonable to me, but I would make sure Serge has a
> > chance to review/ACK this patch as it has a capability impact.
>
> Acked-by: Serge Hallyn <serge@hallyn.com>
>
> Thanks - looks good to me.

I don't see a dedicated IPC maintainer/tree, do you want to take this
via the capabilities tree Serge?

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
  2026-01-27 22:06     ` Paul Moore
@ 2026-01-28  3:26       ` Serge E. Hallyn
  0 siblings, 0 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2026-01-28  3:26 UTC (permalink / raw)
  To: Paul Moore
  Cc: Serge E. Hallyn, Ondrej Mosnacek, Andrew Morton,
	Eric W . Biederman, Alexey Gladkov, linux-kernel,
	linux-security-module, selinux

On Tue, Jan 27, 2026 at 05:06:47PM -0500, Paul Moore wrote:
> On Mon, Jan 26, 2026 at 9:01 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Mon, Jan 26, 2026 at 05:50:12PM -0500, Paul Moore wrote:
> > > On Thu, Jan 22, 2026 at 9:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > >
> > > > The IPC sysctls implement the ctl_table_root::permissions hook and
> > > > they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> > > > capability, which is being checked regardless of whether any access is
> > > > actually denied or not, so if an LSM denies the capability, an audit
> > > > record may be logged even when access is in fact granted.
> > > >
> > > > It wouldn't be viable to restructure the sysctl permission logic to only
> > > > check the capability when the access would be actually denied if it's
> > > > not granted. Thus, do the same as in net_ctl_permissions()
> > > > (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> > > > so that the check never emits an audit record.
> > > >
> > > > Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > ---
> > > >  include/linux/capability.h | 6 ++++++
> > > >  ipc/ipc_sysctl.c           | 2 +-
> > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > This change seems reasonable to me, but I would make sure Serge has a
> > > chance to review/ACK this patch as it has a capability impact.
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> >
> > Thanks - looks good to me.
> 
> I don't see a dedicated IPC maintainer/tree, do you want to take this
> via the capabilities tree Serge?

Will do.

-serge

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-01-28  3:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 14:13 [PATCH] ipc: don't audit capability check in ipc_permissions() Ondrej Mosnacek
2026-01-22 14:37 ` Alexey Gladkov
2026-01-26 22:50 ` Paul Moore
2026-01-27  2:01   ` Serge E. Hallyn
2026-01-27 22:06     ` Paul Moore
2026-01-28  3:26       ` Serge E. Hallyn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox