From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966587AbdEOUIv (ORCPT ); Mon, 15 May 2017 16:08:51 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:39294 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932677AbdEOUIt (ORCPT ); Mon, 15 May 2017 16:08:49 -0400 Date: Mon, 15 May 2017 13:08:44 -0700 From: Andrew Morton To: Manfred Spraul Cc: mtk.manpages@gmail.com, Kees Cook , LKML , 1vier1@web.de, Davidlohr Bueso , mingo@kernel.org, peterz@infradead.org, fabf@skynet.be Subject: Re: [PATCH 1/3] ipc/sem.c: remove sem_base, embed struct sem Message-Id: <20170515130844.6661b79166f983ba24d87710@linux-foundation.org> In-Reply-To: <20170515171912.6298-2-manfred@colorfullife.com> References: <20170508222345.GA52073@beast> <20170515171912.6298-1-manfred@colorfullife.com> <20170515171912.6298-2-manfred@colorfullife.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 15 May 2017 19:19:10 +0200 Manfred Spraul 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 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 Cc: Davidlohr Bueso Cc: Manfred Spraul Signed-off-by: Andrew Morton --- 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); _