From owner-svn-src-head@freebsd.org Sat Jan 14 00:37:00 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6A0E8CAD7EE; Sat, 14 Jan 2017 00:37:00 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 08F5C1B02; Sat, 14 Jan 2017 00:36:59 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-wm0-f48.google.com with SMTP id n129so15746221wmn.0; Fri, 13 Jan 2017 16:36:59 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=rIJcztu5sNA0lB898Q1sD+P9a0YpUOOLCP6QpYOHSUY=; b=mG+2BsPkxaXRK0/r6DCq3TrmeCTwzaokZEFFmlKsxEDeNXYjcGAFYN9wW9YcsTRsHg RNzaEJDK6ddabaAF+j3dnyRTOHTgxxRCqcJpgcFvnfZ0X2TwCdMeglkW8oUszKglqCV4 y2YbGFYiIPMdaMCalp9OBwpejaJxxumI1umEeQh4dLqrpSR4wrChm87F+7JyAOxcxVo5 ufFcaruTIWVx+G8mMC22eCssPr86hqlp2jViAkoPAN11HpjhCTW3xT3VXU0igSlCQlHT XJVUecOG5iGCK6XMJwKxGwvZiw3UQK9/CtGOnOKUXfOUQQQ10NBgQlZa5iPiIbZPylJo t90A== X-Gm-Message-State: AIkVDXKa4NRv4AYTG3EC4875RG8zFML5M9Rpz/OE/GUKMpvb2y6AxxsxeeMMEAKvo2gB3Q== X-Received: by 10.28.147.147 with SMTP id v141mr4471032wmd.110.1484352559816; Fri, 13 Jan 2017 16:09:19 -0800 (PST) Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com. [74.125.82.41]) by smtp.gmail.com with ESMTPSA id s20sm7968534wmb.9.2017.01.13.16.09.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Jan 2017 16:09:19 -0800 (PST) Received: by mail-wm0-f41.google.com with SMTP id c206so93684413wme.0; Fri, 13 Jan 2017 16:09:19 -0800 (PST) X-Received: by 10.223.154.132 with SMTP id a4mr15826415wrc.188.1484352559430; Fri, 13 Jan 2017 16:09:19 -0800 (PST) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.194.29.72 with HTTP; Fri, 13 Jan 2017 16:09:18 -0800 (PST) In-Reply-To: <201701010401.v0141RpZ072608@repo.freebsd.org> References: <201701010401.v0141RpZ072608@repo.freebsd.org> From: Conrad Meyer Date: Fri, 13 Jan 2017 16:09:18 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r310994 - head/tests/sys/vfs To: Ngie Cooper Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Jan 2017 00:37:00 -0000 Um, this is garbage and ruins the entire point of using a standardized framework like ATF. Please revert it. Instead, just have the Kyua framework preopen its output files before running tests. The diff will be an order of magnitude smaller than this one is and it will fix the problem generally instead of adding fork/wait cruft and undetailed assert()s to every test. Conrad On Sat, Dec 31, 2016 at 8:01 PM, Ngie Cooper wrote: > Author: ngie > Date: Sun Jan 1 04:01:27 2017 > New Revision: 310994 > URL: https://svnweb.freebsd.org/changeset/base/310994 > > Log: > Make sys/vfs/lookup_cap_dotdot actually work with "kyua test" > > The tests don't work when reading/writing to file descriptors in the > sandbox after entering capability mode (and wouldn't have, regardless > of the framework), so adjust the tests so they function within the > framework. > > For tests that enter capability mode over the course of the test, the > following is now done: > > 1. Fork child process for capability mode test. > 2. In child... > i. Enter capability mode. > ii. Test invariants. > iii. Exit after calling test function. > 3. Collect status for child and determine whether or not it completed > successfully. > > In order to test the invariants in the child process, they now use assert(3) > instead of ATF_REQUIRE*, as the atf-c-api functions right to results files > in the directories in order to determine where and how tests fail. > > While in the area, fix several -Wshadow and -Wunused warnings found when I > bumped WARNS up to 6, and fix some minor style(9) issues with indentation > and type alignment. > > PR: 215690 > > Modified: > head/tests/sys/vfs/lookup_cap_dotdot.c > > Modified: head/tests/sys/vfs/lookup_cap_dotdot.c > ============================================================================== > --- head/tests/sys/vfs/lookup_cap_dotdot.c Sun Jan 1 00:43:20 2017 (r310993) > +++ head/tests/sys/vfs/lookup_cap_dotdot.c Sun Jan 1 04:01:27 2017 (r310994) > @@ -31,23 +31,27 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > > #include > +#include > #include > #include > #include > > #include "freebsd_test_suite/macros.h" > > -static int dirfd = -1; > -static char *abspath; > +static char *abspath; > +static int dirfd = -1; > + > +typedef void (*child_test_fn_t)(void); > > static void > -touchat(int dirfd, const char *name) > +touchat(int _dirfd, const char *name) > { > int fd; > > - ATF_REQUIRE((fd = openat(dirfd, name, O_CREAT | O_TRUNC | O_WRONLY, > + ATF_REQUIRE((fd = openat(_dirfd, name, O_CREAT | O_TRUNC | O_WRONLY, > 0777)) >= 0); > ATF_REQUIRE(close(fd) == 0); > } > @@ -78,10 +82,43 @@ prepare_dotdot_tests(void) > static void > check_capsicum(void) > { > + > ATF_REQUIRE_FEATURE("security_capabilities"); > ATF_REQUIRE_FEATURE("security_capability_mode"); > } > > +static void > +run_capsicum_test(child_test_fn_t test_func) > +{ > + int child_exit_code, child_status; > + pid_t child_pid; > + > + check_capsicum(); > + prepare_dotdot_tests(); > + > + ATF_REQUIRE_MSG((child_pid = fork()) != -1, > + "fork failed: %s", strerror(errno)); > + > + if (child_pid == 0) { > + test_func(); > + _exit(0); > + } > + > + ATF_REQUIRE_MSG(waitpid(child_pid, &child_status, 0) != -1, > + "waitpid failed: %s", strerror(errno)); > + if (WIFEXITED(child_status)) { > + child_exit_code = WEXITSTATUS(child_status); > + ATF_REQUIRE_MSG(child_exit_code == 0, > + "child exited with non-zero exit code: %d", > + child_exit_code); > + } else if (WIFSIGNALED(child_status)) > + atf_tc_fail("child exited with signal: %d", > + WTERMSIG(child_status)); > + else > + atf_tc_fail("child exited with unexpected status: %d", > + child_status); > +} > + > /* > * Positive tests > */ > @@ -93,6 +130,7 @@ ATF_TC_HEAD(openat__basic_positive, tc) > > ATF_TC_BODY(openat__basic_positive, tc) > { > + > prepare_dotdot_tests(); > > ATF_REQUIRE(openat(dirfd, "d1/d2/d3/f3", O_RDONLY) >= 0); > @@ -114,21 +152,22 @@ ATF_TC_HEAD(lookup_cap_dotdot__basic, tc > "Validate cap-mode (testdir)/d1/.. lookup"); > } > > -ATF_TC_BODY(lookup_cap_dotdot__basic, tc) > +static void > +lookup_cap_dotdot__basic_child(void) > { > cap_rights_t rights; > - int fd; > - > - check_capsicum(); > - prepare_dotdot_tests(); > > cap_rights_init(&rights, CAP_LOOKUP, CAP_READ); > - ATF_REQUIRE(cap_rights_limit(dirfd, &rights) >= 0); > > - ATF_REQUIRE(cap_enter() >= 0); > + assert(cap_rights_limit(dirfd, &rights) >= 0); > + assert(cap_enter() >= 0); > + assert(openat(dirfd, "d1/..", O_RDONLY) >= 0); > +} > + > +ATF_TC_BODY(lookup_cap_dotdot__basic, tc) > +{ > > - ATF_REQUIRE_MSG(openat(dirfd, "d1/..", O_RDONLY) >= 0, "%s", > - strerror(errno)); > + run_capsicum_test(lookup_cap_dotdot__basic_child); > } > > ATF_TC(lookup_cap_dotdot__advanced); > @@ -138,23 +177,26 @@ ATF_TC_HEAD(lookup_cap_dotdot__advanced, > "Validate cap-mode (testdir)/d1/.. lookup"); > } > > -ATF_TC_BODY(lookup_cap_dotdot__advanced, tc) > +static void > +lookup_cap_dotdot__advanced_child(void) > { > cap_rights_t rights; > - int fd; > - > - check_capsicum(); > - prepare_dotdot_tests(); > > cap_rights_init(&rights, CAP_LOOKUP, CAP_READ); > - ATF_REQUIRE(cap_rights_limit(dirfd, &rights) >= 0); > + assert(cap_rights_limit(dirfd, &rights) >= 0); > > - ATF_REQUIRE(cap_enter() >= 0); > + assert(cap_enter() >= 0); > > - ATF_REQUIRE(openat(dirfd, "d1/d2/d3/../../f1", O_RDONLY) >= 0); > - ATF_REQUIRE(openat(dirfd, "l3/../../f1", O_RDONLY) >= 0); > - ATF_REQUIRE(openat(dirfd, "l3/ld1", O_RDONLY) >= 0); > - ATF_REQUIRE(openat(dirfd, "l3/lf1", O_RDONLY) >= 0); > + assert(openat(dirfd, "d1/d2/d3/../../f1", O_RDONLY) >= 0); > + assert(openat(dirfd, "l3/../../f1", O_RDONLY) >= 0); > + assert(openat(dirfd, "l3/ld1", O_RDONLY) >= 0); > + assert(openat(dirfd, "l3/lf1", O_RDONLY) >= 0); > +} > + > +ATF_TC_BODY(lookup_cap_dotdot__advanced, tc) > +{ > + > + run_capsicum_test(lookup_cap_dotdot__advanced_child); > } > > /* > @@ -168,6 +210,7 @@ ATF_TC_HEAD(openat__basic_negative, tc) > > ATF_TC_BODY(openat__basic_negative, tc) > { > + > prepare_dotdot_tests(); > > ATF_REQUIRE_ERRNO(ENOENT, > @@ -182,32 +225,43 @@ ATF_TC_HEAD(capmode__negative, tc) > atf_tc_set_md_var(tc, "descr", "Negative Capability mode testcases"); > } > > -ATF_TC_BODY(capmode__negative, tc) > +static void > +capmode__negative_child(void) > { > int subdirfd; > > - check_capsicum(); > - prepare_dotdot_tests(); > - > - ATF_REQUIRE(cap_enter() == 0); > + assert(cap_enter() == 0); > > /* open() not permitted in capability mode */ > - ATF_REQUIRE_ERRNO(ECAPMODE, open("testdir", O_RDONLY) < 0); > + assert(open("testdir", O_RDONLY) < 0); > + assert(errno == ECAPMODE); > > /* AT_FDCWD not permitted in capability mode */ > - ATF_REQUIRE_ERRNO(ECAPMODE, openat(AT_FDCWD, "d1/f1", O_RDONLY) < 0); > + assert(openat(AT_FDCWD, "d1/f1", O_RDONLY) < 0); > + assert(errno == ECAPMODE); > > /* Relative path above dirfd not capable */ > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, "..", O_RDONLY) < 0); > - ATF_REQUIRE((subdirfd = openat(dirfd, "l3", O_RDONLY)) >= 0); > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, > - openat(subdirfd, "../../f1", O_RDONLY) < 0); > + assert(openat(dirfd, "..", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > + > + assert((subdirfd = openat(dirfd, "l3", O_RDONLY)) >= 0); > + assert(openat(subdirfd, "../../f1", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > + (void)close(subdirfd); > > /* Absolute paths not capable */ > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, abspath, O_RDONLY) < 0); > + assert(openat(dirfd, abspath, O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > > /* Symlink above dirfd */ > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, "lup/f1", O_RDONLY) < 0); > + assert(openat(dirfd, "lup/f1", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > +} > + > +ATF_TC_BODY(capmode__negative, tc) > +{ > + > + run_capsicum_test(capmode__negative_child); > } > > ATF_TC(lookup_cap_dotdot__negative); > @@ -217,22 +271,30 @@ ATF_TC_HEAD(lookup_cap_dotdot__negative, > "Validate cap-mode (testdir)/.. lookup fails"); > } > > -ATF_TC_BODY(lookup_cap_dotdot__negative, tc) > +static void > +lookup_cap_dotdot__negative_child(void) > { > cap_rights_t rights; > - int fd; > - > - check_capsicum(); > - prepare_dotdot_tests(); > > cap_rights_init(&rights, CAP_LOOKUP, CAP_READ); > - ATF_REQUIRE(cap_rights_limit(dirfd, &rights) >= 0); > + assert(cap_rights_limit(dirfd, &rights) >= 0); > > - ATF_REQUIRE(cap_enter() >= 0); > + assert(cap_enter() >= 0); > + > + assert(openat(dirfd, "..", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > + > + assert(openat(dirfd, "d1/../..", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > + > + assert(openat(dirfd, "../testdir/d1/f1", O_RDONLY) < 0); > + assert(errno == ENOTCAPABLE); > +} > + > +ATF_TC_BODY(lookup_cap_dotdot__negative, tc) > +{ > > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, "..", O_RDONLY) < 0); > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, "d1/../..", O_RDONLY) < 0); > - ATF_REQUIRE_ERRNO(ENOTCAPABLE, openat(dirfd, "../testdir/d1/f1", O_RDONLY) < 0); > + run_capsicum_test(lookup_cap_dotdot__negative_child); > } > > ATF_TP_ADD_TCS(tp) >