Before this, I have discussed this problem with Arnd. And Arnd have an idea that by converting timeval to long / long in input_event, so that input_event structure size will be unchanged, and timeval structure will removed entirely. But we also need to avoid using CLOCK_REALTIME in userland, to keep the new input_event structure y2038 safe.
The input_event will only support monotonic time in Arnd's idea. And we still need to add wall time support for old 32-bit binary.
Those patches try to keep original input capacity, and resolve y2038 problem in input_event radically.
struct input_event is only used between kernel and userspace communication (except uinput). So that we can replace input_event with input_event64 in kernel entirely, and add a conversion in input_event_from/to_user() to keep compatible with old 32-bits binary.
userland can switch to input_event64, which is y2038 safe, via ioctl.
WEN Pingbo (3): evdev: convert input_event to input_event64 evdev: add new ioctl EVIOCSEVENT / EVIOCGEVENT uinput: convert input_event to input_event64
drivers/input/evdev.c | 38 ++++++++++++------- drivers/input/input-compat.c | 88 +++++++++++++++++++++++++++++++++++++------- drivers/input/input-compat.h | 9 +++-- drivers/input/misc/uinput.c | 17 ++++++--- include/linux/uinput.h | 2 +- include/uapi/linux/input.h | 18 +++++++++ 6 files changed, 134 insertions(+), 38 deletions(-)
struct input_event is not y2038 safe. This patch try to convert it to input_event64, which replaced timeval with timespec64.
There are many userspace programs, which use input_event to talk with evdev interface. In order to keep compatible with those binary, we add a flag (is_input_event64) to indicate which structure is used in current evdev_client, and do input_event/input_event64 conversion in input_event_from/to_user().
Userland can get / set is_input_event64 flag via EVIOCGEVENT / EVIOCSEVENT ioctl command.
According to current situation, we set is_input_event64 to false by default. So that all old userspace programs will work normally. And we need another patch to change this option, after most of programs have moved to new structure.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org --- drivers/input/evdev.c | 29 ++++++++------- drivers/input/input-compat.c | 88 +++++++++++++++++++++++++++++++++++++------- drivers/input/input-compat.h | 9 +++-- include/uapi/linux/input.h | 15 ++++++++ 4 files changed, 110 insertions(+), 31 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index 08d4964..815487f 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -58,8 +58,9 @@ struct evdev_client { struct list_head node; int clk_type; bool revoked; + bool is_input_event64; unsigned int bufsize; - struct input_event buffer[]; + struct input_event64 *buffer; };
/* flush queued events of type @type, caller must hold client->buffer_lock */ @@ -68,7 +69,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) unsigned int i, head, num; unsigned int mask = client->bufsize - 1; bool is_report; - struct input_event *ev; + struct input_event64 *ev;
BUG_ON(type == EV_SYN);
@@ -110,7 +111,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
static void __evdev_queue_syn_dropped(struct evdev_client *client) { - struct input_event ev; + struct input_event64 ev; ktime_t time;
time = client->clk_type == EV_CLK_REAL ? @@ -119,7 +120,7 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client) ktime_get() : ktime_get_boottime();
- ev.time = ktime_to_timeval(time); + ev.time = ktime_to_timespec64(time); ev.type = EV_SYN; ev.code = SYN_DROPPED; ev.value = 0; @@ -182,7 +183,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid) }
static void __pass_event(struct evdev_client *client, - const struct input_event *event) + const struct input_event64 *event) { client->buffer[client->head++] = *event; client->head &= client->bufsize - 1; @@ -214,13 +215,13 @@ static void evdev_pass_values(struct evdev_client *client, { struct evdev *evdev = client->evdev; const struct input_value *v; - struct input_event event; + struct input_event64 event; bool wakeup = false;
if (client->revoked) return;
- event.time = ktime_to_timeval(ev_time[client->clk_type]); + event.time = ktime_to_timespec64(ev_time[client->clk_type]);
/* Interrupts are disabled, just acquire the lock. */ spin_lock(&client->buffer_lock); @@ -438,7 +439,7 @@ static int evdev_open(struct inode *inode, struct file *file) struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev); unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev); unsigned int size = sizeof(struct evdev_client) + - bufsize * sizeof(struct input_event); + bufsize * sizeof(struct input_event64); struct evdev_client *client; int error;
@@ -473,7 +474,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct input_event64 event; int retval = 0;
if (count != 0 && count < input_event_size()) @@ -490,7 +491,8 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
while (retval + input_event_size() <= count) {
- if (input_event_from_user(buffer + retval, &event)) { + if (input_event_from_user(buffer + retval, &event, + client->is_input_event64)) { retval = -EFAULT; goto out; } @@ -506,7 +508,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, }
static int evdev_fetch_next_event(struct evdev_client *client, - struct input_event *event) + struct input_event64 *event) { int have_event;
@@ -528,7 +530,7 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct input_event64 event; size_t read = 0; int error;
@@ -553,7 +555,8 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, while (read + input_event_size() <= count && evdev_fetch_next_event(client, &event)) {
- if (input_event_to_user(buffer + read, &event)) + if (input_event_to_user(buffer + read, &event, + client->is_input_event64)) return -EFAULT;
read += input_event_size(); diff --git a/drivers/input/input-compat.c b/drivers/input/input-compat.c index 64ca711..9bad69a 100644 --- a/drivers/input/input-compat.c +++ b/drivers/input/input-compat.c @@ -12,10 +12,30 @@ #include <asm/uaccess.h> #include "input-compat.h"
+void input_event_to_event64(const struct input_event *event, + struct input_event64 *event64) +{ + event64->time.tv_sec = event->time.tv_sec; + event64->time.tv_nsec = event->time.tv_usec * NSEC_PER_USEC; + event64->type = event->type; + event64->code = event->code; + event64->value = event->value; +} + +void input_event64_to_event(const struct input_event64 *event64, + struct input_event *event) +{ + event->time.tv_sec = event64->time.tv_sec; + event->time.tv_usec = event64->time.tv_nsec / NSEC_PER_USEC; + event->type = event64->type; + event->code = event64->code; + event->value = event64->value; +} + #ifdef CONFIG_COMPAT
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct input_event64 *event, bool is_input_event64) { if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) { struct input_event_compat compat_event; @@ -25,27 +45,37 @@ int input_event_from_user(const char __user *buffer, return -EFAULT;
event->time.tv_sec = compat_event.time.tv_sec; - event->time.tv_usec = compat_event.time.tv_usec; + event->time.tv_nsec = compat_event.time.tv_usec * NSEC_PER_USEC; event->type = compat_event.type; event->code = compat_event.code; event->value = compat_event.value;
} else { - if (copy_from_user(event, buffer, sizeof(struct input_event))) - return -EFAULT; + if (is_input_event64) { + if (copy_from_user(event, buffer, + sizeof(struct input_event64))) + return -EFAULT; + } else { /* userland use struct input_event */ + struct input_event ev; + + if (copy_from_user(&ev, buffer, + sizeof(struct input_event))) + return -EFAULT; + input_event_to_event64(&ev, event); + } }
return 0; }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct input_event64 *event, bool is_input_event64) { if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) { struct input_event_compat compat_event;
compat_event.time.tv_sec = event->time.tv_sec; - compat_event.time.tv_usec = event->time.tv_usec; + compat_event.time.tv_usec = event->time.tv_nsec / NSEC_PER_USEC; compat_event.type = event->type; compat_event.code = event->code; compat_event.value = event->value; @@ -55,8 +85,18 @@ int input_event_to_user(char __user *buffer, return -EFAULT;
} else { - if (copy_to_user(buffer, event, sizeof(struct input_event))) - return -EFAULT; + if (is_input_event64) { + if (copy_to_user(buffer, event, + sizeof(struct input_event64))) + return -EFAULT; + } else { /* userland use struct input_event */ + struct input_event ev; + + input_event64_to_event(event, &ev); + if (copy_to_user(buffer, &ev, + sizeof(struct input_event))) + return -EFAULT; + } }
return 0; @@ -100,19 +140,39 @@ int input_ff_effect_from_user(const char __user *buffer, size_t size, #else
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct input_event64 *event, bool is_input_event64) { - if (copy_from_user(event, buffer, sizeof(struct input_event))) - return -EFAULT; + if (is_input_event64) { + if (copy_from_user(event, buffer, + sizeof(struct input_event64))) + return -EFAULT; + } else { /* userland use struct input_event */ + struct input_event ev; + + if (copy_from_user(&ev, buffer, + sizeof(struct input_event))) + return -EFAULT; + input_event_to_event64(&ev, event); + }
return 0; }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct input_event64 *event, bool is_input_event64) { - if (copy_to_user(buffer, event, sizeof(struct input_event))) - return -EFAULT; + if (is_input_event64) { + if (copy_to_user(buffer, event, + sizeof(struct input_event64))) + return -EFAULT; + } else { /* userland use struct input_event */ + struct input_event ev; + + input_event64_to_event(event, &ev); + if (copy_to_user(buffer, &ev, + sizeof(struct input_event))) + return -EFAULT; + }
return 0; } diff --git a/drivers/input/input-compat.h b/drivers/input/input-compat.h index 148f66f..f770b5d 100644 --- a/drivers/input/input-compat.h +++ b/drivers/input/input-compat.h @@ -68,23 +68,24 @@ struct ff_effect_compat { static inline size_t input_event_size(void) { return (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) ? - sizeof(struct input_event_compat) : sizeof(struct input_event); + sizeof(struct input_event_compat) : + sizeof(struct input_event64); }
#else
static inline size_t input_event_size(void) { - return sizeof(struct input_event); + return sizeof(struct input_event64); }
#endif /* CONFIG_COMPAT */
int input_event_from_user(const char __user *buffer, - struct input_event *event); + struct input_event64 *event, bool is_input_event64);
int input_event_to_user(char __user *buffer, - const struct input_event *event); + const struct input_event64 *event, bool is_input_event64);
int input_ff_effect_from_user(const char __user *buffer, size_t size, struct ff_effect *effect); diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index 731417c..1e252ff 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -21,6 +21,14 @@ * The event structure itself */
+/* + * This structure should not use anymore, please + * use input_event64 instead. Placing it here is + * only for compatibility purpose. + * + * FIXME: move to input-compat.h if there are no + * compatibility issues. + */ struct input_event { struct timeval time; __u16 type; @@ -28,6 +36,13 @@ struct input_event { __s32 value; };
+struct input_event64 { + struct timespec64 time; + __u16 type; + __u16 code; + __s32 value; +}; + /* * Protocol version. */
The two ioctl command is used to set is_input_event64 flag.
All userspace programs must set this flag to true, before using the new input_event64 structure.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org --- drivers/input/evdev.c | 9 +++++++++ include/uapi/linux/input.h | 3 +++ 2 files changed, 12 insertions(+)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index 815487f..990a083 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -863,6 +863,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, int __user *ip = (int __user *)p; unsigned int i, t, u, v; unsigned int size; + int event_flag; int error;
/* First we check for fixed-length commands */ @@ -937,6 +938,14 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
case EVIOCSKEYCODE_V2: return evdev_handle_set_keycode_v2(dev, p); + case EVIOCSEVENT: + if (get_user(event_flag, ip)) + return -EFAULT; + client->is_input_event64 = !!event_flag; + + return 0; + case EVIOCGEVENT: + return put_user(client->is_input_event64, ip); }
size = _IOC_SIZE(cmd); diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index 1e252ff..966ce8f 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -171,6 +171,9 @@ struct input_keymap_entry {
#define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */
+#define EVIOCSEVENT _IOW('E', 0xa1, int) /* Set input_event64 flag*/ +#define EVIOCGEVENT _IOR('E', 0xa2, int) /* Get input_event64 flag*/ + /* * Device properties and quirks */
evdev has converted to input_event64, so is uinput.
Since uinput uses input_event to talk with driver, we hardcode is_input_event64 flag here directly.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org --- drivers/input/misc/uinput.c | 17 +++++++++++------ include/linux/uinput.h | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 5adbced..d60dc6f 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -50,7 +50,7 @@ static int uinput_dev_event(struct input_dev *dev, udev->buff[udev->head].type = type; udev->buff[udev->head].code = code; udev->buff[udev->head].value = value; - do_gettimeofday(&udev->buff[udev->head].time); + getnstimeofday64(&udev->buff[udev->head].time); udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
wake_up_interruptible(&udev->waitq); @@ -436,7 +436,7 @@ static int uinput_setup_device(struct uinput_device *udev, static ssize_t uinput_inject_events(struct uinput_device *udev, const char __user *buffer, size_t count) { - struct input_event ev; + struct input_event64 ev; size_t bytes = 0;
if (count != 0 && count < input_event_size()) @@ -448,8 +448,10 @@ static ssize_t uinput_inject_events(struct uinput_device *udev, * we are still going to return EFAULT instead of partial * count to let userspace know that it got it's buffers * all wrong. + * + * Force set is_input_event64 to false */ - if (input_event_from_user(buffer + bytes, &ev)) + if (input_event_from_user(buffer + bytes, &ev, 0)) return -EFAULT;
input_event(udev->dev, ev.type, ev.code, ev.value); @@ -482,7 +484,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, }
static bool uinput_fetch_next_event(struct uinput_device *udev, - struct input_event *event) + struct input_event64 *event) { bool have_event;
@@ -502,13 +504,16 @@ static bool uinput_fetch_next_event(struct uinput_device *udev, static ssize_t uinput_events_to_user(struct uinput_device *udev, char __user *buffer, size_t count) { - struct input_event event; + struct input_event64 event; size_t read = 0;
while (read + input_event_size() <= count && uinput_fetch_next_event(udev, &event)) {
- if (input_event_to_user(buffer + read, &event)) + /* + * Force set is_input_event64 to false + */ + if (input_event_to_user(buffer + read, &event, 0)) return -EFAULT;
read += input_event_size(); diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 0994c0d..17859c0 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -66,7 +66,7 @@ struct uinput_device { unsigned char ready; unsigned char head; unsigned char tail; - struct input_event buff[UINPUT_BUFFER_SIZE]; + struct input_event64 buff[UINPUT_BUFFER_SIZE]; unsigned int ff_effects_max;
struct uinput_request *requests[UINPUT_NUM_REQUESTS];
Hi Wen,
On Mon, Nov 02, 2015 at 09:35:36PM +0800, WEN Pingbo wrote:
Before this, I have discussed this problem with Arnd. And Arnd have an idea that by converting timeval to long / long in input_event, so that input_event structure size will be unchanged, and timeval structure will removed entirely. But we also need to avoid using CLOCK_REALTIME in userland, to keep the new input_event structure y2038 safe.
The input_event will only support monotonic time in Arnd's idea. And we still need to add wall time support for old 32-bit binary.
Those patches try to keep original input capacity, and resolve y2038 problem in input_event radically.
struct input_event is only used between kernel and userspace communication (except uinput). So that we can replace input_event with input_event64 in kernel entirely, and add a conversion in input_event_from/to_user() to keep compatible with old 32-bits binary.
userland can switch to input_event64, which is y2038 safe, via ioctl.
If we are forcing userspace to change the protocol I'd rather explore whether we need to transmit the timestamp in each and every event. I would much rather drop it and instead introduce new event code for timestamp (we already have MSC_TIMESTAMP for hardware-generated timestamps, maybe we can introduce new ones for kernel-generated timestamps).
Thanks.
在 2015年11月3日，09:43，Dmitry Torokhov dmitry.torokhov@gmail.com 写道：
On Mon, Nov 02, 2015 at 09:35:36PM +0800, WEN Pingbo wrote:
Before this, I have discussed this problem with Arnd. And Arnd have an idea that by converting timeval to long / long in input_event, so that input_event structure size will be unchanged, and timeval structure will removed entirely. But we also need to avoid using CLOCK_REALTIME in userland, to keep the new input_event structure y2038 safe.
The input_event will only support monotonic time in Arnd's idea. And we still need to add wall time support for old 32-bit binary.
Those patches try to keep original input capacity, and resolve y2038 problem in input_event radically.
struct input_event is only used between kernel and userspace communication (except uinput). So that we can replace input_event with input_event64 in kernel entirely, and add a conversion in input_event_from/to_user() to keep compatible with old 32-bits binary.
userland can switch to input_event64, which is y2038 safe, via ioctl.
If we are forcing userspace to change the protocol I'd rather explore whether we need to transmit the timestamp in each and every event. I would much rather drop it and instead introduce new event code for timestamp (we already have MSC_TIMESTAMP for hardware-generated timestamps, maybe we can introduce new ones for kernel-generated timestamps).
Yes, we can do that by replacing all input_event with input_value in kernel, and injecting a timestamp event after every normal event (maybe we can give userspace a option here).
But we still need do some conversion work in input_event_from/to_user to be compatible with old binary. And we need extend value field in ’struct input_value’ to s64, so the timestamp can be passed safely.
Pingbo
On Wednesday 04 November 2015 11:35:20 Pingbo Wen wrote:
在 2015年11月3日，09:43，Dmitry Torokhov dmitry.torokhov@gmail.com 写道：
On Mon, Nov 02, 2015 at 09:35:36PM +0800, WEN Pingbo wrote:
Before this, I have discussed this problem with Arnd. And Arnd have an idea that by converting timeval to long / long in input_event, so that input_event structure size will be unchanged, and timeval structure will removed entirely. But we also need to avoid using CLOCK_REALTIME in userland, to keep the new input_event structure y2038 safe.
The input_event will only support monotonic time in Arnd's idea. And we still need to add wall time support for old 32-bit binary.
Those patches try to keep original input capacity, and resolve y2038 problem in input_event radically.
struct input_event is only used between kernel and userspace communication (except uinput). So that we can replace input_event with input_event64 in kernel entirely, and add a conversion in input_event_from/to_user() to keep compatible with old 32-bits binary.
userland can switch to input_event64, which is y2038 safe, via ioctl.
If we are forcing userspace to change the protocol I'd rather explore whether we need to transmit the timestamp in each and every event. I would much rather drop it and instead introduce new event code for timestamp (we already have MSC_TIMESTAMP for hardware-generated timestamps, maybe we can introduce new ones for kernel-generated timestamps).
Yes, we can do that by replacing all input_event with input_value in kernel, and injecting a timestamp event after every normal event (maybe we can give userspace a option here).
But we still need do some conversion work in input_event_from/to_user to be compatible with old binary. And we need extend value field in ’struct input_value’ to s64, so the timestamp can be passed safely.
Right, that would be equivalent to Pingbo's approach (fixing the y2038 problem by requiring user space changes that are incompatible with old kernels), but with a nicer interface.
The approach of leaving the event timestamps as 32 bit but requiring montonic times in contrast would require only user space to change when it cares about time stamps and doesn't already use montonic times. Importantly, the modified user space would remain compatible with older kernels as long as they support monotonic times. Those were introduced by John Stultz in a80b83b7b8 ("Input: add infrastructure for selecting clockid for event time stamps") and merged into linux-3.3, and are generally a good idea because of time jumps.
Arnd
-
Arnd Bergmann -
Dmitry Torokhov -
Pingbo Wen -
WEN Pingbo