Pointer arguments passed to ioctls need to pass through compat_ptr() to
work correctly on s390; as explained in Documentation/driver-api/ioctl.rst.
Plumb the compat_ioctl callback through 'struct posix_clock_operations'
and handle the different ioctls cmds in the new ptp_compat_ioctl().

Using compat_ptr_ioctl is not possible.
For the commands PTP_ENABLE_PPS/PTP_ENABLE_PPS2 on s390
it would corrupt the argument 0x80000000, aka BIT(31) to zero.

Fixes: 0606f422b453 ("posix clocks: Introduce dynamic clocks")
Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/ptp/ptp_chardev.c   | 18 ++++++++++++++++++
 drivers/ptp/ptp_clock.c     |  1 +
 drivers/ptp/ptp_private.h   |  7 +++++++
 include/linux/posix-clock.h |  3 +++
 kernel/time/posix-clock.c   |  4 ++--
 5 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index ea96a14d72d141a4b255563b66bac8ed568b45e9..704af620c1173c12ee26b3b6c7982693e3c4c21f 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2010 OMICRON electronics GmbH
  */
+#include <linux/compat.h>
 #include <linux/module.h>
 #include <linux/posix-clock.h>
 #include <linux/poll.h>
@@ -507,6 +508,23 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
 	return err;
 }
 
+#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);
+	}
+
+	return ptp_ioctl(pccontext, cmd, arg);
+}
+#endif
+
 __poll_t ptp_poll(struct posix_clock_context *pccontext, struct file *fp,
 		  poll_table *wait)
 {
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 77a36e7bddd54e8f45eab317e687f033f57cc5bc..dec84b81cedfd13bcf8c97be6c3c27d73cd671f6 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -180,6 +180,7 @@ static struct posix_clock_operations ptp_clock_ops = {
 	.clock_getres	= ptp_clock_getres,
 	.clock_settime	= ptp_clock_settime,
 	.ioctl		= ptp_ioctl,
+	.compat_ioctl	= ptp_compat_ioctl,
 	.open		= ptp_open,
 	.release	= ptp_release,
 	.poll		= ptp_poll,
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 18934e28469ee6e3bf9c9e6d1a1adb82808d88e6..13999a7af47a7b67ae8dc549351fbafef2ae402f 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -133,6 +133,13 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin,
 long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
 	       unsigned long arg);
 
+#ifdef CONFIG_COMPAT
+long ptp_compat_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
+		      unsigned long arg);
+#else
+#define ptp_compat_ioctl NULL
+#endif
+
 int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode);
 
 int ptp_release(struct posix_clock_context *pccontext);
diff --git a/include/linux/posix-clock.h b/include/linux/posix-clock.h
index ef8619f489203eeb369ae580fc4d4b2439c94ae9..8bdc7aec5f7979c42752a233077620818d15acdd 100644
--- a/include/linux/posix-clock.h
+++ b/include/linux/posix-clock.h
@@ -54,6 +54,9 @@ struct posix_clock_operations {
 	long (*ioctl)(struct posix_clock_context *pccontext, unsigned int cmd,
 		      unsigned long arg);
 
+	long (*compat_ioctl)(struct posix_clock_context *pccontext, unsigned int cmd,
+			     unsigned long arg);
+
 	int (*open)(struct posix_clock_context *pccontext, fmode_t f_mode);
 
 	__poll_t (*poll)(struct posix_clock_context *pccontext, struct file *file,
diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index 1af0bb2cc45c0aab843f77eb156992de469c8fb9..63184d92ef139c3fb9254745619ebca120fecce6 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -101,8 +101,8 @@ static long posix_clock_compat_ioctl(struct file *fp,
 	if (!clk)
 		return -ENODEV;
 
-	if (clk->ops.ioctl)
-		err = clk->ops.ioctl(pccontext, cmd, arg);
+	if (clk->ops.compat_ioctl)
+		err = clk->ops.compat_ioctl(pccontext, cmd, arg);
 
 	put_posix_clock(clk);
 

---
base-commit: 0bc21e701a6ffacfdde7f04f87d664d82e8a13bf
change-id: 20250103-posix-clock-compat_ioctl-96fbac549146

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

On Tue, Jan 21, 2025 at 11:41:24PM +0100, Thomas Weißschuh wrote:
> Pointer arguments passed to ioctls need to pass through compat_ptr() to
> work correctly on s390; as explained in Documentation/driver-api/ioctl.rst.

PTP_ENABLE_PPS is either on of off, and the code tests whether the
passed argument is zero or not.

So does this compat code actually fix an issue for s390?

Thanks,
Richard

On Tue, Jan 21, 2025 at 11:41:24PM +0100, Thomas Weißschuh wrote:
> Pointer arguments passed to ioctls need to pass through compat_ptr() to
> work correctly on s390; as explained in Documentation/driver-api/ioctl.rst.
> Plumb the compat_ioctl callback through 'struct posix_clock_operations'
> and handle the different ioctls cmds in the new ptp_compat_ioctl().
> 
> Using compat_ptr_ioctl is not possible.
> For the commands PTP_ENABLE_PPS/PTP_ENABLE_PPS2 on s390
> it would corrupt the argument 0x80000000, aka BIT(31) to zero.
> 
> Fixes: 0606f422b453 ("posix clocks: Introduce dynamic clocks")
> Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

...

> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 77a36e7bddd54e8f45eab317e687f033f57cc5bc..dec84b81cedfd13bcf8c97be6c3c27d73cd671f6 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -180,6 +180,7 @@ static struct posix_clock_operations ptp_clock_ops = {
>  	.clock_getres	= ptp_clock_getres,
>  	.clock_settime	= ptp_clock_settime,
>  	.ioctl		= ptp_ioctl,
> +	.compat_ioctl	= ptp_compat_ioctl,
>  	.open		= ptp_open,
>  	.release	= ptp_release,
>  	.poll		= ptp_poll,


nit: compat_ioctl should also be added to the Kernel doc for
     struct posix_clock_operations

...

On Tue, Jan 21, 2025 at 11:41:24PM +0100, Thomas Weißschuh wrote:
> Pointer arguments passed to ioctls need to pass through compat_ptr() to
> work correctly on s390; as explained in Documentation/driver-api/ioctl.rst.
> Plumb the compat_ioctl callback through 'struct posix_clock_operations'
> and handle the different ioctls cmds in the new ptp_compat_ioctl().
> 
> Using compat_ptr_ioctl is not possible.
> For the commands PTP_ENABLE_PPS/PTP_ENABLE_PPS2 on s390
> it would corrupt the argument 0x80000000, aka BIT(31) to zero.
> 
> Fixes: 0606f422b453 ("posix clocks: Introduce dynamic clocks")
> Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

Thanks, Thomas!