Re: [PATCH 02/20] ipc: merge ipc_rcu and kern_ipc_perm

[Kees Cook]

On Thu, May 25, 2017 at 11:50 AM, Manfred Spraul
<manfred@xxxxxxxxxxxxxxxx> 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.
>
> To simplify code, the ipc_rcu_alloc initializes the allocation to 0.

I don't see this change in the code, only the removal of sem's
memset(..., 0, ...)?

> diff --git a/ipc/sem.c b/ipc/sem.c
> index fff8337..bdff6d9 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -469,20 +469,20 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
>         if (ns->used_sems + nsems > ns->sc_semmns)
>                 return -ENOSPC;
>
> +       BUILD_BUG_ON(offsetof(struct sem_array, sem_perm) != 0);
> +
>         size = sizeof(*sma) + nsems * sizeof(sma->sems[0]);
> -       sma = ipc_rcu_alloc(size);
> +       sma = container_of(ipc_rcu_alloc(size), struct sem_array, sem_perm);
>         if (!sma)
>                 return -ENOMEM;
>
> -       memset(sma, 0, size);
> -
>         sma->sem_perm.mode = (semflg & S_IRWXUGO);
>         sma->sem_perm.key = key;
>
>         sma->sem_perm.security = NULL;
>         retval = security_sem_alloc(sma);
>         if (retval) {
> -               ipc_rcu_putref(sma, ipc_rcu_free);
> +               ipc_rcu_putref(&sma->sem_perm, ipc_rcu_free);
>                 return retval;
>         }
>
> [...]
> diff --git a/ipc/util.c b/ipc/util.c
> index caec7b1..9dcc08b 100644
> --- a/ipc/util.c
> +++ b/ipc/util.c
> @@ -418,46 +418,43 @@ void ipc_free(void *ptr)
>  }
>
>  /**
> - * ipc_rcu_alloc - allocate ipc and rcu space
> + * ipc_rcu_alloc - allocate ipc space
>   * @size: size desired
>   *
> - * Allocate memory for the rcu header structure +  the object.
> - * Returns the pointer to the object or NULL upon failure.
> + * Allocate memory for an ipc object.
> + * The first member must be struct kern_ipc_perm.
>   */
> -void *ipc_rcu_alloc(int size)
> +struct kern_ipc_perm *ipc_rcu_alloc(int size)
>  {
>         /*
>          * We prepend the allocation with the rcu struct
>          */
> -       struct ipc_rcu *out = ipc_alloc(sizeof(struct ipc_rcu) + size);
> +       struct kern_ipc_perm *out = ipc_alloc(size);
>         if (unlikely(!out))
>                 return NULL;
>         atomic_set(&out->refcount, 1);
> -       return out + 1;
> +       return out;
>  }

-Kees

-- 
Kees Cook
Pixel Security