On Mon, May 15, 2017 at 10:19 AM, Manfred Spraul
<manf...@colorfullife.com> wrote:
> 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.
>
> Signed-off-by: Manfred Spraul <manf...@colorfullife.com>
> ---
> include/linux/ipc.h | 3 +++
> ipc/msg.c | 22 ++++++++++++++--------
> ipc/sem.c | 35 ++++++++++++++++++++---------------
> ipc/shm.c | 21 ++++++++++++++-------
> ipc/util.c | 33 +++++++++++++++------------------
> ipc/util.h | 18 +++++++-----------
> 6 files changed, 73 insertions(+), 59 deletions(-)
>
> diff --git a/include/linux/ipc.h b/include/linux/ipc.h
> index 71fd92d..5591f055 100644
> --- a/include/linux/ipc.h
> +++ b/include/linux/ipc.h
> @@ -20,6 +20,9 @@ struct kern_ipc_perm {
> umode_t mode;
> unsigned long seq;
> void *security;
> +
> + struct rcu_head rcu;
> + atomic_t refcount;
> } ____cacheline_aligned_in_smp;
>
> #endif /* _LINUX_IPC_H */
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 104926d..e9785c4 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,13 @@ 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));
> + if (offsetof(struct msg_queue, q_perm) != 0) {
> + pr_err("Invalid struct sem_perm, failing msgget().\n");
> + return -ENOMEM;
> + }
This is a compile-time check that will result in a runtime warning.
This should be a BUILD_BUG_ON() instead. It means we can't randomize
the first element of these structures, but I'll deal with that in the
randstruct plugin (or send another fix when I dig through it).
It looks like the "as first element" is needed to share the alloc/put
routines. To randomize the position of kern_ipc_perm within each
structure means having different accessor functions, which makes this
less fun. :P
Anyway, regardless, these patches make the sem_base thing go away and
consolidates other stuff which looks good to me.
Thanks!
-Kees
--
Kees Cook
Pixel Security
