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 10/11] ipc/msg: Fix msgctl(..., IPC_STAT, ...) between pid namespaces
Date: Fri, 23 Mar 2018 14:16:13 -0500 [thread overview]
Message-ID: <20180323191614.32489-10-ebiederm@xmission.com> (raw)
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
Today msg_lspid and msg_lrpid are remembered in the pid namespace of
the creator and the processes that last send or received a sysvipc
message. If you have processes in multiple pid namespaces that is
just wrong. The process ids reported will not make the least bit of
sense.
This fix is slightly more susceptible to a performance problem than
the related fix for System V shared memory. By definition the pids
are updated by msgsnd and msgrcv, the fast path of System V message
queues. The only concern over the previous implementation is the
incrementing and decrementing of the pid reference count. As that is
the only difference and multiple updates by of the task_tgid by
threads in the same process have been shown in af_unix sockets to
create a cache line ping-pong between cpus of the same processor.
In this case I don't expect cache lines holding pid reference counts
to ping pong between cpus. As senders and receivers update different
pids there is a natural separation there. Further if multiple threads
of the same process either send or receive messages the pid will be
updated to the same value and ipc_update_pid will avoid the reference
count update.
Which means in the common case I expect msg_lspid and msg_lrpid to
remain constant, and reference counts not to be updated when messages
are sent.
In rare cases it may be possible to trigger the issue which was
observed for af_unix sockets, but it will require multiple processes
with multiple threads to be either sending or receiving messages. It
just does not feel likely that anyone would do that in practice.
This change updates msgctl(..., IPC_STAT, ...) to return msg_lspid and
msg_lrpid in the pid namespace of the process calling stat.
This change also updates cat /proc/sysvipc/msg to return print msg_lspid
and msg_lrpid in the pid namespace of the process that opened the proc
file.
Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
ipc/msg.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/ipc/msg.c b/ipc/msg.c
index af5a963306c4..825ad585a6ff 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -52,8 +52,8 @@ struct msg_queue {
unsigned long q_cbytes; /* current number of bytes on queue */
unsigned long q_qnum; /* number of messages in queue */
unsigned long q_qbytes; /* max number of bytes on queue */
- pid_t q_lspid; /* pid of last msgsnd */
- pid_t q_lrpid; /* last receive pid */
+ struct pid *q_lspid; /* pid of last msgsnd */
+ struct pid *q_lrpid; /* last receive pid */
struct list_head q_messages;
struct list_head q_receivers;
@@ -154,7 +154,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
msq->q_ctime = ktime_get_real_seconds();
msq->q_cbytes = msq->q_qnum = 0;
msq->q_qbytes = ns->msg_ctlmnb;
- msq->q_lspid = msq->q_lrpid = 0;
+ msq->q_lspid = msq->q_lrpid = NULL;
INIT_LIST_HEAD(&msq->q_messages);
INIT_LIST_HEAD(&msq->q_receivers);
INIT_LIST_HEAD(&msq->q_senders);
@@ -267,6 +267,8 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
free_msg(msg);
}
atomic_sub(msq->q_cbytes, &ns->msg_bytes);
+ ipc_update_pid(&msq->q_lspid, NULL);
+ ipc_update_pid(&msq->q_lrpid, NULL);
ipc_rcu_putref(&msq->q_perm, msg_rcu_free);
}
@@ -536,8 +538,8 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
p->msg_cbytes = msq->q_cbytes;
p->msg_qnum = msq->q_qnum;
p->msg_qbytes = msq->q_qbytes;
- p->msg_lspid = msq->q_lspid;
- p->msg_lrpid = msq->q_lrpid;
+ p->msg_lspid = pid_vnr(msq->q_lspid);
+ p->msg_lrpid = pid_vnr(msq->q_lrpid);
ipc_unlock_object(&msq->q_perm);
rcu_read_unlock();
@@ -741,7 +743,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg,
wake_q_add(wake_q, msr->r_tsk);
WRITE_ONCE(msr->r_msg, ERR_PTR(-E2BIG));
} else {
- msq->q_lrpid = task_pid_vnr(msr->r_tsk);
+ ipc_update_pid(&msq->q_lrpid, task_pid(msr->r_tsk));
msq->q_rtime = get_seconds();
wake_q_add(wake_q, msr->r_tsk);
@@ -842,7 +844,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext,
}
- msq->q_lspid = task_tgid_vnr(current);
+ ipc_update_pid(&msq->q_lspid, task_tgid(current));
msq->q_stime = get_seconds();
if (!pipelined_send(msq, msg, &wake_q)) {
@@ -1060,7 +1062,7 @@ static long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, in
list_del(&msg->m_list);
msq->q_qnum--;
msq->q_rtime = get_seconds();
- msq->q_lrpid = task_tgid_vnr(current);
+ ipc_update_pid(&msq->q_lrpid, task_tgid(current));
msq->q_cbytes -= msg->m_ts;
atomic_sub(msg->m_ts, &ns->msg_bytes);
atomic_dec(&ns->msg_hdrs);
@@ -1202,6 +1204,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
#ifdef CONFIG_PROC_FS
static int sysvipc_msg_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 msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm);
@@ -1213,8 +1216,8 @@ static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
msq->q_perm.mode,
msq->q_cbytes,
msq->q_qnum,
- msq->q_lspid,
- msq->q_lrpid,
+ pid_nr_ns(msq->q_lspid, pid_ns),
+ pid_nr_ns(msq->q_lrpid, pid_ns),
from_kuid_munged(user_ns, msq->q_perm.uid),
from_kgid_munged(user_ns, msq->q_perm.gid),
from_kuid_munged(user_ns, msq->q_perm.cuid),
--
2.14.1
next prev 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 ` [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces Eric W. Biederman
2018-03-23 21:17 ` 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 ` Eric W. Biederman [this message]
2018-03-23 21:21 ` [REVIEW][PATCH 10/11] ipc/msg: Fix msgctl(..., " 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-10-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®