From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 51E76606A31A for ; Tue, 20 Oct 2026 01:41:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=bb5wacps; dkim=fail reason="signature verification failed" (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=Wti6IN8A DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Mzd/eiq38qEd5nlYTeSogt/6qSI6Km42nRwb5VZIUuc=; b=bb5wacpsVyWhkk vmHHj5i+GTR6sJzZ4YHGz9WuyyQVF1XBsRbGUTxdRjoki3KragoBRcZuAHzuPRMuggX2YoAGBhLKT /4a7XXTIua1DC7HwsbUre8WvxFRp1G+IsNTG3axwcRz92xDtK4cnxeobhfLnC6S7wYakfeSC01pKt cKLoYaJEIQ7hANf7nijSMwawUisXtswoe2LdwycerZTRXk8tD4se/URx48mT36iosgbKGdwhvQ8cm Gkq5SJh5VgwxuXCM6UyHtZyis40Hpx9OfnnIX/QFTZaVrfQbJpAHEHL8X2+b6LlkT5MF516MsRzZl ybLdnfFJ55YnLkKHvRqg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xIyrD-00000008de4-28US; Tue, 20 Oct 2026 01:41:39 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xIyrC-00000008ddf-1QiH for linux-riscv@lists.infradead.org; Tue, 20 Oct 2026 01:41:38 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id C4127601FD; Tue, 20 Oct 2026 01:41:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BABE71F000CB; Tue, 20 Oct 2026 01:41:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1792460497; bh=jV9vQbp8kVwiR6PyktkkrAbRYjtNNONjs0XwKEZKtBY=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=Wti6IN8AaYX6nR1a4nrFjdTh15ygN7WfDZAQ9TLe061bRkGIkDilOcPLyKPzmDSsi k7rXChH/oGTKJN73CSQTcKxofRqOXRonR7NKa7RfPQcKA03BjsGf4rvqOWFSrvuuTh fK3mTNgzah2xKFQa59Ayk/4o6eDa45k2rRhvEigTolOfnZvIUn2w1RoIdXsnlcXnfA hsKPyaVxBDfbYi4AzaKkZCN0x579dx7rLqtw34VnyFvQLOyncVCXFv/sQdRgzlvfMj p2DLgTI+Djf803DsIJ20+9t5/9hSmcI3yX6VwNbYuY7Cyr13ZG5mBDDznnuwUYFd+q ZllEwGVlNnECw== Date: Tue, 20 Oct 2026 09:21:35 +0800 From: Jisheng Zhang To: Nickolai Zeldovich Cc: Paul Walmsley , Palmer Dabbelt , Albert Ou , Alexandre Ghiti , Haorong Lu , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1] riscv: signal: check the full syscall return value for a restart Message-ID: References: <20261018024215.798849-1-nickolai@csail.mit.edu> X-Archived-By: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20261018024215.798849-1-nickolai@csail.mit.edu> X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" X-Spam-Status: No, score=-14.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=4.0.1 X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on aws-us-west-2-korg-listserv-alma10-1 On Sat, Oct 17, 2026 at 10:42:15PM -0400, Nickolai Zeldovich wrote: > A system call that returns a 64-bit value whose low 32 bits happen to > equal one of the -ERESTART* codes is restarted or turned into -EINTR > when a signal is pending on the way back to user space. lseek() returns > the new file offset, so lseek(fd, 0x1fffffe00, SEEK_CUR) followed by a > signal with an SA_RESTART handler runs the lseek() a second time and > leaves the offset at 0x3fffffc00; without SA_RESTART the call fails with > EINTR after it has moved the offset. With a SIGALRM interval timer of > 20 us, 2337 out of 2000000 such lseek() calls moved the offset twice and > 2695 out of 2000000 returned EINTR. brk() can return an arbitrary > address the same way, and a value ending in 0xfffffdfc goes through > restart_syscall() with whatever restart_block.fn the task last set. > > arch_do_signal_or_restart() reads regs->a0 into an int, which drops the > upper half of the return value before it is compared with -ERESTARTSYS > and friends. The code before the restart handling was moved out of > handle_signal() switched on the unsigned long a0 and was not affected. > > Keep the return value in a long. Compat tasks see only the low 32 bits > of a0, and a 32-bit tracer writes it zero-extended, so sign-extend it > from 32 bits for them as before. > > Tested with the program in the notes on QEMU virt (riscv64 defconfig): > the offset is moved exactly once and no EINTR is returned with the patch. > > Fixes: ce4f78f1b53d ("riscv: signal: handle syscall restart before get_signal") > Assisted-by: LLM > Signed-off-by: Nickolai Zeldovich Good catch! Reviewed-by: Jisheng Zhang > --- > > Notes: > Reproducer (static riscv64 binary run as /init's child on QEMU virt, -smp 2; > riscv64-linux-gnu-gcc -O2 -static -o lseek_restart lseek_restart.c): > > #define _GNU_SOURCE > #include > #include > #include > #include > #include > #include > #include > > #define OFF 0x1fffffe00LL /* low 32 bits read as -ERESTARTSYS */ > > static volatile long nsig; > > static void handler(int sig) > { > nsig++; > } > > static void run(int fd, int flags, const char *name) > { > struct sigaction sa; > struct itimerval it = { { 0, 20 }, { 0, 20 } }; > long i, moved = 0, eintr = 0; > > memset(&sa, 0, sizeof(sa)); > sa.sa_handler = handler; > sa.sa_flags = flags; > nsig = 0; > sigaction(SIGALRM, &sa, NULL); > setitimer(ITIMER_REAL, &it, NULL); > > for (i = 0; i < 2000000; i++) { > off_t r; > > lseek(fd, 0, SEEK_SET); > r = lseek(fd, OFF, SEEK_CUR); > if (r == OFF) > continue; > if (r == -1 && errno == EINTR) > eintr++; > else if (r == 2 * OFF) > moved++; > else > printf("%s: unexpected %#llx\n", name, (long long)r); > } > memset(&it, 0, sizeof(it)); > setitimer(ITIMER_REAL, &it, NULL); > printf("%s: %ld signals, %ld calls moved the offset twice, %ld returned EINTR\n", > name, nsig, moved, eintr); > } > > int main(void) > { > int fd = open("/lseek-test", O_RDWR | O_CREAT, 0600); > > if (fd < 0) { > perror("open"); > return 2; > } > run(fd, SA_RESTART, "SA_RESTART"); > run(fd, 0, "no SA_RESTART"); > return 0; > } > > Before: > SA_RESTART: 283700 signals, 2337 calls moved the offset twice, 0 returned EINTR > no SA_RESTART: 246693 signals, 0 calls moved the offset twice, 2695 returned EINTR > After: > SA_RESTART: 306854 signals, 0 calls moved the offset twice, 0 returned EINTR > no SA_RESTART: 278090 signals, 0 calls moved the offset twice, 0 returned EINTR > > arch/riscv/kernel/signal.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c > index 59784dc117e4..b1e92b969d96 100644 > --- a/arch/riscv/kernel/signal.c > +++ b/arch/riscv/kernel/signal.c > @@ -499,7 +499,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) > void arch_do_signal_or_restart(struct pt_regs *regs) > { > unsigned long continue_addr = 0, restart_addr = 0; > - int retval = 0; > + long retval = 0; > struct ksignal ksig; > bool syscall = (regs->cause == EXC_SYSCALL); > > @@ -508,6 +508,8 @@ void arch_do_signal_or_restart(struct pt_regs *regs) > continue_addr = regs->epc; > restart_addr = continue_addr - 4; > retval = regs->a0; > + if (is_compat_task()) > + retval = (int)retval; > > /* Avoid additional syscall restarting via ret_from_exception */ > regs->cause = -1UL; > > base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da > -- > 2.53.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv