Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752023AbdHHBqH (ORCPT ); Mon, 7 Aug 2017 21:46:07 -0400 Received: from mail-it0-f54.google.com ([209.85.214.54]:36401 "EHLO mail-it0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751965AbdHHBqE (ORCPT ); Mon, 7 Aug 2017 21:46:04 -0400 MIME-Version: 1.0 In-Reply-To: References: <1501730353-46840-1-git-send-email-keescook@chromium.org> <1501730353-46840-2-git-send-email-keescook@chromium.org> From: Kees Cook Date: Mon, 7 Aug 2017 18:46:03 -0700 X-Google-Sender-Auth: S-oCKGup_Ro5kQZXrpnmpgEkrvg Message-ID: Subject: Re: [PATCH 1/4] seccomp: Provide matching filter for introspection To: Tyler Hicks Cc: LKML , Fabricio Voznika , Andy Lutomirski , Will Drewry , Shuah Khan , "open list:KERNEL SELFTEST FRAMEWORK" , linux-security-module 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: 1399 Lines: 36 On Mon, Aug 7, 2017 at 6:03 PM, Tyler Hicks wrote: >> -static u32 seccomp_run_filters(const struct seccomp_data *sd) >> +static u32 seccomp_run_filters(const struct seccomp_data *sd, >> + struct seccomp_filter **match) >> { >> struct seccomp_data sd_local; >> u32 ret = SECCOMP_RET_ALLOW; > > My version of this patch initialized *match to f here. The reason I did > that is because if BPF_PROG_RUN() returns RET_ALLOW for all > filters, I didn't want *match to remain NULL when seccomp_run_filters() > returns. FILTER_FLAG_LOG nor FILTER_FLAG_KILL_PROCESS would be affected > by this because they don't care about RET_ALLOW actions but there could > conceivably be a filter flag in the future that cares about RET_ALLOW > and not initializing *match to the first filter could result in a latent > bug for that filter flag. Very true, yes. I did intentionally adjust this because I wanted to keep the hot path as untouched as possible. > I'm fine with not adding the initialization since this is a hot path and > it doesn't help any of the currently existing/planned filter flags but I > wanted to at least mention it. Yeah, and while I doubt I'll want to ever check "match" for RET_ALLOW, I'll add a big comment there to explain it. > Reviewed-by: Tyler Hicks Thanks! -Kees -- Kees Cook Pixel Security