mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Linux Containers <containers@lists.linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	khlebnikov@yandex-team.ru, prakash.sangappa@oracle.com,
	luto@kernel.org, akpm@linux-foundation.org, oleg@redhat.com,
	serge.hallyn@ubuntu.com, esyr@redhat.com, jannh@google.com,
	linux-security-module@vger.kernel.org,
	Pavel Emelyanov <xemul@openvz.org>,
	Nagarathnam Muthusamy <nagarathnam.muthusamy@oracle.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces.
Date: Fri, 23 Mar 2018 14:16:12 -0500	[thread overview]
Message-ID: <20180323191614.32489-9-ebiederm@xmission.com> (raw)
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>

Today shm_cpid and shm_lpid are remembered in the pid namespace of the
creator and the processes that last touched a sysvipc shared memory
segment.   If you have processes in multiple pid namespaces that
is just wrong, and I don't know how this has been over-looked for
so long.

As only creation and shared memory attach and shared memory detach
update the pids I do not expect there to be a repeat of the issues
when struct pid was attached to each af_unix skb, which in some
notable cases cut the performance in half.  The problem was threads of
the same process updating same struct pid from different cpus causing
the cache line to be highly contended and bounce between cpus.

As creation, attach, and detach are expected to be rare operations for
sysvipc shared memory segments I do not expect that kind of cache line
ping pong to cause probems.  In addition because the pid is at a fixed
location in the structure instead of being dynamic on a skb, the
reference count of the pid does not need to be updated on each
operation if the pid is the same.  This ability to simply skip the pid
reference count changes if the pid is unchanging further reduces the
likelihood of the a cache line holding a pid reference count
ping-ponging between cpus.

Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 ipc/shm.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 0565669ebe5c..932b7e411c6c 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -57,8 +57,8 @@ struct shmid_kernel /* private to the kernel */
 	time64_t		shm_atim;
 	time64_t		shm_dtim;
 	time64_t		shm_ctim;
-	pid_t			shm_cprid;
-	pid_t			shm_lprid;
+	struct pid		*shm_cprid;
+	struct pid		*shm_lprid;
 	struct user_struct	*mlock_user;
 
 	/* The task created the shm object.  NULL if the task is dead. */
@@ -226,7 +226,7 @@ static int __shm_open(struct vm_area_struct *vma)
 		return PTR_ERR(shp);
 
 	shp->shm_atim = ktime_get_real_seconds();
-	shp->shm_lprid = task_tgid_vnr(current);
+	ipc_update_pid(&shp->shm_lprid, task_tgid(current));
 	shp->shm_nattch++;
 	shm_unlock(shp);
 	return 0;
@@ -267,6 +267,8 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
 		user_shm_unlock(i_size_read(file_inode(shm_file)),
 				shp->mlock_user);
 	fput(shm_file);
+	ipc_update_pid(&shp->shm_cprid, NULL);
+	ipc_update_pid(&shp->shm_lprid, NULL);
 	ipc_rcu_putref(&shp->shm_perm, shm_rcu_free);
 }
 
@@ -311,7 +313,7 @@ static void shm_close(struct vm_area_struct *vma)
 	if (WARN_ON_ONCE(IS_ERR(shp)))
 		goto done; /* no-op */
 
-	shp->shm_lprid = task_tgid_vnr(current);
+	ipc_update_pid(&shp->shm_lprid, task_tgid(current));
 	shp->shm_dtim = ktime_get_real_seconds();
 	shp->shm_nattch--;
 	if (shm_may_destroy(ns, shp))
@@ -614,8 +616,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	if (IS_ERR(file))
 		goto no_file;
 
-	shp->shm_cprid = task_tgid_vnr(current);
-	shp->shm_lprid = 0;
+	shp->shm_cprid = get_pid(task_tgid(current));
+	shp->shm_lprid = NULL;
 	shp->shm_atim = shp->shm_dtim = 0;
 	shp->shm_ctim = ktime_get_real_seconds();
 	shp->shm_segsz = size;
@@ -648,6 +650,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 		user_shm_unlock(size, shp->mlock_user);
 	fput(file);
 no_file:
+	ipc_update_pid(&shp->shm_cprid, NULL);
+	ipc_update_pid(&shp->shm_lprid, NULL);
 	call_rcu(&shp->shm_perm.rcu, shm_rcu_free);
 	return error;
 }
@@ -970,8 +974,8 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
 	tbuf->shm_atime	= shp->shm_atim;
 	tbuf->shm_dtime	= shp->shm_dtim;
 	tbuf->shm_ctime	= shp->shm_ctim;
-	tbuf->shm_cpid	= shp->shm_cprid;
-	tbuf->shm_lpid	= shp->shm_lprid;
+	tbuf->shm_cpid	= pid_vnr(shp->shm_cprid);
+	tbuf->shm_lpid	= pid_vnr(shp->shm_lprid);
 	tbuf->shm_nattch = shp->shm_nattch;
 
 	ipc_unlock_object(&shp->shm_perm);
@@ -1605,6 +1609,7 @@ SYSCALL_DEFINE1(shmdt, char __user *, shmaddr)
 #ifdef CONFIG_PROC_FS
 static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
 {
+	struct pid_namespace *pid_ns = ipc_seq_pid_ns(s);
 	struct user_namespace *user_ns = seq_user_ns(s);
 	struct kern_ipc_perm *ipcp = it;
 	struct shmid_kernel *shp;
@@ -1627,8 +1632,8 @@ static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
 		   shp->shm_perm.id,
 		   shp->shm_perm.mode,
 		   shp->shm_segsz,
-		   shp->shm_cprid,
-		   shp->shm_lprid,
+		   pid_nr_ns(shp->shm_cprid, pid_ns),
+		   pid_nr_ns(shp->shm_lprid, pid_ns),
 		   shp->shm_nattch,
 		   from_kuid_munged(user_ns, shp->shm_perm.uid),
 		   from_kgid_munged(user_ns, shp->shm_perm.gid),
-- 
2.14.1


  parent reply	other threads:[~2018-03-23 19:18 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-12 17:18 [RESEND RFC] translate_pid API nagarathnam.muthusamy
2018-03-13 20:47 ` Jann Horn
2018-03-13 21:20   ` Nagarathnam Muthusamy
2018-03-13 21:28     ` Jann Horn
2018-03-13 21:44       ` Nagarathnam Muthusamy
2018-03-13 22:00         ` Jann Horn
2018-03-13 22:45           ` Nagarathnam Muthusamy
2018-03-13 23:10             ` Jann Horn
2018-03-13 23:52               ` Nagarathnam Muthusamy
2018-03-14  3:29 ` Eric W. Biederman
2018-03-14 21:22   ` Nagarathnam Muthusamy
2018-03-14 22:03     ` Eric W. Biederman
2018-03-20 20:14       ` Nagarathnam Muthusamy
2018-03-21  0:33         ` Eric W. Biederman
2018-03-23 19:11           ` [REVIEW][PATCH 00/11] ipc: Fixing the pid namespace support Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 01/11] sem/security: Pass kern_ipc_perm not sem_array into the sem security hooks Eric W. Biederman
2018-03-23 21:46               ` Casey Schaufler
2018-03-28 23:20                 ` Davidlohr Bueso
2018-03-23 19:16             ` [REVIEW][PATCH 02/11] shm/security: Pass kern_ipc_perm not shmid_kernel into the shm " Eric W. Biederman
2018-03-23 21:54               ` Casey Schaufler
2018-03-23 19:16             ` [REVIEW][PATCH 03/11] msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue " Eric W. Biederman
2018-03-23 21:55               ` Casey Schaufler
2018-03-24  5:37                 ` Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 04/11] sem: Move struct sem and struct sem_array into ipc/sem.c Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 05/11] shm: Move struct shmid_kernel into ipc/shm.c Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 06/11] msg: Move struct msg_queue into ipc/msg.c Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 07/11] ipc: Move IPCMNI from include/ipc.h into ipc/util.h Eric W. Biederman
2018-03-23 19:16             ` [REVIEW][PATCH 08/11] ipc/util: Helpers for making the sysvipc operations pid namespace aware Eric W. Biederman
2018-03-23 19:16             ` Eric W. Biederman [this message]
2018-03-23 21:17               ` [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces NAGARATHNAM MUTHUSAMY
2018-03-23 21:33                 ` Eric W. Biederman
2018-03-23 21:41                   ` NAGARATHNAM MUTHUSAMY
2018-03-28 23:04                     ` Eric W. Biederman
2018-03-28 23:18                       ` Nagarathnam Muthusamy
2018-03-23 19:16             ` [REVIEW][PATCH 10/11] ipc/msg: Fix msgctl(..., " Eric W. Biederman
2018-03-23 21:21               ` NAGARATHNAM MUTHUSAMY
2018-03-23 19:16             ` [REVIEW][PATCH 11/11] ipc/sem: Fix semctl(..., GETPID, " Eric W. Biederman
2018-03-29  0:52               ` Davidlohr Bueso
2018-03-30 19:09                 ` Davidlohr Bueso
2018-03-30 20:12                   ` Eric W. Biederman
2018-03-30 20:45                     ` Davidlohr Bueso
2018-04-02 11:11                   ` Manfred Spraul
2018-03-24  5:40             ` [REVIEW][PATCH 12/11] ipc: Directly call the security hook in ipc_ops.associate Eric W. Biederman
2018-03-28 23:40               ` Davidlohr Bueso
2018-03-31  2:13               ` James Morris
2018-03-24  5:42             ` [REVIEW][PATCH 13/11] ipc/smack: Tidy up from the change in type of the ipc security hooks Eric W. Biederman
2018-03-25  0:05               ` Casey Schaufler
2018-03-28 23:38                 ` Davidlohr Bueso
2018-03-28 23:57               ` Davidlohr Bueso
2018-03-29  1:12             ` [REVIEW][PATCH 00/11] ipc: Fixing the pid namespace support Davidlohr Bueso
2018-03-29 18:42               ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180323191614.32489-9-ebiederm@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=esyr@redhat.com \
    --cc=jannh@google.com \
    --cc=khlebnikov@yandex-team.ru \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=nagarathnam.muthusamy@oracle.com \
    --cc=oleg@redhat.com \
    --cc=prakash.sangappa@oracle.com \
    --cc=serge.hallyn@ubuntu.com \
    --cc=xemul@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®