On Mon, 15 May 2017 19:19:10 +0200 Manfred Spraul <manf...@colorfullife.com> wrote:
> sma->sem_base is initialized with > sma->sem_base = (struct sem *) &sma[1]; > > The current code has four problems: > - There is an unnecessary pointer dereference - sem_base is not needed. > - Alignment for struct sem only works by chance. > - The current code causes false positive for static code analysis. > - This is a cast between different non-void types, which the future > randstruct GCC plugin warns on. > > And, as bonus, the code size gets smaller: > > Before: > 0 .text 00003770 > After: > 0 .text 0000374e This clashes with Kees's patch, below. Does it have the same effect? From: Kees Cook <keesc...@chromium.org> Subject: ipc/sem: avoid indexing past end of sem_array This changes the struct + trailing data pattern to using a void * so that the end of sem_array is found without possibly indexing past the end which can upset some static analyzers. Mostly, this ends up avoiding a cast between different non-void types, which the future randstruct GCC plugin was warning about. Link: http://lkml.kernel.org/r/20170508222345.GA52073@beast Signed-off-by: Kees Cook <keesc...@chromium.org> Cc: Davidlohr Bueso <d...@stgolabs.net> Cc: Manfred Spraul <manf...@colorfullife.com> Signed-off-by: Andrew Morton <a...@linux-foundation.org> --- ipc/sem.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff -puN ipc/sem.c~ipc-sem-avoid-indexing-past-end-of-sem_array ipc/sem.c --- a/ipc/sem.c~ipc-sem-avoid-indexing-past-end-of-sem_array +++ a/ipc/sem.c @@ -475,6 +475,7 @@ static int newary(struct ipc_namespace * { int id; int retval; + void *sem_alloc; struct sem_array *sma; int size; key_t key = params->key; @@ -488,11 +489,14 @@ static int newary(struct ipc_namespace * return -ENOSPC; size = sizeof(*sma) + nsems * sizeof(struct sem); - sma = ipc_rcu_alloc(size); - if (!sma) + sem_alloc = ipc_rcu_alloc(size); + if (!sem_alloc) return -ENOMEM; - memset(sma, 0, size); + memset(sem_alloc, 0, size); + + sma = sem_alloc; + sma->sem_base = sem_alloc + sizeof(*sma); sma->sem_perm.mode = (semflg & S_IRWXUGO); sma->sem_perm.key = key; @@ -504,8 +508,6 @@ static int newary(struct ipc_namespace * return retval; } - sma->sem_base = (struct sem *) &sma[1]; - for (i = 0; i < nsems; i++) { INIT_LIST_HEAD(&sma->sem_base[i].pending_alter); INIT_LIST_HEAD(&sma->sem_base[i].pending_const); _
