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=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 AF380CA90AF for ; Wed, 13 May 2020 11:18:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 920E5206E5 for ; Wed, 13 May 2020 11:18:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732484AbgEMLSJ (ORCPT ); Wed, 13 May 2020 07:18:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34814 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732405AbgEMLSH (ORCPT ); Wed, 13 May 2020 07:18:07 -0400 Received: from smtp-42ab.mail.infomaniak.ch (smtp-42ab.mail.infomaniak.ch [IPv6:2001:1600:3:17::42ab]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6FB72C061A0C for ; Wed, 13 May 2020 04:18:07 -0700 (PDT) Received: from smtp-2-0000.mail.infomaniak.ch (unknown [10.5.36.107]) by smtp-2-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 49MXGL6nZBzlhDCt; Wed, 13 May 2020 13:18:02 +0200 (CEST) Received: from ns3096276.ip-94-23-54.eu (unknown [94.23.54.103]) by smtp-2-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 49MXGJ55kJzlhFDc; Wed, 13 May 2020 13:18:00 +0200 (CEST) Subject: Re: [PATCH v5 4/6] selftest/openat2: Add tests for O_MAYEXEC enforcing To: Kees Cook Cc: linux-kernel@vger.kernel.org, Aleksa Sarai , Alexei Starovoitov , Al Viro , Andy Lutomirski , Christian Heimes , Daniel Borkmann , Deven Bowers , Eric Chiang , Florian Weimer , James Morris , Jan Kara , Jann Horn , Jonathan Corbet , Lakshmi Ramasubramanian , Matthew Garrett , Matthew Wilcox , Michael Kerrisk , =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= , Mimi Zohar , =?UTF-8?Q?Philippe_Tr=c3=a9buchet?= , Scott Shell , Sean Christopherson , Shuah Khan , Steve Dower , Steve Grubb , Thibaut Sautereau , Vincent Strubel , kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org References: <20200505153156.925111-1-mic@digikod.net> <20200505153156.925111-5-mic@digikod.net> <202005121452.4DED41A@keescook> From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: <6fc6bbe4-da18-1b8d-a61c-558f5a4a4908@digikod.net> Date: Wed, 13 May 2020 13:18:00 +0200 User-Agent: MIME-Version: 1.0 In-Reply-To: <202005121452.4DED41A@keescook> Content-Type: text/plain; charset=iso-8859-15 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Antivirus: Dr.Web (R) for Unix mail servers drweb plugin ver.6.0.2.8 X-Antivirus-Code: 0x100000 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/05/2020 23:57, Kees Cook wrote: > On Tue, May 05, 2020 at 05:31:54PM +0200, Mickaël Salaün wrote: >> Test propagation of noexec mount points or file executability through >> files open with or without O_MAYEXEC, thanks to the >> fs.open_mayexec_enforce sysctl. >> >> Signed-off-by: Mickaël Salaün >> Reviewed-by: Thibaut Sautereau >> Cc: Aleksa Sarai >> Cc: Al Viro >> Cc: Kees Cook >> Cc: Shuah Khan > > Yay tests! :) Notes below... > >> diff --git a/tools/testing/selftests/openat2/Makefile b/tools/testing/selftests/openat2/Makefile >> index 4b93b1417b86..cb98bdb4d5b1 100644 >> --- a/tools/testing/selftests/openat2/Makefile >> +++ b/tools/testing/selftests/openat2/Makefile >> @@ -1,7 +1,8 @@ >> # SPDX-License-Identifier: GPL-2.0-or-later >> >> CFLAGS += -Wall -O2 -g -fsanitize=address -fsanitize=undefined >> -TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test >> +LDLIBS += -lcap >> +TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test omayexec_test > > I realize the others have _test in their name, but that feels intensely > redundant to me. :) It is redundant in the path name but it is useful to match the generated files e.g., in gitignore. > >> [...] >> diff --git a/tools/testing/selftests/openat2/omayexec_test.c b/tools/testing/selftests/openat2/omayexec_test.c >> new file mode 100644 >> index 000000000000..7052c852daf8 >> --- /dev/null >> +++ b/tools/testing/selftests/openat2/omayexec_test.c >> [...] >> +FIXTURE_DATA(mount_exec_file_exec) { }; > > For each of these, Please use "FIXTURE" not "FIXTURE_DATA". See: > 1ae81d78a8b2 ("selftests/seccomp: Adjust test fixture counts") Indeed. > >> +FIXTURE_SETUP(mount_exec_file_exec) >> +{ >> + create_workspace(_metadata, 1, 1); > > Maybe save the system's original sysctl in create_workspace() instead > of always restoring it to 0 in delete_workspace()? Right. > > Otherwise, looks good! > Thanks.