Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp1881928pxb; Thu, 28 Oct 2021 11:50:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz/4OZ7C3QR8xsAb5ePTLBphNzGcvRuQ3LakI7L7BMTDxWxcNPyiVMBoQSor9fAboN2IS/g X-Received: by 2002:a17:90b:3a84:: with SMTP id om4mr14591803pjb.35.1635447017160; Thu, 28 Oct 2021 11:50:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1635447017; cv=none; d=google.com; s=arc-20160816; b=cHOlvarX3xR6rmEKUYEkrhvvsNLEPY+OZuGZaPHAES0GhdQk04mv4shBGlsGaDg2fh WBZP7p/pMAVoBjqewYCTIOj+4L8rAXW787Lnd5tosbQhsXMRdLN5yzifp/Kb1iyexoq1 iSuS5VX3adBPOulhI7KTp5h8xohDIy6Ju+nFUkbOV62saXAgerL77wOWcG65zoVvXZ0S 2pdnvRn+ACnhQ1oZq7jQoUmgfOJXQCzo7vQBrhTuTj3oAVOBNqiW5rjUlXj1nGFJ04LR wLd/rTrTidJyAi8ElzO9wiyCXyZQe4fYmncL8rHB7J+oz27z0cbO/LNovJbX/PP61GtT ZFzg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=OVU9szkq61aNLb2lvzFxnap0kX1k7Tq4ULR6zE1UQeA=; b=gyEbX11/UC/xVZczT3rdRu8W0GB7E9h84bw/049qt/Jm6qJzYK6x+Ija+scWOpmr8l uVJw3G5EAerGOh4/waN7oGD4R81Poj3mGmQkLHA//QvFtKWw3km/owaoJWCwbqEGk20l WZEPPcvLbsAhsTr5Uc/MyjFCDVpktV9HmDh3enuQNdsSn6+LdsBVwPkN/JxDiVX5rU+G EbLOx/y8zJNjQi+KXOXVPIVTEep3sX6xyS4BWAECNYNKheSaFaU2pCBTcNk0weKhdpKT vNehj974gjwRyYr8PtaObbrRBQJciSJzy23qchpFJN988z48cRWf9oaCdFSMV2EAibAE BeoA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=JhzxYSWI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id n13si6401059plh.370.2021.10.28.11.50.03; Thu, 28 Oct 2021 11:50:17 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=JhzxYSWI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230517AbhJ1SuS (ORCPT + 99 others); Thu, 28 Oct 2021 14:50:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34964 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231132AbhJ1SuR (ORCPT ); Thu, 28 Oct 2021 14:50:17 -0400 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF03CC061767 for ; Thu, 28 Oct 2021 11:47:49 -0700 (PDT) Received: by mail-pf1-x42d.google.com with SMTP id 187so6807060pfc.10 for ; Thu, 28 Oct 2021 11:47:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=OVU9szkq61aNLb2lvzFxnap0kX1k7Tq4ULR6zE1UQeA=; b=JhzxYSWISXsDoknoTCT5nmTH4RRJulSB1Z9lahh/ZTXx4d4gioMrgqswO8/VYuE+GL uTMcpMs9GnlYYgE8fyW79aM/M3HNcJR6IINXi2Cr2HpJoNnLX2FgIGH8po0lCth0yQ2M DkaEnizVNx0xEvzdLz5QigtPDn2z/jCMKZQZU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=OVU9szkq61aNLb2lvzFxnap0kX1k7Tq4ULR6zE1UQeA=; b=NhRnac4vfBwt3YZdcjMM01fWomO4r0S9r9+uykl93LiRoCdvz1+iayYpiLBsWMd4iT GprNousoizBRv2pySWEcSJT4Jo6+Lgl7zVZlGtmGN1rhtiFQPbqR6RDj73GwQ2o+eE8U Htq1oMg1KW42M/09QSabj2IeJ8L4A+pxC6XTllSfgdDrmIAEsvZ08ATJZEKgkLBLH+Gy Q/IevZzCf4lfgpiyj6ApmsbSsxgk4JsYfbQbRJaIv7dHFfm7ALiCiKhcslqmAc3WP/9o phggvkESlHSvneuDpu9Krpzzn1JGA5ta+7HgzPiJbn7R8DVbhlO/OUA1PpJt1cYp2e2G ixlA== X-Gm-Message-State: AOAM532rgt+kTAW9x4gW2zMO4bdV3dE681hMJusoQ5+kj5CirldMQOYA 2MiOZyS57ByfWBjEAZJLTZPn721+ZdzmnQ== X-Received: by 2002:aa7:8a0e:0:b0:47c:1116:3ce with SMTP id m14-20020aa78a0e000000b0047c111603cemr5964030pfa.76.1635446869394; Thu, 28 Oct 2021 11:47:49 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id ls6sm144127pjb.53.2021.10.28.11.47.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Oct 2021 11:47:49 -0700 (PDT) Date: Thu, 28 Oct 2021 11:47:48 -0700 From: Kees Cook To: "Eric W. Biederman" Cc: Andrea Righi , Shuah Khan , Alexei Starovoitov , Andy Lutomirski , Will Drewry , linux-kselftest@vger.kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: selftests: seccomp_bpf failure on 5.15 Message-ID: <202110281136.5CE65399A7@keescook> References: <202110280955.B18CB67@keescook> <878rydm56l.fsf@disp2133> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <878rydm56l.fsf@disp2133> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 28, 2021 at 12:26:26PM -0500, Eric W. Biederman wrote: > Kees Cook writes: > > > On Thu, Oct 28, 2021 at 06:21:12PM +0200, Andrea Righi wrote: > >> The following sub-tests are failing in seccomp_bpf selftest: > >> > >> 18:56:54 DEBUG| [stdout] # selftests: seccomp: seccomp_bpf > >> ... > >> 18:56:57 DEBUG| [stdout] # # RUN TRACE_syscall.ptrace.kill_after ... > >> 18:56:57 DEBUG| [stdout] # # seccomp_bpf.c:2023:kill_after:Expected entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY : PTRACE_EVENTMSG_SYSCALL_EXIT (1) == msg (0) > >> 18:56:57 DEBUG| [stdout] # # seccomp_bpf.c:2023:kill_after:Expected entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY : PTRACE_EVENTMSG_SYSCALL_EXIT (2) == msg (1) > >> 18:56:57 DEBUG| [stdout] # # seccomp_bpf.c:2023:kill_after:Expected entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY : PTRACE_EVENTMSG_SYSCALL_EXIT (1) == msg (2) > >> 18:56:57 DEBUG| [stdout] # # kill_after: Test exited normally instead of by signal (code: 12) > >> 18:56:57 DEBUG| [stdout] # # FAIL TRACE_syscall.ptrace.kill_after > >> ... > >> 18:56:57 DEBUG| [stdout] # # RUN TRACE_syscall.seccomp.kill_after ... > >> 18:56:57 DEBUG| [stdout] # # seccomp_bpf.c:1547:kill_after:Expected !ptrace_syscall (1) == IS_SECCOMP_EVENT(status) (0) > >> 18:56:57 DEBUG| [stdout] # # kill_after: Test exited normally instead of by signal (code: 0) > >> 18:56:57 DEBUG| [stdout] # # FAIL TRACE_syscall.seccomp.kill_after > >> 18:56:57 DEBUG| [stdout] # not ok 80 TRACE_syscall.seccomp.kill_after > >> ... > >> 18:56:57 DEBUG| [stdout] # # FAILED: 85 / 87 tests passed. > >> 18:56:57 DEBUG| [stdout] # # Totals: pass:85 fail:2 xfail:0 xpass:0 skip:0 error:0 > >> 18:56:57 DEBUG| [stdout] not ok 1 selftests: seccomp: seccomp_bpf # exit=1 > >> > >> I did some bisecting and found that the failures started to happen with: > >> > >> 307d522f5eb8 ("signal/seccomp: Refactor seccomp signal and coredump generation") > >> > >> Not sure if the test needs to be fixed after this commit, or if the > >> commit is actually introducing an issue. I'll investigate more, unless > >> someone knows already what's going on. > > > > Ah thanks for noticing; I will investigate... > > > I just did a quick read through of the test and while > I don't understand everything having a failure seems > very weird. > > I don't understand the comment: > /* Tracer will redirect getpid to getppid, and we should die. */ > > As I think what happens is it the bpf programs loads the signal > number. Tests to see if the signal number if GETPPID and allows > that system call and causes any other system call to be terminated. The test suite runs a series of seccomp filter vs syscalls under tracing, either with ptrace or with seccomp SECCOMP_RET_TRACE, to validate the expected behavioral states. It seems that what's happened is that the SIGSYS has suddenly become non-killing: # RUN TRACE_syscall.ptrace.kill_after ... # seccomp_bpf.c:1555:kill_after:Expected WSTOPSIG(status) & 0x80 (0) == 0x80 (128) # seccomp_bpf.c:1556:kill_after:WSTOPSIG: 31 # kill_after: Test exited normally instead of by signal (code: 12) # FAIL TRACE_syscall.ptrace.kill_after i.e. the ptracer no longer sees a dead tracee, which would pass through here: if (WIFSIGNALED(status) || WIFEXITED(status)) /* Child is dead. Time to go. */ return; So the above saw a SIG_TRAP|SIGSYS rather than a killing SIGSYS. i.e. instead of WIFSIGNALED(stauts) being true, it instead catches a PTRACE_EVENT_STOP for SIGSYS, which should be impossible (the process should be getting killed). > Which being single threaded would seem to cause the kernel to execute > the changed code. > > How there kernel at that point is having the process exit with anything > except SIGSYS I am not immediately seeing. I've run out of time at the moment to debug further, but I've appended my changes to the test, and a brute-force change to kernel/seccomp.c to restore original behavior (though I haven't tested if coredumping works still). I'll return to this in a few hours... > > The logic is the same as that for SECCOMP_RET_TRAP is there a test for > that, that is also failing? > > How do you run that test anyway? cd tools/testing/selftests/seccomp make seccomp_bpf scp seccomp_bpf target: ssh target ./seccomp_bpf diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 4d8f44a17727..b6c8c8f8bd69 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -1269,10 +1269,12 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd, syscall_rollback(current, current_pt_regs()); /* Trigger a coredump with SIGSYS */ force_sig_seccomp(this_syscall, data, true); - } else { - do_exit(SIGSYS); + do_group_exit(SIGSYS); } - return -1; /* skip the syscall go directly to signal handling */ + if (action == SECCOMP_RET_KILL_THREAD) + do_exit(SIGSYS); + else + do_group_exit(SIGSYS); } unreachable(); diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 1d64891e6492..8f8c1df885d6 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -1487,7 +1487,7 @@ TEST_F(precedence, log_is_fifth_in_any_order) #define PTRACE_EVENT_SECCOMP 7 #endif -#define IS_SECCOMP_EVENT(status) ((status >> 16) == PTRACE_EVENT_SECCOMP) +#define PTRACE_EVENT_MASK(status) ((status) >> 16) bool tracer_running; void tracer_stop(int sig) { @@ -1536,17 +1536,34 @@ void start_tracer(struct __test_metadata *_metadata, int fd, pid_t tracee, /* Run until we're shut down. Must assert to stop execution. */ while (tracer_running) { int status; + bool run_callback = true; if (wait(&status) != tracee) continue; + if (WIFSIGNALED(status) || WIFEXITED(status)) /* Child is dead. Time to go. */ return; - /* Check if this is a seccomp event. */ - ASSERT_EQ(!ptrace_syscall, IS_SECCOMP_EVENT(status)); + /* Check if we got an expected event. */ + ASSERT_EQ(WIFCONTINUED(status), false); + ASSERT_EQ(WIFSTOPPED(status), true); + ASSERT_EQ(WSTOPSIG(status) & SIGTRAP, SIGTRAP) { + TH_LOG("WSTOPSIG: %d", WSTOPSIG(status)); + } + if (ptrace_syscall) { + EXPECT_EQ(WSTOPSIG(status) & 0x80, 0x80) { + TH_LOG("WSTOPSIG: %d", WSTOPSIG(status)); + run_callback = false; + }; + } else { + EXPECT_EQ(PTRACE_EVENT_MASK(status), PTRACE_EVENT_SECCOMP) { + run_callback = false; + }; + } - tracer_func(_metadata, tracee, status, args); + if (run_callback) + tracer_func(_metadata, tracee, status, args); ret = ptrace(ptrace_syscall ? PTRACE_SYSCALL : PTRACE_CONT, tracee, NULL, 0); -- Kees Cook