All of lore.kernel.org
 help / color / mirror / Atom feed
* + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree
@ 2021-09-16 17:51 akpm
  2021-09-17 18:14 ` +ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Davidlohr Bueso
  2021-09-18 14:54 ` + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Manfred Spraul
  0 siblings, 2 replies; 3+ messages in thread
From: akpm @ 2021-09-16 17:51 UTC (permalink / raw)
  To: dave, ebiederm, manfred, mclapinski, mm-commits


The patch titled
     Subject: ipc: check checkpoint_restore_ns_capable() to modify C/R proc files
has been added to the -mm tree.  Its filename is
     ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch

This patch should soon appear at
    https://ozlabs.org/~akpm/mmots/broken-out/ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch
and later at
    https://ozlabs.org/~akpm/mmotm/broken-out/ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Michal Clapinski <mclapinski@google.com>
Subject: ipc: check checkpoint_restore_ns_capable() to modify C/R proc files

This commit removes the requirement to be root to modify sem_next_id,
msg_next_id and shm_next_id and checks checkpoint_restore_ns_capable
instead.

Since those files are specific to the IPC namespace, there is no reason
they should require root privileges.  This is similar to ns_last_pid,
which also only checks checkpoint_restore_ns_capable.

Link: https://lkml.kernel.org/r/20210916163717.3179496-1-mclapinski@google.com
Signed-off-by: Michal Clapinski <mclapinski@google.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 ipc/ipc_sysctl.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

--- a/ipc/ipc_sysctl.c~ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files
+++ a/ipc/ipc_sysctl.c
@@ -104,6 +104,19 @@ static int proc_ipc_sem_dointvec(struct
 	return ret;
 }
 
+#ifdef CONFIG_CHECKPOINT_RESTORE
+static int proc_ipc_dointvec_minmax_checkpoint_restore(struct ctl_table *table,
+		int write, void *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct user_namespace *user_ns = current->nsproxy->ipc_ns->user_ns;
+
+	if (write && !checkpoint_restore_ns_capable(user_ns))
+		return -EPERM;
+
+	return proc_ipc_dointvec_minmax(table, write, buffer, lenp, ppos);
+}
+#endif
+
 #else
 #define proc_ipc_doulongvec_minmax NULL
 #define proc_ipc_dointvec	   NULL
@@ -111,6 +124,9 @@ static int proc_ipc_sem_dointvec(struct
 #define proc_ipc_dointvec_minmax_orphans   NULL
 #define proc_ipc_auto_msgmni	   NULL
 #define proc_ipc_sem_dointvec	   NULL
+#ifdef CONFIG_CHECKPOINT_RESTORE
+#define proc_ipc_dointvec_minmax_checkpoint_restore	NULL
+#endif	/* CONFIG_CHECKPOINT_RESTORE */
 #endif
 
 int ipc_mni = IPCMNI;
@@ -198,8 +214,8 @@ static struct ctl_table ipc_kern_table[]
 		.procname	= "sem_next_id",
 		.data		= &init_ipc_ns.ids[IPC_SEM_IDS].next_id,
 		.maxlen		= sizeof(init_ipc_ns.ids[IPC_SEM_IDS].next_id),
-		.mode		= 0644,
-		.proc_handler	= proc_ipc_dointvec_minmax,
+		.mode		= 0666,
+		.proc_handler	= proc_ipc_dointvec_minmax_checkpoint_restore,
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_INT_MAX,
 	},
@@ -207,8 +223,8 @@ static struct ctl_table ipc_kern_table[]
 		.procname	= "msg_next_id",
 		.data		= &init_ipc_ns.ids[IPC_MSG_IDS].next_id,
 		.maxlen		= sizeof(init_ipc_ns.ids[IPC_MSG_IDS].next_id),
-		.mode		= 0644,
-		.proc_handler	= proc_ipc_dointvec_minmax,
+		.mode		= 0666,
+		.proc_handler	= proc_ipc_dointvec_minmax_checkpoint_restore,
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_INT_MAX,
 	},
@@ -216,8 +232,8 @@ static struct ctl_table ipc_kern_table[]
 		.procname	= "shm_next_id",
 		.data		= &init_ipc_ns.ids[IPC_SHM_IDS].next_id,
 		.maxlen		= sizeof(init_ipc_ns.ids[IPC_SHM_IDS].next_id),
-		.mode		= 0644,
-		.proc_handler	= proc_ipc_dointvec_minmax,
+		.mode		= 0666,
+		.proc_handler	= proc_ipc_dointvec_minmax_checkpoint_restore,
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_INT_MAX,
 	},
_

Patches currently in -mm which might be from mclapinski@google.com are

ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch


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

* Re: +ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree
  2021-09-16 17:51 + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree akpm
@ 2021-09-17 18:14 ` Davidlohr Bueso
  2021-09-18 14:54 ` + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Manfred Spraul
  1 sibling, 0 replies; 3+ messages in thread
From: Davidlohr Bueso @ 2021-09-17 18:14 UTC (permalink / raw)
  To: akpm; +Cc: ebiederm, manfred, mclapinski, mm-commits

On Thu, 16 Sep 2021, akpm@linux-foundation.org wrote:

>Subject: ipc: check checkpoint_restore_ns_capable() to modify C/R proc files
>
>This commit removes the requirement to be root to modify sem_next_id,
>msg_next_id and shm_next_id and checks checkpoint_restore_ns_capable
>instead.

After reading 124ea650d3072b005457faed69909221c2905a1f this makes sense
to me.

>
>Since those files are specific to the IPC namespace, there is no reason
>they should require root privileges.  This is similar to ns_last_pid,
>which also only checks checkpoint_restore_ns_capable.
>
>Link: https://lkml.kernel.org/r/20210916163717.3179496-1-mclapinski@google.com
>Signed-off-by: Michal Clapinski <mclapinski@google.com>
>Cc: "Eric W. Biederman" <ebiederm@xmission.com>
>Cc: Davidlohr Bueso <dave@stgolabs.net>
>Cc: Manfred Spraul <manfred@colorfullife.com>
>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Reviewed-by: Davidlohr Bueso <dbueso@suse.de>

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

* Re: + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree
  2021-09-16 17:51 + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree akpm
  2021-09-17 18:14 ` +ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Davidlohr Bueso
@ 2021-09-18 14:54 ` Manfred Spraul
  1 sibling, 0 replies; 3+ messages in thread
From: Manfred Spraul @ 2021-09-18 14:54 UTC (permalink / raw)
  To: akpm, dave, ebiederm, mclapinski, mm-commits

Hi,

On 9/16/21 7:51 PM, akpm@linux-foundation.org wrote:
> The patch titled
>       Subject: ipc: check checkpoint_restore_ns_capable() to modify C/R proc files
> has been added to the -mm tree.  Its filename is
>       ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch
>
> ------------------------------------------------------
> From: Michal Clapinski <mclapinski@google.com>
> Subject: ipc: check checkpoint_restore_ns_capable() to modify C/R proc files
>
> This commit removes the requirement to be root to modify sem_next_id,
> msg_next_id and shm_next_id and checks checkpoint_restore_ns_capable
> instead.
>
> Since those files are specific to the IPC namespace, there is no reason
> they should require root privileges.  This is similar to ns_last_pid,
> which also only checks checkpoint_restore_ns_capable.
>
> Link: https://lkml.kernel.org/r/20210916163717.3179496-1-mclapinski@google.com
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: Manfred Spraul <manfred@colorfullife.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-By: Manfred Spraul <manfred@colorfullife.com>
> [...]
>   #else
>   #define proc_ipc_doulongvec_minmax NULL
>   #define proc_ipc_dointvec	   NULL
> @@ -111,6 +124,9 @@ static int proc_ipc_sem_dointvec(struct
>   #define proc_ipc_dointvec_minmax_orphans   NULL
>   #define proc_ipc_auto_msgmni	   NULL
>   #define proc_ipc_sem_dointvec	   NULL
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +#define proc_ipc_dointvec_minmax_checkpoint_restore	NULL
> +#endif	/* CONFIG_CHECKPOINT_RESTORE */
>   #endif

This fallback is actually not required:

 From what I see, the whole file is not compiled when sysctl is disabled.

--

     Manfred


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

end of thread, other threads:[~2021-09-18 14:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 17:51 + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch added to -mm tree akpm
2021-09-17 18:14 ` +ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Davidlohr Bueso
2021-09-18 14:54 ` + ipc-check-checkpoint_restore_ns_capable-to-modify-c-r-proc-files.patch " Manfred Spraul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.