From dba4cdd39e698d8dcdad0656825423052ac90ccd Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Wed, 12 Jul 2017 14:34:41 -0700 Subject: ipc: merge ipc_rcu and kern_ipc_perm ipc has two management structures that exist for every id: - struct kern_ipc_perm, it contains e.g. the permissions. - struct ipc_rcu, it contains the rcu head for rcu handling and the refcount. The patch merges both structures. As a bonus, we may save one cacheline, because both structures are cacheline aligned. In addition, it reduces the number of casts, instead most codepaths can use container_of. To simplify code, the ipc_rcu_alloc initializes the allocation to 0. [manfred@colorfullife.com: really include the memset() into ipc_alloc_rcu()] Link: http://lkml.kernel.org/r/564f8612-0601-b267-514f-a9f650ec9b32@colorfullife.com Link: http://lkml.kernel.org/r/20170525185107.12869-3-manfred@colorfullife.com Signed-off-by: Manfred Spraul Cc: Davidlohr Bueso Cc: Kees Cook Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 104926dc72be..0ed7dae7d4e8 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -97,8 +97,8 @@ static inline void msg_rmid(struct ipc_namespace *ns, struct msg_queue *s) static void msg_rcu_free(struct rcu_head *head) { - struct ipc_rcu *p = container_of(head, struct ipc_rcu, rcu); - struct msg_queue *msq = ipc_rcu_to_struct(p); + 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); ipc_rcu_free(head); @@ -118,7 +118,10 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) key_t key = params->key; int msgflg = params->flg; - msq = ipc_rcu_alloc(sizeof(*msq)); + BUILD_BUG_ON(offsetof(struct msg_queue, q_perm) != 0); + + msq = container_of(ipc_rcu_alloc(sizeof(*msq)), struct msg_queue, + q_perm); if (!msq) return -ENOMEM; @@ -128,7 +131,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) msq->q_perm.security = NULL; retval = security_msg_queue_alloc(msq); if (retval) { - ipc_rcu_putref(msq, ipc_rcu_free); + ipc_rcu_putref(&msq->q_perm, ipc_rcu_free); return retval; } @@ -144,7 +147,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) /* ipc_addid() locks msq upon success. */ id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni); if (id < 0) { - ipc_rcu_putref(msq, msg_rcu_free); + ipc_rcu_putref(&msq->q_perm, msg_rcu_free); return id; } @@ -249,7 +252,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp) free_msg(msg); } atomic_sub(msq->q_cbytes, &ns->msg_bytes); - ipc_rcu_putref(msq, msg_rcu_free); + ipc_rcu_putref(&msq->q_perm, msg_rcu_free); } /* @@ -688,7 +691,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext, /* enqueue the sender and prepare to block */ ss_add(msq, &s, msgsz); - if (!ipc_rcu_getref(msq)) { + if (!ipc_rcu_getref(&msq->q_perm)) { err = -EIDRM; goto out_unlock0; } @@ -700,7 +703,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext, rcu_read_lock(); ipc_lock_object(&msq->q_perm); - ipc_rcu_putref(msq, msg_rcu_free); + ipc_rcu_putref(&msq->q_perm, msg_rcu_free); /* raced with RMID? */ if (!ipc_valid_object(&msq->q_perm)) { err = -EIDRM; -- cgit v1.2.3 From 9ef5932f8a1134b9d93676ee26701b2be90c7a95 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 12 Jul 2017 14:34:56 -0700 Subject: ipc/msg: do not use ipc_rcu_free() Avoid using ipc_rcu_free, since it just re-finds the original structure pointer. For the pre-list-init failure path, there is no RCU needed, since it was just allocated. It can be directly freed. Link: http://lkml.kernel.org/r/20170525185107.12869-8-manfred@colorfullife.com Signed-off-by: Kees Cook Signed-off-by: Manfred Spraul Cc: Davidlohr Bueso Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 0ed7dae7d4e8..25d43e27ef12 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -95,13 +95,18 @@ static inline void msg_rmid(struct ipc_namespace *ns, struct msg_queue *s) ipc_rmid(&msg_ids(ns), &s->q_perm); } +static void __msg_free(struct msg_queue *msq) +{ + kvfree(msq); +} + 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); - ipc_rcu_free(head); + __msg_free(msq); } /** @@ -131,7 +136,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) msq->q_perm.security = NULL; retval = security_msg_queue_alloc(msq); if (retval) { - ipc_rcu_putref(&msq->q_perm, ipc_rcu_free); + __msg_free(msq); return retval; } -- cgit v1.2.3 From 52f908904e7e05b6300162faa48152df073be645 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 12 Jul 2017 14:35:07 -0700 Subject: ipc/msg: avoid ipc_rcu_alloc() Instead of using ipc_rcu_alloc() which only performs the refcount bump, open code it. This also allows for msg_queue structure layout to be randomized in the future. Link: http://lkml.kernel.org/r/20170525185107.12869-12-manfred@colorfullife.com Signed-off-by: Kees Cook Signed-off-by: Manfred Spraul Cc: Davidlohr Bueso Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 25d43e27ef12..10094a731b8e 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -109,6 +109,19 @@ static void msg_rcu_free(struct rcu_head *head) __msg_free(msq); } +static struct msg_queue *msg_alloc(void) +{ + struct msg_queue *msq; + + msq = kvmalloc(sizeof(*msq), GFP_KERNEL); + if (unlikely(!msq)) + return NULL; + + atomic_set(&msq->q_perm.refcount, 1); + + return msq; +} + /** * newque - Create a new msg queue * @ns: namespace @@ -123,10 +136,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) key_t key = params->key; int msgflg = params->flg; - BUILD_BUG_ON(offsetof(struct msg_queue, q_perm) != 0); - - msq = container_of(ipc_rcu_alloc(sizeof(*msq)), struct msg_queue, - q_perm); + msq = msg_alloc(); if (!msq) return -ENOMEM; -- cgit v1.2.3 From 51c23b7b7db52493d4fc869cec8c3e8fe27bfcd3 Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Wed, 12 Jul 2017 14:35:19 -0700 Subject: ipc/msg.c: avoid ipc_rcu_putref for failed ipc_addid() Loosely based on a patch from Kees Cook : - id and retval can be merged - if ipc_addid() fails, then use call_rcu() directly. The difference is that call_rcu is used for failed ipc_addid() calls, to continue to guaranteed an rcu delay for security_msg_queue_free(). Link: http://lkml.kernel.org/r/20170525185107.12869-16-manfred@colorfullife.com Signed-off-by: Manfred Spraul Cc: Kees Cook Cc: Davidlohr Bueso Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 10094a731b8e..cd90bfde89a4 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -132,7 +132,7 @@ static struct msg_queue *msg_alloc(void) static int newque(struct ipc_namespace *ns, struct ipc_params *params) { struct msg_queue *msq; - int id, retval; + int retval; key_t key = params->key; int msgflg = params->flg; @@ -160,10 +160,10 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) INIT_LIST_HEAD(&msq->q_senders); /* ipc_addid() locks msq upon success. */ - id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni); - if (id < 0) { - ipc_rcu_putref(&msq->q_perm, msg_rcu_free); - return id; + retval = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni); + if (retval < 0) { + call_rcu(&msq->q_perm.rcu, msg_rcu_free); + return retval; } ipc_unlock_object(&msq->q_perm); -- cgit v1.2.3 From 3d3653f9732c73feb8c4addfc1cbdaa292a399fa Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 12 Jul 2017 14:35:22 -0700 Subject: ipc: move atomic_set() to where it is needed Only after ipc_addid() has succeeded will refcounting be used, so move initialization into ipc_addid() and remove from open-coded *_alloc() routines. Link: http://lkml.kernel.org/r/20170525185107.12869-17-manfred@colorfullife.com Signed-off-by: Kees Cook Signed-off-by: Manfred Spraul Cc: Davidlohr Bueso Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 2 -- ipc/sem.c | 1 - ipc/shm.c | 2 -- ipc/util.c | 1 + 4 files changed, 1 insertion(+), 5 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index cd90bfde89a4..770342e1d327 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -117,8 +117,6 @@ static struct msg_queue *msg_alloc(void) if (unlikely(!msq)) return NULL; - atomic_set(&msq->q_perm.refcount, 1); - return msq; } diff --git a/ipc/sem.c b/ipc/sem.c index 2b2ed56e0fde..5f137738819d 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -465,7 +465,6 @@ static struct sem_array *sem_alloc(size_t nsems) return NULL; memset(sma, 0, size); - atomic_set(&sma->sem_perm.refcount, 1); return sma; } diff --git a/ipc/shm.c b/ipc/shm.c index c5976d318ed1..d1988ef821a1 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -526,8 +526,6 @@ static struct shmid_kernel *shm_alloc(void) if (unlikely(!shp)) return NULL; - atomic_set(&shp->shm_perm.refcount, 1); - return shp; } diff --git a/ipc/util.c b/ipc/util.c index 2428dd44ca97..1a2cb02467ab 100644 --- a/ipc/util.c +++ b/ipc/util.c @@ -232,6 +232,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size) idr_preload(GFP_KERNEL); + atomic_set(&new->refcount, 1); spin_lock_init(&new->lock); new->deleted = false; rcu_read_lock(); -- cgit v1.2.3 From fb259c310f79d295c2da2934ff2282e1b7c30529 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 12 Jul 2017 14:35:28 -0700 Subject: ipc/msg: remove special msg_alloc/free There is nothing special about the msg_alloc/free routines any more, so remove them to make code more readable. [manfred@colorfullife.com: Rediff to keep rcu protection for security_msg_queue_alloc()] Link: http://lkml.kernel.org/r/20170525185107.12869-19-manfred@colorfullife.com Signed-off-by: Kees Cook Signed-off-by: Manfred Spraul Cc: Davidlohr Bueso Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/msg.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) (limited to 'ipc/msg.c') diff --git a/ipc/msg.c b/ipc/msg.c index 770342e1d327..5b25e0755656 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -95,29 +95,13 @@ static inline void msg_rmid(struct ipc_namespace *ns, struct msg_queue *s) ipc_rmid(&msg_ids(ns), &s->q_perm); } -static void __msg_free(struct msg_queue *msq) -{ - kvfree(msq); -} - 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); - __msg_free(msq); -} - -static struct msg_queue *msg_alloc(void) -{ - struct msg_queue *msq; - - msq = kvmalloc(sizeof(*msq), GFP_KERNEL); - if (unlikely(!msq)) - return NULL; - - return msq; + kvfree(msq); } /** @@ -134,8 +118,8 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) key_t key = params->key; int msgflg = params->flg; - msq = msg_alloc(); - if (!msq) + msq = kvmalloc(sizeof(*msq), GFP_KERNEL); + if (unlikely(!msq)) return -ENOMEM; msq->q_perm.mode = msgflg & S_IRWXUGO; @@ -144,7 +128,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) msq->q_perm.security = NULL; retval = security_msg_queue_alloc(msq); if (retval) { - __msg_free(msq); + kvfree(msq); return retval; } -- cgit v1.2.3