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 EF9231ADFF8 for ; Mon, 20 Jan 2025 22:41:29 +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=1737412891; cv=none; b=KAmbmBz3MnA/REyqzGKguo0L4k6drDkCpBbsyTAvarOuN8TDjZQpjvZEyPh4NHhOFjTO5iRMzlUhJkMIJhQZWR5/KPl5k2c1/BpT71tGzqlsF6+HK6hCkRAa3NV7kEIQh8QUdF1+fe5CRBOKIMwj/Or0N2Q1rX5TFj5w+HI4tT0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737412891; c=relaxed/simple; bh=sbkeC/YPODPeyqE7bpJJX+VlF53491jAs4WmSbiBrBg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AjeNwWmjv8W2SnbTxFcZwj+06WIN4g9I2C4DnUCoZ2MB/z6M8CBSnVrnMqE5dUVh8Vm3lOCjl/B7L0KA51clsQzm6RvW9DPvMRNhghd9SOioBJrgYSYAkkjTkR9DOP2O5ITAjno8HRSbGaWaga/0EgIf/w+WZk6RS4rNb6Zchsk= 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=GG16ya0Z; 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="GG16ya0Z" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=weissschuh.net; s=mail; t=1737412888; bh=sbkeC/YPODPeyqE7bpJJX+VlF53491jAs4WmSbiBrBg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GG16ya0ZZG/kIq3lgskbrj4cYz0HESK5us1EqzckYkJHHrRym/mSLAiJ106tAXKEQ qnXlEvF77bRfpm8a7TlbV8Lv2Vkqff8/p+XQ+rrjCQK52OKw3CrrYLruzAKy1WzLnR f90vud+lmMeJptfr4GBvs0u36YMRDLUgDuBdPzTg= Date: Mon, 20 Jan 2025 23:41:26 +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: <31acd2d0-8384-4ec5-acaa-187323b1968d@t-8ch.de> References: <4229b992-77ef-404d-b045-8f1ccad1035a@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 01:30:11+0300, Cyrill Gorcunov wrote: > On Mon, Jan 20, 2025 at 11:22:52PM +0100, Thomas Weißschuh wrote: > ... > > > -#ifdef CONFIG_COMPAT > > > - .compat_ioctl = posix_clock_compat_ioctl, > > > -#endif > > > + .compat_ioctl = compat_ptr_ioctl, > > > > This is not correct on s390. (It wasn't before either, though) > > The improved patch below is in my personal queue, but I didn't get > > around to actually testing and submitting it yet. > > Hi, Thomas! Thanks for looking into the patch! I somehow miss why it won't > work 'cause compat_ptr_ioctl already does the same conversion as in your > code below, no? I miss something obvious? See below. > > ``` > long compat_ptr_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > { > if (!file->f_op->unlocked_ioctl) > return -ENOIOCTLCMD; > > return file->f_op->unlocked_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); > } > EXPORT_SYMBOL(compat_ptr_ioctl); > ``` > > > +#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(). 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. > > > + } > > + > > + return ptp_ioctl(pccontext, cmd, arg); > > +} > > +#endif