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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DCFF6E77184 for ; Thu, 19 Dec 2024 21:30:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 69B926B007B; Thu, 19 Dec 2024 16:30:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6499F6B0082; Thu, 19 Dec 2024 16:30:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 511B76B0083; Thu, 19 Dec 2024 16:30:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 338AA6B007B for ; Thu, 19 Dec 2024 16:30:03 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id CADBDAEB6F for ; Thu, 19 Dec 2024 21:30:02 +0000 (UTC) X-FDA: 82913000352.21.71E5B0A Received: from flow-b5-smtp.messagingengine.com (flow-b5-smtp.messagingengine.com [202.12.124.140]) by imf20.hostedemail.com (Postfix) with ESMTP id 9BFDD1C000A for ; Thu, 19 Dec 2024 21:29:25 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=coelacanthus.name header.s=fm2 header.b=Mp1U2niv; dkim=pass header.d=messagingengine.com header.s=fm1 header.b="x EmPJhI"; spf=pass (imf20.hostedemail.com: domain of uwu@coelacanthus.name designates 202.12.124.140 as permitted sender) smtp.mailfrom=uwu@coelacanthus.name; dmarc=pass (policy=quarantine) header.from=coelacanthus.name ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734643763; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Ly5Y8jVk5moLHMzQ7W8exXV5U16sE/p4pCtMgqqBuII=; b=YIItys8A+42H8TQ+sZoBmkJyepqXFfHQLEGui2/j/yJZFOdKeEVf63emM2PMpH0FoQexhU WePqVQWLQYjFN8Hz/at5kiC06VNI3faT/vXvffvQdz0r/E+Q8gk/tpfIWUcec483nH+TI7 zNATSdboRBoA/fwt3kMwPk0AOyYce8k= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734643763; a=rsa-sha256; cv=none; b=5g5iLoJQiE+BjVPGVHY+r82DamARTsQVWenASaRaT3jdJy6fmcuZmYVz0GOaRfVKmiUQSZ HU0M9OHuCoNLsZvBiZC0AngfXYkaI0KYLXQ80Wn/75XAir6Cg3H7u/Vv3hJ7h3iToEoZ7K uHvMhTN6IDsnTk1QdFInYp2aT7gQUDk= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=coelacanthus.name header.s=fm2 header.b=Mp1U2niv; dkim=pass header.d=messagingengine.com header.s=fm1 header.b="x EmPJhI"; spf=pass (imf20.hostedemail.com: domain of uwu@coelacanthus.name designates 202.12.124.140 as permitted sender) smtp.mailfrom=uwu@coelacanthus.name; dmarc=pass (policy=quarantine) header.from=coelacanthus.name Received: from phl-compute-05.internal (phl-compute-05.phl.internal [10.202.2.45]) by mailflow.stl.internal (Postfix) with ESMTP id F41541D40584; Thu, 19 Dec 2024 16:29:58 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Thu, 19 Dec 2024 16:29:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= coelacanthus.name; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1734643798; x=1734650998; bh=Ly5Y8jVk5m oLHMzQ7W8exXV5U16sE/p4pCtMgqqBuII=; b=Mp1U2nivsGGD80upLCLOGHvbCr oaNvhOH+NhMVGOszjBPvJhb6rmQhba0k4N8psWIMmOJgi2Gzr+DZm4H/V0yLNcT2 qVKzRnf+wO2rjJijf55cmphYq4S5Ry8NXL9r5OUQaJYh3Dw88QmP+RrzC8itNZf7 diiAHjmvtVns58TL/rjl0/Ai3mmrm+Jtb7i53WHt/UjRlHIjuFz0x0n/LzTn2Hrc mEFZqTh2Isx9h6uotGuqG6AVsnL9rdd7lSAzv2IDLoZeQ/m4iguMEjwqmui/p/y1 6fvd188JCzi4T5MkuhVv9J3t7dQflCoJAxg7wIfTvgKZOwxb6T6UkahIkZ2w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1734643798; x= 1734650998; bh=Ly5Y8jVk5moLHMzQ7W8exXV5U16sE/p4pCtMgqqBuII=; b=x EmPJhIPrE3uJpeIijsDG14VGPg2S5j5sejI3Me1x8kIQ5A9O6yotGXCxb13QC5bw 7e/sq7OuuAxiPa+thcmElCqcO8UGrZ6kJpbKmGjt6S74FpkLlI+Iy3K5fqCVoybZ RDuQYtnZ4hqoWIHKQsQKTB6O9QOM62ARCXzfdrU9g57qzqOH1OvUVpJEHTmRk+wp VDeBzID0JlxGKrScMarxGTQSb3l/RWH+mha9fzwtN4DRFjDqFV6uo6KJgZBJVFyH UAXpEQme1vBoIN+Gdw4UZrfXck8fk1pzNraKXbnttjUDze4n9SoGFv14ZU5h57qL WFGtuWK4ggTwHXcqt099g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddruddttddgudegjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg hnthhsucdlqddutddtmdenucfjughrpefkffggfgfuvfevfhfhjggtgfesthejredttddv jeenucfhrhhomhepvegvlhgvshhtvgcunfhiuhcuoehufihusegtohgvlhgrtggrnhhthh hushdrnhgrmhgvqeenucggtffrrghtthgvrhhnpeefgfdvgfetveevkeehleffteekvdfh vedvheduueehkedvuedvhfeiieejudekueenucevlhhushhtvghrufhiiigvpedtnecurf grrhgrmhepmhgrihhlfhhrohhmpehufihusegtohgvlhgrtggrnhhthhhushdrnhgrmhgv pdhnsggprhgtphhtthhopedvvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheptg hhrghrlhhivgesrhhivhhoshhinhgtrdgtohhmpdhrtghpthhtoheprghjohhnvghssehv vghnthgrnhgrmhhitghrohdrtghomhdprhgtphhtthhopeholhgvghesrhgvughhrghtrd gtohhmpdhrtghpthhtohepphgruhhlrdifrghlmhhslhgvhiesshhifhhivhgvrdgtohhm pdhrtghpthhtohepphgrlhhmvghrsegurggssggvlhhtrdgtohhmpdhrtghpthhtoheprg houhesvggvtghsrdgsvghrkhgvlhgvhidrvgguuhdprhgtphhtthhopegvsghivgguvghr mhesgihmihhsshhiohhnrdgtohhmpdhrtghpthhtohepkhgvvghssehkvghrnhgvlhdroh hrghdprhgtphhtthhopehshhhurghhsehkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: i95c648bc:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 19 Dec 2024 16:29:49 -0500 (EST) Message-ID: <50a62291-5ae1-47b0-8f64-a42271820789@coelacanthus.name> Date: Fri, 20 Dec 2024 05:29:45 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/2] riscv: selftests: Add a ptrace test to verify syscall parameter modification Content-Language: en-GB-large To: Charlie Jenkins , Andrew Jones Cc: Oleg Nesterov , Paul Walmsley , Palmer Dabbelt , Albert Ou , Eric Biederman , Kees Cook , Shuah Khan , Alexandre Ghiti , "Dmitry V. Levin" , Andrea Bolognani , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , Thomas Gleixner , Ron Economos , Quan Zhou , Guo Ren , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, stable@vger.kernel.org, linux-kselftest@vger.kernel.org References: <20241203-riscv-new-regset-v2-0-d37da8c0cba6@coelacanthus.name> <20241203-riscv-new-regset-v2-2-d37da8c0cba6@coelacanthus.name> <20241203-55c76715e16422ddaf8d7edf@orel> From: Celeste Liu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 9BFDD1C000A X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: xadd4z59ezqbaxt3okar8w3pzfxe1taz X-HE-Tag: 1734643765-649035 X-HE-Meta: U2FsdGVkX1/CyUR3KuKusyC38isRMnLh+yQ84u28NGsVCMSQ+tjH/vX8hxo+bakIG5SnfvSOIbs4FpJ2RG6EMdAcpE+APam0z7pB4FA7JKouUXqecPI+9uKP/VViYVtgz3eiUcZHGbNz8YDrH4xL+K28J8l0Ektmrhbp3PZPPy20CV+qZXk2j2pUkXRfh42LgDHi8uNzEQ2U9HWtyJ0fj9hBLv5aGjKEfkd4R/TnTiLALPS9Mjb89VGoVzzWYNY61li/Rsg1b+DLUDJwP0hr+xHT9u0Ts3oP4W2HUrwdUpedPuDsIIHTrn9nY2ihWVIbiqF/7j+W8nySgz1UryKKfowmK7KGyXQVe5HG4nhJ1oZ/k073N+grca8tn6bIwcm+BY0oWJmVvrhlxPzTlxV06OLi35uCv3fgDm+94SIRCK6UX5R4lnajctzo5XBTAMR+EBdXleYo1zM1ckcw43fZshOKixZpHoc9Ek+QXAdiAahVlF/go/xjrW+sLlOBDOle1Bij5nIxekHKHbPbNrgug8vvMgZD7N60Fon5yjwIBKPZt9WWbf40v7p+QoCQ2hz3XPp5ihWU30WSKkUDbBgT+W5uvdBCqeizvv7mvuDr9nymRTM9/3s8phoM1N8x+EPF2RYd32dk7LfX3jTcVvuG1vJWC6g89xZC5lmjWl33Vjto2c0L2mh/GoAx8qVL0qLlUycPSRoq68JZvVhy3VYHZMYsQV8U/p96/GfvXsLvqlBp86svf9W3LLfEE4BVUTts5YEXu9Fmmu5RWxRO6M8kucOtJCZZDfzdLRylrwU4xdkrwRZdg4W82+iOOQWaL+XZxZXlphzw08qObEcGqvHkyr/AxJV2+qU2GhUsKFaqH0bGSZ0KC98flABB/G0a1IiaGtUs6decnHYubCVRO+heUC1eprLj18OAu6D3GeTDLGyKDT/EPpYQYcIKu1kcLbH2tQFa0jCbt1XESr1iMOY CuVlVWJf Po/Jiw958h2g4cweG8ZJA9eJ7d61gt5NYjZLqcQsFTCy3lUC0+9eHM3LOIR0zEmP6ET1oitThcCEfwzEtXcWmN34EHqjcCc5YOQ/fPIxmixDWHW6r3PTRagasR2mfFByxFQjTL5LI/k3dLAeMy15US1pnApykNfeIK5I6N9ZclWQh9703fSyR9itgS7vTGD+kfs1w39t2tNCpQFf8vZ2rDiUnsXTAwZb8GeVCqglrVPA4ek99okNf2vCAcEhHs8gyEiovO6K20xnIE3uPqy3LdXbG4MDdWxR1jogiJt3+OMsDq2jDVxy+JKGMm/yvqjUi3k7wX/WxrMeaqf3aEF+1CKOvh2AL6B0jzxb3g1X3zxkzAQFu/12ArrCWVUeA4+fsjYNNknOZZ1uGIvPi26yd5RJCAXNCarquf2lPhm5WUEMPLAQd3e8+RsXlkUQRzp27yxLOny5dbGYxsgLA365Lo8o/VaIDN4uMVuH4D/hzTYjk/G4Wxn6/Oes8cAkUjvv+x/YDxT6JKiPn3lnP+i2LdRWDcJ0BgTT6UdzM57wOxHNpFEMWUnboNS/1xlYaUMWaNxVz X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2024-12-20 02:26, Charlie Jenkins wrote: > On Tue, Dec 03, 2024 at 01:55:07PM +0100, Andrew Jones wrote: >> On Tue, Dec 03, 2024 at 05:30:05PM +0800, Celeste Liu wrote: >>> From: Charlie Jenkins >>> >>> This test checks that orig_a0 allows a syscall argument to be modified, >>> and that changing a0 does not change the syscall argument. >>> >>> Cc: stable@vger.kernel.org >>> Co-developed-by: Quan Zhou >>> Signed-off-by: Quan Zhou >>> Co-developed-by: Celeste Liu >>> Signed-off-by: Celeste Liu >>> Signed-off-by: Charlie Jenkins >>> --- >>> tools/testing/selftests/riscv/abi/.gitignore | 1 + >>> tools/testing/selftests/riscv/abi/Makefile | 5 +- >>> tools/testing/selftests/riscv/abi/ptrace.c | 134 +++++++++++++++++++++++++++ >>> 3 files changed, 139 insertions(+), 1 deletion(-) >>> >>> diff --git a/tools/testing/selftests/riscv/abi/.gitignore b/tools/testing/selftests/riscv/abi/.gitignore >>> index b38358f91c4d2240ae64892871d9ca98bda1ae58..378c605919a3b3d58eec2701eb7af430cfe315d6 100644 >>> --- a/tools/testing/selftests/riscv/abi/.gitignore >>> +++ b/tools/testing/selftests/riscv/abi/.gitignore >>> @@ -1 +1,2 @@ >>> pointer_masking >>> +ptrace >>> diff --git a/tools/testing/selftests/riscv/abi/Makefile b/tools/testing/selftests/riscv/abi/Makefile >>> index ed82ff9c664e7eb3f760cbab81fb957ff72579c5..3f74d059dfdcbce4d45d8ff618781ccea1419061 100644 >>> --- a/tools/testing/selftests/riscv/abi/Makefile >>> +++ b/tools/testing/selftests/riscv/abi/Makefile >>> @@ -2,9 +2,12 @@ >>> >>> CFLAGS += -I$(top_srcdir)/tools/include >>> >>> -TEST_GEN_PROGS := pointer_masking >>> +TEST_GEN_PROGS := pointer_masking ptrace >>> >>> include ../../lib.mk >>> >>> $(OUTPUT)/pointer_masking: pointer_masking.c >>> $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ >>> + >>> +$(OUTPUT)/ptrace: ptrace.c >>> + $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^ >>> diff --git a/tools/testing/selftests/riscv/abi/ptrace.c b/tools/testing/selftests/riscv/abi/ptrace.c >>> new file mode 100644 >>> index 0000000000000000000000000000000000000000..d192764b428d1f1c442f3957c6fedeb01a48d556 >>> --- /dev/null >>> +++ b/tools/testing/selftests/riscv/abi/ptrace.c >>> @@ -0,0 +1,134 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include "../../kselftest_harness.h" >>> + >>> +#define ORIG_A0_MODIFY 0x01 >>> +#define A0_MODIFY 0x02 >>> +#define A0_OLD 0x03 >>> +#define A0_NEW 0x04 >> >> Shouldn't A0_OLD and A0_NEW set more bits, since 3 and 4 aren't very >> unique (we could have those values by accident)? And should we include >> setting bits over 31 for 64-bit targets? >> >>> + >>> +#define perr_and_exit(fmt, ...) \ >>> + ({ \ >>> + char buf[256]; \ >>> + snprintf(buf, sizeof(buf), "%s:%d:" fmt ": %m\n", \ >>> + __func__, __LINE__, ##__VA_ARGS__); \ >>> + perror(buf); \ >>> + exit(-1); \ >>> + }) >> >> Can we use e.g. ksft_exit_fail_perror() instead? I'd prefer we try to >> consolidate testing/selftests/riscv/* tests on a common format for >> errors and exit codes and we're already using other kselftest stuff. >> >>> + >>> +static inline void resume_and_wait_tracee(pid_t pid, int flag) >>> +{ >>> + int status; >>> + >>> + if (ptrace(flag, pid, 0, 0)) >>> + perr_and_exit("failed to resume the tracee %d\n", pid); >>> + >>> + if (waitpid(pid, &status, 0) != pid) >>> + perr_and_exit("failed to wait for the tracee %d\n", pid); >>> +} >>> + >>> +static void ptrace_test(int opt, int *result) >>> +{ >>> + int status; >>> + pid_t pid; >>> + struct user_regs_struct regs; >>> + struct iovec iov = { >>> + .iov_base = ®s, >>> + .iov_len = sizeof(regs), >>> + }; >>> + >>> + unsigned long orig_a0; >>> + struct iovec a0_iov = { >>> + .iov_base = &orig_a0, >>> + .iov_len = sizeof(orig_a0), >>> + }; >>> + >>> + pid = fork(); >>> + if (pid == 0) { >>> + /* Mark oneself being traced */ >>> + long val = ptrace(PTRACE_TRACEME, 0, 0, 0); >>> + >>> + if (val) >>> + perr_and_exit("failed to request for tracer to trace me: %ld\n", val); >>> + >>> + kill(getpid(), SIGSTOP); >>> + >>> + /* Perform exit syscall that will be intercepted */ >>> + exit(A0_OLD); >>> + } >>> + >>> + if (pid < 0) >>> + exit(1); >> >> This unexpected error condition deserves a message, so I'd use >> ksft_exit_fail_perror() here. >> >>> + >>> + if (waitpid(pid, &status, 0) != pid) >>> + perr_and_exit("failed to wait for the tracee %d\n", pid); >>> + >>> + /* Stop at the entry point of the syscall */ >>> + resume_and_wait_tracee(pid, PTRACE_SYSCALL); >>> + >>> + /* Check tracee regs before the syscall */ >>> + if (ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov)) >>> + perr_and_exit("failed to get tracee registers\n"); >>> + if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov)) >>> + perr_and_exit("failed to get tracee registers\n"); >>> + if (orig_a0 != A0_OLD) >>> + perr_and_exit("unexpected orig_a0: 0x%lx\n", orig_a0); >>> + >>> + /* Modify a0/orig_a0 for the syscall */ >>> + switch (opt) { >>> + case A0_MODIFY: >>> + regs.a0 = A0_NEW; >>> + break; >>> + case ORIG_A0_MODIFY: >>> + orig_a0 = A0_NEW; >>> + break; >>> + } >>> + >>> + if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_ORIG_A0, &a0_iov)) >>> + perr_and_exit("failed to set tracee registers\n"); >>> + >>> + /* Resume the tracee */ >>> + ptrace(PTRACE_CONT, pid, 0, 0); >>> + if (waitpid(pid, &status, 0) != pid) >>> + perr_and_exit("failed to wait for the tracee\n"); >>> + >>> + *result = WEXITSTATUS(status); >>> +} >>> + >>> +TEST(ptrace_modify_a0) >>> +{ >>> + int result; >>> + >>> + ptrace_test(A0_MODIFY, &result); >>> + >>> + /* The modification of a0 cannot affect the first argument of the syscall */ >>> + EXPECT_EQ(A0_OLD, result); >> >> What about checking that we actually set regs.a0 to A0_NEW? We'd need >> A0_NEW to be more unique than 4, though. >> >>> +} >>> + >>> +TEST(ptrace_modify_orig_a0) >>> +{ >>> + int result; >>> + >>> + ptrace_test(ORIG_A0_MODIFY, &result); >>> + >>> + /* Only modify orig_a0 to change the first argument of the syscall */ >> >> If we run ptrace_modify_a0 first then we've already set regs.a0 to A0_NEW >> and can't check with this test that we don't set it to A0_NEW. We should >> probably have two different test values, one for regs.a0 and one for >> orig_a0 and ensure on both tests that we aren't writing both. >> > > Celeste, do you want to fix this up or are you waiting for me to? Sorry for delay. I was busy with household affairs in the past few weeks. v3 will be sent tomorrow or the day after tomorrow. I am deeply sorry for this. > > - Charlie > >>> + EXPECT_EQ(A0_NEW, result); >>> +} >>> + >>> +TEST_HARNESS_MAIN >>> >>> -- >>> 2.47.0 >>> >> >> Thanks, >> drew