From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E449C4346E for ; Fri, 25 Sep 2020 01:28:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3C60F20809 for ; Fri, 25 Sep 2020 01:28:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="PKiLyTOI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726962AbgIYB1w (ORCPT ); Thu, 24 Sep 2020 21:27:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726704AbgIYB1v (ORCPT ); Thu, 24 Sep 2020 21:27:51 -0400 Received: from mail-pf1-x444.google.com (mail-pf1-x444.google.com [IPv6:2607:f8b0:4864:20::444]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A6BD0C0613CE; Thu, 24 Sep 2020 18:27:51 -0700 (PDT) Received: by mail-pf1-x444.google.com with SMTP id f18so1553571pfa.10; Thu, 24 Sep 2020 18:27:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=WX2GerDQN/1DZwYhOgS9HfUlktjE9DG6T36wNYd6ERA=; b=PKiLyTOIgHLW+moRjohVGN3nBXtAMxY2kYeSeyQ+lEwxjVykj1TU2m/L4+JKoM4xJP ZD5NQacU8BFBKmyzQZHKsWN/98bPRw49TlHi9pKqHibcCLtAf1QS0VXLijE1Cs6czB3N 3FV6Ph1hXkRMiZsuVZd/MWznqd7sOgwK+P+/mwrQIl6x2+Wewik48GDaSWMpCnMaLw/l 2HUPAkCy6bvpxVpZvsvkJtXo8s30ovp8FcyPPFv7BMfv142P8Wh5DhEXpOdYiaEwF8JR k5lYb97ny5XkQfNh77xbpa2eGyzBKLqYNA2qMwCsBFfi1R8pmaM0ASTB0cNqrvjoK7Fk rr2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=WX2GerDQN/1DZwYhOgS9HfUlktjE9DG6T36wNYd6ERA=; b=t67l/UWl/2zTJ1OOA0rPX6ySHullJfYZ6Zu66PmITeIz/f/oB/BjKknf1pYQAUODlS Xqli4K5cl0KUTHm104urWB0ItzPVUyRub1M3+A8jJWPsiMo2uxkFgtOxg9Dt5gC62ja+ NNNYFHFJZi/dGvVhYqPV4KOKQtPwpnB4CaOax2RWPN3vQrez0JJ8zOH0KUBy9/SWa1jS OeTwN7jdDsw3akYL+4jpfmwkEBGbXsY1Q0vw4KQGyCSQagCt6D739yyAW5nF9YtijBnK r1V0VN0M0O+AhYN7bQnioZYQssVOROd2+Xio7H9j8hfeLfVmSNns1EYXUoquuqhEZToP k3Rg== X-Gm-Message-State: AOAM532IKmP/EFD44U2whG5uiRFY4KN3Dc7dzkykZNxdi4TZlDcEg8Mv WYOSD7aP32WSs+xsgc6vo/v1FPcMZiFOojElE78= X-Google-Smtp-Source: ABdhPJySE+Qder4PaqnMJALNyB2UUS+F0xdqjGoIiv8n/RD6wdcY8DBlVZoIdbJGt48BGDMCA2TPysEzgBxUQCqP50s= X-Received: by 2002:aa7:8d4c:0:b029:150:f692:4129 with SMTP id s12-20020aa78d4c0000b0290150f6924129mr1854155pfe.11.1600997271069; Thu, 24 Sep 2020 18:27:51 -0700 (PDT) MIME-Version: 1.0 References: <202009241658.A062D6AE@keescook> In-Reply-To: <202009241658.A062D6AE@keescook> From: YiFei Zhu Date: Thu, 24 Sep 2020 20:27:40 -0500 Message-ID: Subject: Re: [PATCH v2 seccomp 2/6] asm/syscall.h: Add syscall_arches[] array To: Kees Cook Cc: YiFei Zhu , Linux Containers , bpf , kernel list , Aleksa Sarai , Andrea Arcangeli , Andy Lutomirski , Dimitrios Skarlatos , Giuseppe Scrivano , Hubertus Franke , Jack Chen , Jann Horn , Josep Torrellas , Tianyin Xu , Tobin Feldman-Fitzthum , Tycho Andersen , Valentin Rothberg , Will Drewry Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [resending this too] On Thu, Sep 24, 2020 at 6:01 PM Kees Cook wrote: > Disregarding the "how" of this, yeah, we'll certainly need something to > tell seccomp about the arrangement of syscall tables and how to find > them. > > However, I'd still prefer to do this on a per-arch basis, and include > more detail, as I've got in my v1. > > Something missing from both styles, though, is a consolidation of > values, where the AUDIT_ARCH* isn't reused in both the seccomp info and > the syscall_get_arch() return. The problems here were two-fold: > > 1) putting this in syscall.h meant you do not have full NR_syscall* > visibility on some architectures (e.g. arm64 plays weird games with > header include order). I don't get this one -- I'm not playing with NR_syscall here. > 2) seccomp needs to handle "multiplexed" tables like x86_x32 (distros > haven't removed CONFIG_X86_X32 widely yet, so it is a reality that > it must be dealt with), which means seccomp's idea of the arch > "number" can't be the same as the AUDIT_ARCH. Why so? Does anyone actually use x32 in a container? The memory cost and analysis cost is on everyone. The worst case scenario if we don't support it is that the syscall is not accelerated. > So, likely a combo of approaches is needed: an array (or more likely, > enum), declared in the per-arch seccomp.h file. And I don't see a way > to solve #1 cleanly. > > Regardless, it needs to be split per architecture so that regressions > can be bisected/reverted/isolated cleanly. And if we can't actually test > it at runtime (or find someone who can) it's not a good idea to make the > change. :) You have a good point regarding tests. Don't see how it affects regressions though. Only one file here is ever included per-build. > > [...] > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > index 7cbf733d11af..e13bb2a65b6f 100644 > > --- a/arch/x86/include/asm/syscall.h > > +++ b/arch/x86/include/asm/syscall.h > > @@ -97,6 +97,10 @@ static inline void syscall_set_arguments(struct task_struct *task, > > memcpy(®s->bx + i, args, n * sizeof(args[0])); > > } > > > > +static __maybe_unused const int syscall_arches[] = { > > + AUDIT_ARCH_I386 > > +}; > > + > > static inline int syscall_get_arch(struct task_struct *task) > > { > > return AUDIT_ARCH_I386; > > @@ -152,6 +156,13 @@ static inline void syscall_set_arguments(struct task_struct *task, > > } > > } > > > > +static __maybe_unused const int syscall_arches[] = { > > + AUDIT_ARCH_X86_64, > > +#ifdef CONFIG_IA32_EMULATION > > + AUDIT_ARCH_I386, > > +#endif > > +}; > > I'm leaving this section quoted because I'll refer to it in a later > patch review... > > -- > Kees Cook