From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752936AbdLETJ1 (ORCPT ); Tue, 5 Dec 2017 14:09:27 -0500 Received: from mail-vk0-f68.google.com ([209.85.213.68]:45361 "EHLO mail-vk0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752705AbdLETJZ (ORCPT ); Tue, 5 Dec 2017 14:09:25 -0500 X-Google-Smtp-Source: AGs4zMbldyjJweLJ5byRu+v+l0XYg9DCNo9sJ2Cp3Ey7ce7NJrq3xKl/QKoxckjWzHnjf5TCV56y+hbBFsjKsC9VwwU= MIME-Version: 1.0 In-Reply-To: <20171205151724.1764896-1-arnd@arndb.de> References: <20171205151724.1764896-1-arnd@arndb.de> From: Kees Cook Date: Tue, 5 Dec 2017 11:09:23 -0800 X-Google-Sender-Auth: f0Vp2bLKz0gC6zENiyHJbiV4qlw Message-ID: Subject: Re: [PATCH v2] exec: avoid gcc-8 warning for get_task_comm To: Arnd Bergmann Cc: Alexander Viro , Ingo Molnar , Peter Zijlstra , Serge Hallyn , James Morris , Andrew Morton , Aleksa Sarai , "Eric W. Biederman" , Frederic Weisbecker , Thomas Gleixner , "linux-fsdevel@vger.kernel.org" , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 5, 2017 at 7:17 AM, Arnd Bergmann wrote: > gcc-8 warns about using strncpy() with the source size as the limit: > > fs/exec.c:1223:32: error: argument to 'sizeof' in 'strncpy' call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess] > > This is indeed slightly suspicious, as it protects us from source > arguments without NUL-termination, but does not guarantee that the > destination is terminated. > > This keeps the strncpy() to ensure we have properly padded target buffer, > but ensures that we use the correct length, by passing the actual length > of the destination buffer as well as adding a build-time check to ensure > it is exactly TASK_COMM_LEN. There are only 23 callsights which I all > reviewed to ensure this is currently the case. We could get away with > doing only the check or passing the right length, but it doesn't hurt > to do both. > > Suggested-by: Kees Cook > Signed-off-by: Arnd Bergmann Awesome. Thanks for checking them all! Acked-by: Kees Cook -Kees > --- > fs/exec.c | 7 +++---- > include/linux/sched.h | 6 +++++- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 6be2aa0ab26f..156f56acfe8e 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1216,15 +1216,14 @@ static int de_thread(struct task_struct *tsk) > return -EAGAIN; > } > > -char *get_task_comm(char *buf, struct task_struct *tsk) > +char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *tsk) > { > - /* buf must be at least sizeof(tsk->comm) in size */ > task_lock(tsk); > - strncpy(buf, tsk->comm, sizeof(tsk->comm)); > + strncpy(buf, tsk->comm, buf_size); > task_unlock(tsk); > return buf; > } > -EXPORT_SYMBOL_GPL(get_task_comm); > +EXPORT_SYMBOL_GPL(__get_task_comm); > > /* > * These functions flushes out all traces of the currently running executable > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 21991d668d35..5124ba709830 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1503,7 +1503,11 @@ static inline void set_task_comm(struct task_struct *tsk, const char *from) > __set_task_comm(tsk, from, false); > } > > -extern char *get_task_comm(char *to, struct task_struct *tsk); > +extern char *__get_task_comm(char *to, size_t len, struct task_struct *tsk); > +#define get_task_comm(buf, tsk) ({ \ > + BUILD_BUG_ON(sizeof(buf) != TASK_COMM_LEN); \ > + __get_task_comm(buf, sizeof(buf), tsk); \ > +}) > > #ifdef CONFIG_SMP > void scheduler_ipi(void); > -- > 2.9.0 > -- Kees Cook Pixel Security