[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4229b992-77ef-404d-b045-8f1ccad1035a@t-8ch.de>
Date: Mon, 20 Jan 2025 23:22:52 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Cyrill Gorcunov <gorcunov@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Anna-Maria Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] posix-clock: drop code duplication using compat_ptr_ioctl
Hi Cyrill,
On 2025-01-21 01:10:27+0300, Cyrill Gorcunov wrote:
> The posix_clock_compat_ioctl() is just a clone of posix_clock_ioctl(),
> drop the redundance.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@...il.com>
> Cc: Anna-Maria Behnsen <anna-maria@...utronix.de>
> Cc: Frederic Weisbecker <frederic@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> ---
> kernel/time/posix-clock.c | 24 +-----------------------
> 1 file changed, 1 insertion(+), 23 deletions(-)
>
> Index: linux-tip.git/kernel/time/posix-clock.c
> ===================================================================
> --- linux-tip.git.orig/kernel/time/posix-clock.c
> +++ linux-tip.git/kernel/time/posix-clock.c
> @@ -90,26 +90,6 @@ static long posix_clock_ioctl(struct fil
> return err;
> }
>
> -#ifdef CONFIG_COMPAT
> -static long posix_clock_compat_ioctl(struct file *fp,
> - unsigned int cmd, unsigned long arg)
> -{
> - struct posix_clock_context *pccontext = fp->private_data;
> - struct posix_clock *clk = get_posix_clock(fp);
> - int err = -ENOTTY;
> -
> - if (!clk)
> - return -ENODEV;
> -
> - if (clk->ops.ioctl)
> - err = clk->ops.ioctl(pccontext, cmd, arg);
> -
> - put_posix_clock(clk);
> -
> - return err;
> -}
> -#endif
> -
> static int posix_clock_open(struct inode *inode, struct file *fp)
> {
> int err;
> @@ -173,9 +153,7 @@ static const struct file_operations posi
> .unlocked_ioctl = posix_clock_ioctl,
> .open = posix_clock_open,
> .release = posix_clock_release,
> -#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.
> };
>
> int posix_clock_register(struct posix_clock *clk, struct device *dev)
>From 9d59a26da95ad416710c152dc87ee41f08933c2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= <linux@...ssschuh.net>
Date: Fri, 3 Jan 2025 18:04:56 +0100
Subject: [PATCH] posix-clock: Fix compat ioctls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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().
Fixes: 0606f422b453 ("posix clocks: Introduce dynamic clocks")
Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
Cc: stable@...r.kernel.org
Signed-off-by: Thomas Weißschuh <linux@...ssschuh.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 ea96a14d72d1..704af620c117 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 77a36e7bddd5..dec84b81cedf 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 18934e28469e..13999a7af47a 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 ef8619f48920..8bdc7aec5f79 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 1af0bb2cc45c..63184d92ef13 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);
Powered by blists - more mailing lists