From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vmicros1.altlinux.org (vmicros1.altlinux.org [194.107.17.57]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B67323596D for ; Mon, 27 Jan 2025 11:13:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=194.107.17.57 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737976400; cv=none; b=Gs9NdQp2TwCMf7ocJcfWkBZ2N1MX+uVYNTK6gVkjxVrUpTsjQSjJSVsOfO55PVibgjGL7etaAJjib38VhNGjP6EwWcAahJULsX+DmlYulIg8ehC2TsMJVrKG4AHJ1iTlrz9Nu7GQbsUzPop0pwPMM0jzOdd6ck8MB7jwSdWZTtk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737976400; c=relaxed/simple; bh=6xQfzPJ/uChgdrN8Ta/Cpz5CYzj01+MXR0RawGfXLY8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=K357K9P9jojWTeFxgKxI1Lw2JQWhrC5cC7SMe5HmkzaL8GlC4sJeNGKbPJhHnz6FCVIWNn25sNjysx2CphtHPZyKZuz6RjhvetCsNRXI7XYo9l6YsU+PsFXe2ezSq/30s+o8/8n4FkOCY30kJy1Xufj606TIohhrHJV8Nd/R+Dk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=strace.io; spf=pass smtp.mailfrom=altlinux.org; arc=none smtp.client-ip=194.107.17.57 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=strace.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=altlinux.org Received: from mua.local.altlinux.org (mua.local.altlinux.org [192.168.1.14]) by vmicros1.altlinux.org (Postfix) with ESMTP id D10AF72C8CC; Mon, 27 Jan 2025 14:13:10 +0300 (MSK) Received: by mua.local.altlinux.org (Postfix, from userid 508) id C56D77CCB3A; Mon, 27 Jan 2025 13:13:10 +0200 (IST) Date: Mon, 27 Jan 2025 13:13:10 +0200 From: "Dmitry V. Levin" To: Michael Ellerman Cc: Alexey Gladkov , Christophe Leroy , Oleg Nesterov , Eugene Syromyatnikov , Mike Frysinger , Renzo Davoli , Davide Berardi , strace-devel@lists.strace.io, Madhavan Srinivasan , Nicholas Piggin , Naveen N Rao , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value() Message-ID: <20250127111310.GA29522@strace.io> References: <20250113171054.GA589@strace.io> <6558110c-c2cb-4aa3-9472-b3496f71ebb8@csgroup.eu> <20250114170400.GB11820@strace.io> <20250123182815.GA20994@strace.io> <20250123234321.GA23582@strace.io> <87ikq3jddt.fsf@mpe.ellerman.id.au> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ikq3jddt.fsf@mpe.ellerman.id.au> On Sat, Jan 25, 2025 at 11:18:06PM +1100, Michael Ellerman wrote: > Alexey Gladkov writes: > > > ... > > I'm not a powerpc expert but shouldn't be used regs->gpr[3] via a > > regs_return_value() in system_call_exception() ? > > Yes I agree. > > > notrace long system_call_exception(struct pt_regs *regs, unsigned long r0) > > { > > ... > > r0 = do_syscall_trace_enter(regs); > > if (unlikely(r0 >= NR_syscalls)) > > return regs->gpr[3]; > > This is the case where we're expecting the r3 value to be a negative > error code, to match the in-kernel semantics. But after this change it > would be a positive error value. It is probably harmless with the > current code structure, but that's just luck. I'm afraid that's not just luck. do_seccomp() from the very beginning supports both the generic kernel -ERRORCODE return value ABI and the powerpc sc syscall return ABI, thanks to syscall_exit_prepare() that converts the former to the latter. Given that this inconsistency was exposed to user space via PTRACE_EVENT_SECCOMP tracers for so many years, I suppose backwards compatibility has to be provided. Consequently, since the point of __secure_computing() invocation and up to the point of conversion in syscall_exit_prepare(), gpr[3] may be set according to either of these two ABIs. Unfortunately, this means any future attempt to avoid the inconsistency would be inherently incomplete. For this reason, I doubt it would make sense to include into the patch any changes that are needed only to address this consistency issue. -- ldv