Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752813AbdI0Pmk (ORCPT ); Wed, 27 Sep 2017 11:42:40 -0400 Received: from mail-io0-f169.google.com ([209.85.223.169]:46317 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752187AbdI0Pmi (ORCPT ); Wed, 27 Sep 2017 11:42:38 -0400 X-Google-Smtp-Source: AOwi7QCb11uYJO5HW+E+rRzzf4+4QSsapmxnl+lyUpRw/pXGTW2Ao2JAg7CZ/MzqWnBvYOWKIV1pp3kTzDFykM7KOeA= MIME-Version: 1.0 In-Reply-To: <20170927152530.26520-1-tycho@docker.com> References: <20170927152530.26520-1-tycho@docker.com> From: Kees Cook Date: Wed, 27 Sep 2017 17:42:35 +0200 X-Google-Sender-Auth: IlKnoLVAntdUB7PgNF9z6Y_3GSM Message-ID: Subject: Re: [PATCH v2] seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter() To: Tycho Andersen Cc: LKML , Andy Lutomirski , Will Drewry , "security@kernel.org" , Chris Salls , Oleg Nesterov , "# 3.4.x" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3213 Lines: 99 On Wed, Sep 27, 2017 at 5:25 PM, Tycho Andersen wrote: > From: Oleg Nesterov > > As Chris explains, get_seccomp_filter() and put_seccomp_filter() can > use the different filters, once we drop ->siglock task->seccomp.filter > can be replaced by SECCOMP_FILTER_FLAG_TSYNC. > > v2: add __get_seccomp_filter vs. open coding refcount_inc() > > Fixes: f8e529ed941b ("seccomp, ptrace: add support for dumping seccomp filters") > Reported-by: Chris Salls > Cc: stable@vger.kernel.org > Signed-off-by: Oleg Nesterov > Signed-off-by: Tycho Andersen Thanks, applied! -Kees > --- > kernel/seccomp.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > index c24579dfa7a1..bb3a38005b9c 100644 > --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -473,14 +473,19 @@ static long seccomp_attach_filter(unsigned int flags, > return 0; > } > > +void __get_seccomp_filter(struct seccomp_filter *filter) > +{ > + /* Reference count is bounded by the number of total processes. */ > + refcount_inc(&filter->usage); > +} > + > /* get_seccomp_filter - increments the reference count of the filter on @tsk */ > void get_seccomp_filter(struct task_struct *tsk) > { > struct seccomp_filter *orig = tsk->seccomp.filter; > if (!orig) > return; > - /* Reference count is bounded by the number of total processes. */ > - refcount_inc(&orig->usage); > + __get_seccomp_filter(orig); > } > > static inline void seccomp_filter_free(struct seccomp_filter *filter) > @@ -491,10 +496,8 @@ static inline void seccomp_filter_free(struct seccomp_filter *filter) > } > } > > -/* put_seccomp_filter - decrements the ref count of tsk->seccomp.filter */ > -void put_seccomp_filter(struct task_struct *tsk) > +static void __put_seccomp_filter(struct seccomp_filter *orig) > { > - struct seccomp_filter *orig = tsk->seccomp.filter; > /* Clean up single-reference branches iteratively. */ > while (orig && refcount_dec_and_test(&orig->usage)) { > struct seccomp_filter *freeme = orig; > @@ -503,6 +506,12 @@ void put_seccomp_filter(struct task_struct *tsk) > } > } > > +/* put_seccomp_filter - decrements the ref count of tsk->seccomp.filter */ > +void put_seccomp_filter(struct task_struct *tsk) > +{ > + __put_seccomp_filter(tsk->seccomp.filter); > +} > + > static void seccomp_init_siginfo(siginfo_t *info, int syscall, int reason) > { > memset(info, 0, sizeof(*info)); > @@ -1025,13 +1034,13 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off, > if (!data) > goto out; > > - get_seccomp_filter(task); > + __get_seccomp_filter(filter); > spin_unlock_irq(&task->sighand->siglock); > > if (copy_to_user(data, fprog->filter, bpf_classic_proglen(fprog))) > ret = -EFAULT; > > - put_seccomp_filter(task); > + __put_seccomp_filter(filter); > return ret; > > out: > -- > 2.11.0 > -- Kees Cook Pixel Security