From d8c6e8543294428426578d74dc7aaf121e762d58 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 22 Mar 2018 21:22:26 -0500 Subject: msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue security hooks All of the implementations of security hooks that take msg_queue only access q_perm the struct kern_ipc_perm member. This means the dependencies of the msg_queue security hooks can be simplified by passing the kern_ipc_perm member of msg_queue. Making this change will allow struct msg_queue to become private to ipc/msg.c. Signed-off-by: "Eric W. Biederman" --- ipc/msg.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 0dcc6699dc53..cdfab0825fce 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -101,7 +101,7 @@ static void msg_rcu_free(struct rcu_head *head) struct kern_ipc_perm *p = container_of(head, struct kern_ipc_perm, rcu); struct msg_queue *msq = container_of(p, struct msg_queue, q_perm); - security_msg_queue_free(msq); + security_msg_queue_free(&msq->q_perm); kvfree(msq); } @@ -127,7 +127,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) msq->q_perm.key = key; msq->q_perm.security = NULL; - retval = security_msg_queue_alloc(msq); + retval = security_msg_queue_alloc(&msq->q_perm); if (retval) { kvfree(msq); return retval; @@ -258,9 +258,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp) */ static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg) { - struct msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm); - - return security_msg_queue_associate(msq, msgflg); + return security_msg_queue_associate(ipcp, msgflg); } SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg) @@ -380,7 +378,7 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd, msq = container_of(ipcp, struct msg_queue, q_perm); - err = security_msg_queue_msgctl(msq, cmd); + err = security_msg_queue_msgctl(&msq->q_perm, cmd); if (err) goto out_unlock1; @@ -502,7 +500,7 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid, if (ipcperms(ns, &msq->q_perm, S_IRUGO)) goto out_unlock; - err = security_msg_queue_msgctl(msq, cmd); + err = security_msg_queue_msgctl(&msq->q_perm, cmd); if (err) goto out_unlock; @@ -718,7 +716,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg, list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) { if (testmsg(msg, msr->r_msgtype, msr->r_mode) && - !security_msg_queue_msgrcv(msq, msg, msr->r_tsk, + !security_msg_queue_msgrcv(&msq->q_perm, msg, msr->r_tsk, msr->r_msgtype, msr->r_mode)) { list_del(&msr->r_list); @@ -784,7 +782,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext, goto out_unlock0; } - err = security_msg_queue_msgsnd(msq, msg, msgflg); + err = security_msg_queue_msgsnd(&msq->q_perm, msg, msgflg); if (err) goto out_unlock0; @@ -960,7 +958,7 @@ static struct msg_msg *find_msg(struct msg_queue *msq, long *msgtyp, int mode) list_for_each_entry(msg, &msq->q_messages, m_list) { if (testmsg(msg, *msgtyp, mode) && - !security_msg_queue_msgrcv(msq, msg, current, + !security_msg_queue_msgrcv(&msq->q_perm, msg, current, *msgtyp, mode)) { if (mode == SEARCH_LESSEQUAL && msg->m_type != 1) { *msgtyp = msg->m_type - 1; -- cgit v1.2.3-59-g8ed1b From 34b56df922b10ac2876f268c522951785bf333fd Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 22 Mar 2018 21:37:34 -0500 Subject: msg: Move struct msg_queue into ipc/msg.c All of the users are now in ipc/msg.c so make the definition local to that file to make code maintenance easier. AKA to prevent rebuilding the entire kernel when struct msg_queue changes. Signed-off-by: "Eric W. Biederman" --- include/linux/msg.h | 18 ------------------ ipc/msg.c | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 18 deletions(-) (limited to 'ipc/msg.c') diff --git a/include/linux/msg.h b/include/linux/msg.h index 0a7eefeee0d1..9a972a296b95 100644 --- a/include/linux/msg.h +++ b/include/linux/msg.h @@ -3,7 +3,6 @@ #define _LINUX_MSG_H #include -#include #include /* one msg_msg structure for each message */ @@ -16,21 +15,4 @@ struct msg_msg { /* the actual message follows immediately */ }; -/* one msq_queue structure for each present queue on the system */ -struct msg_queue { - struct kern_ipc_perm q_perm; - time64_t q_stime; /* last msgsnd time */ - time64_t q_rtime; /* last msgrcv time */ - time64_t q_ctime; /* last change time */ - 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 list_head q_messages; - struct list_head q_receivers; - struct list_head q_senders; -} __randomize_layout; - #endif /* _LINUX_MSG_H */ diff --git a/ipc/msg.c b/ipc/msg.c index cdfab0825fce..af5a963306c4 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -43,6 +43,23 @@ #include #include "util.h" +/* one msq_queue structure for each present queue on the system */ +struct msg_queue { + struct kern_ipc_perm q_perm; + time64_t q_stime; /* last msgsnd time */ + time64_t q_rtime; /* last msgrcv time */ + time64_t q_ctime; /* last change time */ + 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 list_head q_messages; + struct list_head q_receivers; + struct list_head q_senders; +} __randomize_layout; + /* one msg_receiver structure for each sleeping receiver */ struct msg_receiver { struct list_head r_list; -- cgit v1.2.3-59-g8ed1b From 39a4940eaa185910bb802ca9829c12268fd2c855 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 23 Mar 2018 00:42:21 -0500 Subject: ipc/msg: Fix msgctl(..., IPC_STAT, ...) between pid namespaces 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") Reviewed-by: Nagarathnam Muthusamy Signed-off-by: "Eric W. Biederman" --- ipc/msg.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'ipc/msg.c') 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), -- cgit v1.2.3-59-g8ed1b From 50ab44b1c5d1b13305ce8acb74c8e50e0dcbaedc Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 23 Mar 2018 23:41:55 -0500 Subject: ipc: Directly call the security hook in ipc_ops.associate After the last round of cleanups the shm, sem, and msg associate operations just became trivial wrappers around the appropriate security method. Simplify things further by just calling the security method directly. Signed-off-by: "Eric W. Biederman" --- ipc/msg.c | 10 +--------- ipc/sem.c | 10 +--------- ipc/shm.c | 10 +--------- 3 files changed, 3 insertions(+), 27 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 825ad585a6ff..d667dd8e97ab 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -272,20 +272,12 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp) ipc_rcu_putref(&msq->q_perm, msg_rcu_free); } -/* - * Called with msg_ids.rwsem and ipcp locked. - */ -static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg) -{ - return security_msg_queue_associate(ipcp, msgflg); -} - SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg) { struct ipc_namespace *ns; static const struct ipc_ops msg_ops = { .getnew = newque, - .associate = msg_security, + .associate = security_msg_queue_associate, }; struct ipc_params msg_params; diff --git a/ipc/sem.c b/ipc/sem.c index 47b263960524..09d54af076a4 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -564,14 +564,6 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params) } -/* - * Called with sem_ids.rwsem and ipcp locked. - */ -static inline int sem_security(struct kern_ipc_perm *ipcp, int semflg) -{ - return security_sem_associate(ipcp, semflg); -} - /* * Called with sem_ids.rwsem and ipcp locked. */ @@ -592,7 +584,7 @@ SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg) struct ipc_namespace *ns; static const struct ipc_ops sem_ops = { .getnew = newary, - .associate = sem_security, + .associate = security_sem_associate, .more_checks = sem_more_checks, }; struct ipc_params sem_params; diff --git a/ipc/shm.c b/ipc/shm.c index 932b7e411c6c..018db3d0e70e 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -656,14 +656,6 @@ no_file: return error; } -/* - * Called with shm_ids.rwsem and ipcp locked. - */ -static inline int shm_security(struct kern_ipc_perm *ipcp, int shmflg) -{ - return security_shm_associate(ipcp, shmflg); -} - /* * Called with shm_ids.rwsem and ipcp locked. */ @@ -684,7 +676,7 @@ SYSCALL_DEFINE3(shmget, key_t, key, size_t, size, int, shmflg) struct ipc_namespace *ns; static const struct ipc_ops shm_ops = { .getnew = newseg, - .associate = shm_security, + .associate = security_shm_associate, .more_checks = shm_more_checks, }; struct ipc_params shm_params; -- cgit v1.2.3-59-g8ed1b