From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8E83B285CB3 for ; Tue, 2 Dec 2025 10:51:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764672687; cv=none; b=Imf6DaNcObnDPmvRnyedHTu8R2+rBkKWrSmyMO4shxO7u6BTyXx5VWfHrWHmlbV9ccqBuxu6p27ji7DIFcAdblwnikneMWKWKzZVDU+zcgwdWBLQQA4tJyuLLZ/WAazF3xuXJG+YvWYNMCbUZvKAd2Qj88RmNhLb61ha6Ujz53E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764672687; c=relaxed/simple; bh=N93bf/KZw0h+VJH4mzQ3LTBQpyL19LRzVwWCH9yZjAk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sZ5dScjYPlSaY5TPIba+AV3yAgPNB8MrHeP0rcHnamrT1NamVX6QpNfYVAVRJJvBGL/tNfL/k9+FiDLKX+WPeXuZJBrgIratsf7CESJM3B+L9u0FRUuvbPbplW+kcHiEKUhg7YbwqoUIoNMOOHswUesnoUK+OaRh74Qix7KYga8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=D6v+npYt; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="D6v+npYt" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-47796a837c7so37749555e9.0 for ; Tue, 02 Dec 2025 02:51:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1764672684; x=1765277484; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=on3Kan0NbvHupUMJLZOKeYAcH+YqhDINUzUWKZMJC6Y=; b=D6v+npYtu8tgUo8IF0GU/JqlN7N4i5DXXhmNZTzGAKsHSxzETIR3B30LMAfq3G1UXe kA4OmL0Kx7ueMjNBolGgpUn3yNgOFMnYN4vNERJ7uD1ecOHO288oL0neIFWFJI+Rut/d 8qkDMHMx8pltxgl+L0kRvSqCQPMsfQGKcu7x+Gd6YXUvu6OeMlsd0HsfUwQZ4FvJDmbE oothnQEJ26tA0R4d6kxJmdCV6K+/gV1eRyDxSfcrNUSI2GSYnmTkHkhAUZ4GDSZuwwhm Pqul/opVAcNR2n9iPyLCB214zXy3Am34BgftrBuZWrSShCplOApn9iLOUKalXrcBDfZL b8Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764672684; x=1765277484; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=on3Kan0NbvHupUMJLZOKeYAcH+YqhDINUzUWKZMJC6Y=; b=LgkqMaKHhJK/ReBLgerQ1PzQLQeRx79qA/U0UKgpEv+ZK1BDCHn1G3CL6lSxQSz5uW C9zJA+gwjs4MiwkbuVQas0VF3117amlRulTCPhvRNXUdQzON8x1SeGF82ljDFF3ZVTxY 6aq6jVGxqHcqQq9UBtmyX2hqnJWJW11JSAt5NR2clJk4LEB6ex+yoTzcRpA499SiZ/ig FjT8x33WU+ROTTLPYqLbd06WiXOEa0WD9sop0mjqYzgbcpf6hPRd/+JhSiBBK2/gy9wp q1ZYRsMHKyfWH94Qcx2VBg/cqqqkIfbRMFyRayiawGx4gKhJjIAHiLNgmWWtMJWIM5Pp 0c0Q== X-Forwarded-Encrypted: i=1; AJvYcCVWr5JwLcrrOnBjl2JZTkJae9abvUdG6ZVKmPgNPtH16JOjhflCZk0KG2Rz8q6FC/pm73yMBzxUcUvqGXkQY6FPkr4=@vger.kernel.org X-Gm-Message-State: AOJu0YxN/9qiESeMkQFUgU4WMTOomPRBk5SZuq753o9qYkL/L5VvGlnL aI8vcP0YK2tVn8/fCScWIAbLzLCSTptzwW1Xgbagc8txhQVP5SncOqTQMLuVY195JA== X-Gm-Gg: ASbGnctNwoom0p71eEUeOQynF7eZdVpB7Rt3Qt4PWN80DJH7xj7lTWdR/5W72kYCGQG I9Q8DBNGsdWvNIKNemCzUVhcPYdkVCmaD0cmFItIDm/vV3yZa051r746ZAjiYxh6L+AB4hSvhKG 6GNZDtAQFGuVcID6eJo/97vxKC4DqKkoUULBmd3n/ZGLmbami7OOUrsxQq6rTvAo/HOJRFLMpGB TIpO57oIKMSuMvX8XV8CI6PwvtuutBrg4iuHaXpU2qE/Y4pWNlwXsyVq5W4RiGeKkVoDg/kd0dH Ab/PJkM5EGG1RpBEaeUiki7acVDzUxIKn72+P4CXRzQDY9Rsqc5em6GwNmsg6Ptgp+KZyKB3hxy UpdFSsYPM28Oi80FrxhgixKBdGxtnfw4eNUlbhGavmohBZIdeOehWXeY126GZiM9OUSch8Skp9k BNCfW7NfEpC+uwHyBzEHwTckUAMtZ6sKuWrAeBPaRje5OzPp9j3zX9WUmvSykw65LS8nE= X-Google-Smtp-Source: AGHT+IFfI32AxldlfwCLkGNskF/o6XCATWWX6oiTRLGmi8WmK+KLfPtB4E3WKWIeymgEdapC8W5o3Q== X-Received: by 2002:a05:600c:1f8f:b0:477:a0dd:b2af with SMTP id 5b1f17b1804b1-477c01ea502mr377572095e9.33.1764672683650; Tue, 02 Dec 2025 02:51:23 -0800 (PST) Received: from google.com (49.185.141.34.bc.googleusercontent.com. [34.141.185.49]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4790adc6f7bsm375078255e9.2.2025.12.02.02.51.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Dec 2025 02:51:23 -0800 (PST) Date: Tue, 2 Dec 2025 10:51:19 +0000 From: Matt Bobrowski To: Shuran Liu , g@google.com Cc: song@kernel.org, bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Zesen Liu , Peili Gao , Haoran Ni Subject: Re: [PATCH bpf v2 2/2] selftests/bpf: fix and consolidate d_path LSM regression test Message-ID: References: <20251202075441.1409-1-electronlsr@gmail.com> <20251202075441.1409-3-electronlsr@gmail.com> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Tue, Dec 02, 2025 at 05:30:15PM +0800, Shuran Liu wrote: > Hi Matt, > > Thanks a lot for the review and for re-sending your Reviewed-by tag. > > In the next version of the series I’ll add your > > Reviewed-by: Matt Bobrowski > > to the patch that introduces the new selftest, and I’ll also make sure to > remove /tmp/bpf_d_path_test in the test cleanup path as you suggested. SGTM. > I also noticed that the CI is currently failing due to the `#pragma unroll` > around the loop in prog_lsm(). Would you prefer that I simply drop the pragma > in the next version, given that the loop bound is small and constant anyway, > or is there a better way you’d recommend to handle this? Yeah, I don't think the use of this directive is required here given the iteration count is tiny. Alternatively, perhaps you could switch over to using a more BPF verifier preferred alternative (i.e. bpf_for() or better yet and simpler bpf_repeat())? > > On Tue, Dec 02, 2025 at 00:59:45AM -0800, Matt Bobrowski wrote： > > > > On Tue, Dec 02, 2025 at 03:54:41PM +0800, Shuran Liu wrote: > >> Add a regression test for bpf_d_path() when invoked from an LSM program. > >> The test attaches to the bprm_check_security hook, calls bpf_d_path() on > >> the binary being executed, and verifies that a simple prefix comparison on > >> the returned pathname behaves correctly after the fix in patch 1. > >> > >> To avoid nondeterminism, the LSM program now filters based on the > >> expected PID, which is populated from userspace before the test binary is > >> executed. This prevents unrelated processes that also trigger the > >> bprm_check_security LSM hook from overwriting test results. Parent and > >> child processes are synchronized through a pipe to ensure the PID is set > >> before the child execs the test binary. > >> > >> Per review feedback, the new test is merged into the existing d_path > >> selftest rather than adding new prog_tests/ or progs/ files. > >> > >> Co-developed-by: Zesen Liu > >> Signed-off-by: Zesen Liu > >> Co-developed-by: Peili Gao > >> Signed-off-by: Peili Gao > >> Co-developed-by: Haoran Ni > >> Signed-off-by: Haoran Ni > >> Signed-off-by: Shuran Liu > > > > Feel free to add: > > > > Reviewed-by: Matt Bobrowski > > > > >> --- > >> .../testing/selftests/bpf/prog_tests/d_path.c | 64 +++++++++++++++++++ > >> .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++ > >> 2 files changed, 97 insertions(+) > >> > >> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c > >> index ccc768592e66..2909ca3bae0f 100644 > >> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c > >> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c > >> @@ -195,6 +195,67 @@ static void test_d_path_check_types(void) > >> test_d_path_check_types__destroy(skel); > >> } > >> > >> +static void test_d_path_lsm(void) > >> +{ > >> + struct test_d_path *skel; > >> + int err; > >> + int pipefd[2]; > >> + pid_t pid; > >> + > >> + skel = test_d_path__open_and_load(); > >> + if (!ASSERT_OK_PTR(skel, "d_path skeleton failed")) > >> + return; > >> + > >> + err = test_d_path__attach(skel); > >> + if (!ASSERT_OK(err, "attach failed")) > >> + goto cleanup; > >> + > >> + /* Prepare the test binary */ > >> + system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :"); > > > > I'd much prefer if we also cleaned up after ourselves, but it's not > > that much of an issue I guess. > > > >> + if (!ASSERT_OK(pipe(pipefd), "pipe failed")) > >> + goto cleanup; > >> + > >> + pid = fork(); > >> + if (!ASSERT_GE(pid, 0, "fork failed")) { > >> + close(pipefd[0]); > >> + close(pipefd[1]); > >> + goto cleanup; > >> + } > >> + > >> + if (pid == 0) { > >> + /* Child */ > >> + char buf; > >> + > >> + close(pipefd[1]); > >> + /* Wait for parent to set PID in BPF map */ > >> + if (read(pipefd[0], &buf, 1) != 1) > >> + exit(1); > >> + close(pipefd[0]); > >> + execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL); > >> + exit(1); > >> + } > >> + > >> + /* Parent */ > >> + close(pipefd[0]); > >> + > >> + /* Update BPF map with child PID */ > >> + skel->bss->my_pid = pid; > >> + > >> + /* Signal child to proceed */ > >> + write(pipefd[1], "G", 1); > >> + close(pipefd[1]); > >> + > >> + /* Wait for child */ > >> + waitpid(pid, NULL, 0); > >> + > >> + ASSERT_EQ(skel->bss->called_lsm, 1, "lsm hook called"); > >> + ASSERT_EQ(skel->bss->lsm_match, 1, "lsm match"); > >> + > >> +cleanup: > >> + test_d_path__destroy(skel); > >> +} > >> + > >> void test_d_path(void) > >> { > >> if (test__start_subtest("basic")) > >> @@ -205,4 +266,7 @@ void test_d_path(void) > >> > >> if (test__start_subtest("check_alloc_mem")) > >> test_d_path_check_types(); > >> + > >> + if (test__start_subtest("lsm")) > >> + test_d_path_lsm(); > >> } > >> diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c > >> index 84e1f883f97b..7f65c282069a 100644 > >> --- a/tools/testing/selftests/bpf/progs/test_d_path.c > >> +++ b/tools/testing/selftests/bpf/progs/test_d_path.c > >> @@ -17,6 +17,8 @@ int rets_close[MAX_FILES] = {}; > >> > >> int called_stat = 0; > >> int called_close = 0; > >> +int called_lsm = 0; > >> +int lsm_match = 0; > >> > >> SEC("fentry/security_inode_getattr") > >> int BPF_PROG(prog_stat, struct path *path, struct kstat *stat, > >> @@ -62,4 +64,35 @@ int BPF_PROG(prog_close, struct file *file, void *id) > >> return 0; > >> } > >> > >> +SEC("lsm/bprm_check_security") > >> +int BPF_PROG(prog_lsm, struct linux_binprm *bprm) > >> +{ > >> + pid_t pid = bpf_get_current_pid_tgid() >> 32; > >> + char path[MAX_PATH_LEN] = {}; > >> + int ret; > >> + > >> + if (pid != my_pid) > >> + return 0; > >> + > >> + called_lsm = 1; > >> + ret = bpf_d_path(&bprm->file->f_path, path, MAX_PATH_LEN); > >> + if (ret < 0) > >> + return 0; > >> + > >> + { > >> + static const char target_dir[] = "/tmp/"; > >> + > >> +#pragma unroll > >> + for (int i = 0; i < sizeof(target_dir) - 1; i++) { > >> + if (path[i] != target_dir[i]) { > >> + lsm_match = -1; /* mismatch */ > >> + return 0; > >> + } > >> + } > >> + } > >> + > >> + lsm_match = 1; /* prefix match */ > >> + return 0; > >> +} > >> + > >> char _license[] SEC("license") = "GPL"; > >> -- > >> 2.52.0 >