From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from todd.t-8ch.de (todd.t-8ch.de [159.69.126.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 521E51F131F for ; Tue, 21 Jan 2025 12:48:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=159.69.126.157 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737463713; cv=none; b=X4LEdU4okjbS1cOD7m1UqixqHQhywatsG67UEZvqmV8YYQ9khIMkxd7R36qAEx4bDvTdC7Bd0a0gZzhXM2dTIJ4Bj0tTtbri/gb1hW6Dd1qSQSrGZqtPCZAIWKAYUcb90HdGRzN/m6u0zW2eU9QfoIhDr9DQbRKU+aOxan/YPe8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737463713; c=relaxed/simple; bh=5fR+5+EG2iedZZYXcSF42Jxgx31EaZt+d2wP8roXJgo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KNPbhrMzjZ7R1+/Bmlu6ZJiw0HKdHMl/ZuI9gkYk6Jg8CE3Z3T/vU8RYjlxoCTutp5j+7qZxqRZXMtfpqVkbwUev/PfKiduJ+wNIT14jAJVboVZML4plTHCQwZtz9bELhbnS25qmtKJfgXbAJ035xtCA9BMDwam4mHDIXjX5/Go= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weissschuh.net; spf=pass smtp.mailfrom=weissschuh.net; dkim=pass (1024-bit key) header.d=weissschuh.net header.i=@weissschuh.net header.b=L/0Fx13m; arc=none smtp.client-ip=159.69.126.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weissschuh.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=weissschuh.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=weissschuh.net header.i=@weissschuh.net header.b="L/0Fx13m" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=weissschuh.net; s=mail; t=1737463704; bh=5fR+5+EG2iedZZYXcSF42Jxgx31EaZt+d2wP8roXJgo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=L/0Fx13moJ68uEMwuO4myzJdJ2BAghNt2yHmcCGJOBq2rix2/npXMrGQ6qyyYMTZk t8Fo9rCS50n8nE/Mmznb2y5o7EC64NyN9mACb1VDffTjACFyV8oCINU2wPJpWqfUUc wcdIYwDDpeuVnvLxw7WWLiyezprRPpSpEQz0Vqro= Date: Tue, 21 Jan 2025 13:48:23 +0100 From: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= To: Cyrill Gorcunov Cc: LKML , Anna-Maria Behnsen , Frederic Weisbecker , Thomas Gleixner Subject: Re: [PATCH] posix-clock: drop code duplication using compat_ptr_ioctl Message-ID: References: <4229b992-77ef-404d-b045-8f1ccad1035a@t-8ch.de> <31acd2d0-8384-4ec5-acaa-187323b1968d@t-8ch.de> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Hi Cyrill! On 2025-01-21 09:48:28+0300, Cyrill Gorcunov wrote: > On Mon, Jan 20, 2025 at 11:41:26PM +0100, Thomas Weißschuh wrote: > > > > > > > +#ifdef CONFIG_COMPAT > > > > +long ptp_compat_ioctl(struct posix_clock_context *pccontext, unsigned int cmd, > > > > + unsigned long arg) > > > > +{ > > > > + switch (cmd) { > > > > + case PTP_ENABLE_PPS: > > > > + case PTP_ENABLE_PPS2: > > > > + /* These take in scalar arg, do not convert */ > > > > + break; > > > > + default: > > > > + arg = (unsigned long)compat_ptr(arg); > > > > > > Here^^^ > > The key is to only call compat_ptr() on *pointers*. > > Scalars have to be passed through unmodified. > > For ptp_ioctl(), PTP_ENABLE_PPS and PTP_ENABLE_PPS2 take such scalars, > > which is why those two *can not* use compat_ptr(). > > compat_ptr_ioctl() however passes all arguments through compat_ptr(). > > Yeah, and the PTP_ENABLE_PPS/PTP_ENABLE_PPS2 consider `arg` as 0/1 flip-flop > so compat_ptr won't screw it. So I personally would rather stick with a more > simple code (taking into account that ptp is the only real underlied device > so far sitting in code for so long). It is valid to pass any value to these ioctls, not only booleans. On s390 the value 0x80000000 aka BIT(31) would interpreted as "true" by a native 32bit kernel and "false" by a 64bit kernel in compat mode. It's indeed an edge case. Personally I prefer the correct solution. > > Admittedly it's quite unlikely anybody would pass a value where it would > > make a difference in practice. But if we fix this now, it might as well > > be correct. > > Sure, I see your point. Thanks for comments! Let's see what the maintainers prefer :-) Thomas