2023-06-07 21:47:01

by Jason Gerecke

[permalink] [raw]
Subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps

Code which interacts with timestamps needs to use the ktime_t type
returned by functions like ktime_get. The int type does not offer
enough space to store these values, and attempting to use it is a
recipe for problems. In this particular case, overflows would occur
when calculating/storing timestamps leading to incorrect values being
reported to userspace. In some cases these bad timestamps cause input
handling in userspace to appear hung.

Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
Signed-off-by: Jason Gerecke <[email protected]>
---
drivers/hid/wacom_wac.c | 4 ++--
drivers/hid/wacom_wac.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2ccf83837134..2f16e47e4b69 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
struct input_dev *pen_input = wacom->pen_input;
unsigned char *data = wacom->data;
int number_of_valid_frames = 0;
- int time_interval = 15000000;
+ ktime_t time_interval = 15000000;
ktime_t time_packet_received = ktime_get();
int i;

@@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
bool range = frame[0] & 0x20;
bool invert = frame[0] & 0x10;
int frames_number_reversed = number_of_valid_frames - i - 1;
- int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
+ ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;

if (!valid)
continue;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1a40bb8c5810..ee21bb260f22 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -324,7 +324,7 @@ struct hid_data {
int ps_connected;
bool pad_input_event_flag;
unsigned short sequence_number;
- int time_delayed;
+ ktime_t time_delayed;
};

struct wacom_remote_data {
--
2.41.0



2023-06-08 03:04:06

by Peter Hutterer

[permalink] [raw]
Subject: Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps

On Wed, Jun 07, 2023 at 02:41:02PM -0700, Jason Gerecke wrote:
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> Signed-off-by: Jason Gerecke <[email protected]>

Reviewed-by: Peter Hutterer <[email protected]>

Cheers,
Peter

> ---
> drivers/hid/wacom_wac.c | 4 ++--
> drivers/hid/wacom_wac.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2ccf83837134..2f16e47e4b69 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> struct input_dev *pen_input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int number_of_valid_frames = 0;
> - int time_interval = 15000000;
> + ktime_t time_interval = 15000000;
> ktime_t time_packet_received = ktime_get();
> int i;
>
> @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> bool range = frame[0] & 0x20;
> bool invert = frame[0] & 0x10;
> int frames_number_reversed = number_of_valid_frames - i - 1;
> - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
>
> if (!valid)
> continue;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 1a40bb8c5810..ee21bb260f22 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -324,7 +324,7 @@ struct hid_data {
> int ps_connected;
> bool pad_input_event_flag;
> unsigned short sequence_number;
> - int time_delayed;
> + ktime_t time_delayed;
> };
>
> struct wacom_remote_data {
> --
> 2.41.0
>

2023-06-08 14:27:15

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arc-randconfig-c003-20230608 (https://download.01.org/0day-ci/archive/20230608/[email protected]/config)
compiler: arc-elf-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__divdi3" [drivers/hid/wacom.ko] undefined!

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-08 14:59:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arm-randconfig-r036-20230608 (https://download.01.org/0day-ci/archive/20230608/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: __aeabi_ldivmod
>>> referenced by wacom_wac.c
>>> drivers/hid/wacom_wac.o:(wacom_wac_irq) in archive vmlinux.a
>>> did you mean: __aeabi_idivmod
>>> defined in: arch/arm/lib/lib.a(lib1funcs.o)

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-08 22:05:20

by Jason Gerecke

[permalink] [raw]
Subject: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps

Code which interacts with timestamps needs to use the ktime_t type
returned by functions like ktime_get. The int type does not offer
enough space to store these values, and attempting to use it is a
recipe for problems. In this particular case, overflows would occur
when calculating/storing timestamps leading to incorrect values being
reported to userspace. In some cases these bad timestamps cause input
handling in userspace to appear hung.

Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
CC: [email protected]
Signed-off-by: Jason Gerecke <[email protected]>
---
v2: Use div_u64 to perform division to deal with ARC and ARM architectures
(as found by the kernel test robot)

drivers/hid/wacom_wac.c | 6 +++---
drivers/hid/wacom_wac.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2ccf838371343..174bf03908d7c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
struct input_dev *pen_input = wacom->pen_input;
unsigned char *data = wacom->data;
int number_of_valid_frames = 0;
- int time_interval = 15000000;
+ ktime_t time_interval = 15000000;
ktime_t time_packet_received = ktime_get();
int i;

@@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
if (number_of_valid_frames) {
if (wacom->hid_data.time_delayed)
time_interval = ktime_get() - wacom->hid_data.time_delayed;
- time_interval /= number_of_valid_frames;
+ time_interval = div_u64(time_interval, number_of_valid_frames);
wacom->hid_data.time_delayed = time_packet_received;
}

@@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
bool range = frame[0] & 0x20;
bool invert = frame[0] & 0x10;
int frames_number_reversed = number_of_valid_frames - i - 1;
- int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
+ ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;

if (!valid)
continue;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1a40bb8c5810c..ee21bb260f22f 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -324,7 +324,7 @@ struct hid_data {
int ps_connected;
bool pad_input_event_flag;
unsigned short sequence_number;
- int time_delayed;
+ ktime_t time_delayed;
};

struct wacom_remote_data {
--
2.41.0


2023-06-21 16:01:32

by Jason Gerecke

[permalink] [raw]
Subject: Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps

Following up since no action seems to have been taken on this patch yet.

On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <[email protected]> wrote:
>
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> CC: [email protected]
> Signed-off-by: Jason Gerecke <[email protected]>
> ---
> v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> (as found by the kernel test robot)
>
> drivers/hid/wacom_wac.c | 6 +++---
> drivers/hid/wacom_wac.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2ccf838371343..174bf03908d7c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> struct input_dev *pen_input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int number_of_valid_frames = 0;
> - int time_interval = 15000000;
> + ktime_t time_interval = 15000000;
> ktime_t time_packet_received = ktime_get();
> int i;
>
> @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> if (number_of_valid_frames) {
> if (wacom->hid_data.time_delayed)
> time_interval = ktime_get() - wacom->hid_data.time_delayed;
> - time_interval /= number_of_valid_frames;
> + time_interval = div_u64(time_interval, number_of_valid_frames);
> wacom->hid_data.time_delayed = time_packet_received;
> }
>
> @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> bool range = frame[0] & 0x20;
> bool invert = frame[0] & 0x10;
> int frames_number_reversed = number_of_valid_frames - i - 1;
> - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
>
> if (!valid)
> continue;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 1a40bb8c5810c..ee21bb260f22f 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -324,7 +324,7 @@ struct hid_data {
> int ps_connected;
> bool pad_input_event_flag;
> unsigned short sequence_number;
> - int time_delayed;
> + ktime_t time_delayed;
> };
>
> struct wacom_remote_data {
> --
> 2.41.0
>

2023-06-21 16:25:24

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps

On Wed, Jun 21, 2023 at 5:18 PM Jason Gerecke <[email protected]> wrote:
>
> Following up since no action seems to have been taken on this patch yet.

Sorry, this went through the cracks (I seem to have a lot of cracks recently...)

>
> On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <[email protected]> wrote:
> >
> > Code which interacts with timestamps needs to use the ktime_t type
> > returned by functions like ktime_get. The int type does not offer
> > enough space to store these values, and attempting to use it is a
> > recipe for problems. In this particular case, overflows would occur
> > when calculating/storing timestamps leading to incorrect values being
> > reported to userspace. In some cases these bad timestamps cause input
> > handling in userspace to appear hung.

I have to ask, is this something we should consider writing a test for? :)

Also, you are missing the rev-by from Peter, not sure if the tools
will pick it up automatically then.

Reviewed-by: Benjamin Tissoires <[email protected]>

Cheers,
Benjamin

> >
> > Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> > Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> > CC: [email protected]
> > Signed-off-by: Jason Gerecke <[email protected]>
> > ---
> > v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> > (as found by the kernel test robot)
> >
> > drivers/hid/wacom_wac.c | 6 +++---
> > drivers/hid/wacom_wac.h | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 2ccf838371343..174bf03908d7c 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > struct input_dev *pen_input = wacom->pen_input;
> > unsigned char *data = wacom->data;
> > int number_of_valid_frames = 0;
> > - int time_interval = 15000000;
> > + ktime_t time_interval = 15000000;
> > ktime_t time_packet_received = ktime_get();
> > int i;
> >
> > @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > if (number_of_valid_frames) {
> > if (wacom->hid_data.time_delayed)
> > time_interval = ktime_get() - wacom->hid_data.time_delayed;
> > - time_interval /= number_of_valid_frames;
> > + time_interval = div_u64(time_interval, number_of_valid_frames);
> > wacom->hid_data.time_delayed = time_packet_received;
> > }
> >
> > @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > bool range = frame[0] & 0x20;
> > bool invert = frame[0] & 0x10;
> > int frames_number_reversed = number_of_valid_frames - i - 1;
> > - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> >
> > if (!valid)
> > continue;
> > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > index 1a40bb8c5810c..ee21bb260f22f 100644
> > --- a/drivers/hid/wacom_wac.h
> > +++ b/drivers/hid/wacom_wac.h
> > @@ -324,7 +324,7 @@ struct hid_data {
> > int ps_connected;
> > bool pad_input_event_flag;
> > unsigned short sequence_number;
> > - int time_delayed;
> > + ktime_t time_delayed;
> > };
> >
> > struct wacom_remote_data {
> > --
> > 2.41.0
> >
>


2023-06-21 21:26:27

by Jason Gerecke

[permalink] [raw]
Subject: Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps

On Wed, Jun 21, 2023 at 9:04 AM Benjamin Tissoires
<[email protected]> wrote:
>
> On Wed, Jun 21, 2023 at 5:18 PM Jason Gerecke <[email protected]> wrote:
> >
> > Following up since no action seems to have been taken on this patch yet.
>
> Sorry, this went through the cracks (I seem to have a lot of cracks recently...)
>
> >
> > On Thu, Jun 8, 2023 at 2:38 PM Jason Gerecke <[email protected]> wrote:
> > >
> > > Code which interacts with timestamps needs to use the ktime_t type
> > > returned by functions like ktime_get. The int type does not offer
> > > enough space to store these values, and attempting to use it is a
> > > recipe for problems. In this particular case, overflows would occur
> > > when calculating/storing timestamps leading to incorrect values being
> > > reported to userspace. In some cases these bad timestamps cause input
> > > handling in userspace to appear hung.
>
> I have to ask, is this something we should consider writing a test for? :)
>

Very good point! I'm happy to work on such a test.

Are you open to merging this patch without delay while I write a
testcase? I don't like the idea of this (apparent) system freeze being
affecting users any longer than absolutely necessary.

> Also, you are missing the rev-by from Peter, not sure if the tools
> will pick it up automatically then.
>
> Reviewed-by: Benjamin Tissoires <[email protected]>
>

Oof, good catch. Apologies to Peter :)

Jason

> Cheers,
> Benjamin
>
> > >
> > > Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> > > Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> > > CC: [email protected]
> > > Signed-off-by: Jason Gerecke <[email protected]>
> > > ---
> > > v2: Use div_u64 to perform division to deal with ARC and ARM architectures
> > > (as found by the kernel test robot)
> > >
> > > drivers/hid/wacom_wac.c | 6 +++---
> > > drivers/hid/wacom_wac.h | 2 +-
> > > 2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > > index 2ccf838371343..174bf03908d7c 100644
> > > --- a/drivers/hid/wacom_wac.c
> > > +++ b/drivers/hid/wacom_wac.c
> > > @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > struct input_dev *pen_input = wacom->pen_input;
> > > unsigned char *data = wacom->data;
> > > int number_of_valid_frames = 0;
> > > - int time_interval = 15000000;
> > > + ktime_t time_interval = 15000000;
> > > ktime_t time_packet_received = ktime_get();
> > > int i;
> > >
> > > @@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > if (number_of_valid_frames) {
> > > if (wacom->hid_data.time_delayed)
> > > time_interval = ktime_get() - wacom->hid_data.time_delayed;
> > > - time_interval /= number_of_valid_frames;
> > > + time_interval = div_u64(time_interval, number_of_valid_frames);
> > > wacom->hid_data.time_delayed = time_packet_received;
> > > }
> > >
> > > @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> > > bool range = frame[0] & 0x20;
> > > bool invert = frame[0] & 0x10;
> > > int frames_number_reversed = number_of_valid_frames - i - 1;
> > > - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > > + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> > >
> > > if (!valid)
> > > continue;
> > > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > > index 1a40bb8c5810c..ee21bb260f22f 100644
> > > --- a/drivers/hid/wacom_wac.h
> > > +++ b/drivers/hid/wacom_wac.h
> > > @@ -324,7 +324,7 @@ struct hid_data {
> > > int ps_connected;
> > > bool pad_input_event_flag;
> > > unsigned short sequence_number;
> > > - int time_delayed;
> > > + ktime_t time_delayed;
> > > };
> > >
> > > struct wacom_remote_data {
> > > --
> > > 2.41.0
> > >
> >
>

2023-06-26 15:31:57

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps

From: Benjamin Tissoires <[email protected]>

On Thu, 08 Jun 2023 14:38:28 -0700, Jason Gerecke wrote:
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> [...]

Updated the "from" email from your patch.
It would be nice if you could fix your workflow (I know you well enough
to know what is your gmail address, but not having to fix this by hand
would be preferable)

Applied to hid/hid.git (for-6.5/wacom), thanks!

[1/1] HID: wacom: Use ktime_t rather than int when dealing with timestamps
https://git.kernel.org/hid/hid/c/9a6c0e28e215

Cheers,
--
Benjamin Tissoires <[email protected]>