Introduce a set of tracepoints for the input subsystem to enable
better debugging, monitoring, and performance analysis of input
device operations.
Suggested-by: Winston Wen <wentao@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
MAINTAINERS | 1 +
drivers/input/input.c | 18 ++-
include/trace/events/input.h | 251 +++++++++++++++++++++++++++++++++++
3 files changed, 269 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/input.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 00d7dcee88cb..c1b03679a5a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11933,6 +11933,7 @@ F: include/linux/i8042.h
F: include/linux/input.h
F: include/linux/input/
F: include/linux/libps2.h
+F: include/trace/events/input.h
F: include/linux/serio.h
F: include/uapi/linux/gameport.h
F: include/uapi/linux/input-event-codes.h
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 44887e51e049..925c76a50742 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -29,6 +29,9 @@
#include "input-core-private.h"
#include "input-poller.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/input.h>
+
MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
MODULE_DESCRIPTION("Input core");
MODULE_LICENSE("GPL");
@@ -363,6 +366,7 @@ void input_handle_event(struct input_dev *dev,
disposition = input_get_disposition(dev, type, code, &value);
if (disposition != INPUT_IGNORE_EVENT) {
+ trace_input_event(dev, type, code, value);
if (type != EV_SYN)
add_input_randomness(type, code, value);
@@ -525,6 +529,7 @@ int input_grab_device(struct input_handle *handle)
return -EBUSY;
rcu_assign_pointer(dev->grab, handle);
+ trace_input_grab_device(dev, handle);
}
return 0;
@@ -539,6 +544,7 @@ static void __input_release_device(struct input_handle *handle)
grabber = rcu_dereference_protected(dev->grab,
lockdep_is_held(&dev->mutex));
if (grabber == handle) {
+ trace_input_release_device(dev, handle);
rcu_assign_pointer(dev->grab, NULL);
/* Make sure input_pass_values() notices that grab is gone */
synchronize_rcu();
@@ -612,6 +618,7 @@ int input_open_device(struct input_handle *handle)
if (dev->poller)
input_dev_poller_start(dev->poller);
+ trace_input_open_device(dev);
}
return 0;
@@ -652,6 +659,7 @@ void input_close_device(struct input_handle *handle)
input_dev_poller_stop(dev->poller);
if (dev->close)
dev->close(dev);
+ trace_input_close_device(dev);
}
}
@@ -991,9 +999,13 @@ static int input_attach_handler(struct input_dev *dev, struct input_handler *han
return -ENODEV;
error = handler->connect(handler, dev, id);
- if (error && error != -ENODEV)
+ if (error && error != -ENODEV) {
pr_err("failed to attach handler %s to device %s, error: %d\n",
handler->name, kobject_name(&dev->dev.kobj), error);
+ } else if (!error) {
+ /* Connection successful, trace it */
+ trace_input_handler_connect(handler, dev, handler->minor);
+ }
return error;
}
@@ -1791,6 +1803,7 @@ static int input_inhibit_device(struct input_dev *dev)
}
dev->inhibited = true;
+ trace_input_inhibit_device(dev);
return 0;
}
@@ -1818,6 +1831,7 @@ static int input_uninhibit_device(struct input_dev *dev)
scoped_guard(spinlock_irq, &dev->event_lock)
input_dev_toggle(dev, true);
+ trace_input_uninhibit_device(dev);
return 0;
}
@@ -2216,6 +2230,7 @@ static void __input_unregister_device(struct input_dev *dev)
{
struct input_handle *handle, *next;
+ trace_input_device_unregister(dev);
input_disconnect_device(dev);
scoped_guard(mutex, &input_mutex) {
@@ -2414,6 +2429,7 @@ int input_register_device(struct input_dev *dev)
input_wakeup_procfs_readers();
}
+ trace_input_device_register(dev);
if (dev->devres_managed) {
dev_dbg(dev->dev.parent, "%s: registering %s with devres.\n",
__func__, dev_name(&dev->dev));
diff --git a/include/trace/events/input.h b/include/trace/events/input.h
new file mode 100644
index 000000000000..3c5ffcfb7c8d
--- /dev/null
+++ b/include/trace/events/input.h
@@ -0,0 +1,251 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* input tracepoints
+ *
+ * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM input
+
+#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INPUT_H
+
+#include <linux/tracepoint.h>
+#include <linux/input.h>
+
+/**
+ * input_event - called when an input event is processed
+ * @dev: input device that generated the event
+ * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
+ * @code: event code within the type
+ * @value: event value
+ *
+ * This tracepoint fires for every input event processed by the input core.
+ * It can be used to monitor input device activity and debug input issues.
+ */
+TRACE_EVENT(
+ input_event,
+
+ TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
+ int value),
+
+ TP_ARGS(dev, type, code, value),
+
+ TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
+ unsigned int, type) __field(unsigned int, code)
+ __field(int, value) __field(u16, bustype)
+ __field(u16, vendor)
+ __field(u16, product)),
+
+ TP_fast_assign(__assign_str(name); __entry->type = type;
+ __entry->code = code; __entry->value = value;
+ __entry->bustype = dev->id.bustype;
+ __entry->vendor = dev->id.vendor;
+ __entry->product = dev->id.product;),
+
+ TP_printk(
+ "dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
+ __get_str(name), __entry->type, __entry->code, __entry->value,
+ __entry->bustype, __entry->vendor, __entry->product));
+
+/**
+ * input_device_register - called when an input device is registered
+ * @dev: input device being registered
+ *
+ * This tracepoint fires when a new input device is registered with the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(
+ input_device_register,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev),
+
+ TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
+ phys, dev->phys ?: "") __field(u16, bustype)
+ __field(u16, vendor) __field(u16, product)
+ __field(u16, version)),
+
+ TP_fast_assign(__assign_str(name); __assign_str(phys);
+ __entry->bustype = dev->id.bustype;
+ __entry->vendor = dev->id.vendor;
+ __entry->product = dev->id.product;
+ __entry->version = dev->id.version;),
+
+ TP_printk(
+ "dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
+ __get_str(name), __get_str(phys), __entry->bustype,
+ __entry->vendor, __entry->product, __entry->version));
+
+/**
+ * input_device_unregister - called when an input device is unregistered
+ * @dev: input device being unregistered
+ *
+ * This tracepoint fires when an input device is unregistered from the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(input_device_unregister,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev),
+
+ TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
+ phys, dev->phys ?: "") __field(u16, bustype)
+ __field(u16, vendor)
+ __field(u16, product)),
+
+ TP_fast_assign(__assign_str(name); __assign_str(phys);
+ __entry->bustype = dev->id.bustype;
+ __entry->vendor = dev->id.vendor;
+ __entry->product = dev->id.product;),
+
+ TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04x",
+ __get_str(name), __get_str(phys), __entry->bustype,
+ __entry->vendor, __entry->product));
+
+/**
+ * input_handler_connect - called when an input handler connects to a device
+ * @handler: input handler
+ * @dev: input device
+ * @minor: assigned minor number (if applicable)
+ *
+ * This tracepoint fires when an input handler (like evdev, mousedev) connects
+ * to an input device, creating a new input handle.
+ */
+TRACE_EVENT(input_handler_connect,
+
+ TP_PROTO(struct input_handler *handler, struct input_dev *dev,
+ int minor),
+
+ TP_ARGS(handler, dev, minor),
+
+ TP_STRUCT__entry(__string(handler_name, handler->name)
+ __string(dev_name, dev->name ?: "unknown")
+ __field(int, minor)),
+
+ TP_fast_assign(__assign_str(handler_name); __assign_str(dev_name);
+ __entry->minor = minor;),
+
+ TP_printk("handler=%s dev=%s minor=%d", __get_str(handler_name),
+ __get_str(dev_name), __entry->minor));
+
+/**
+ * input_grab_device - called when a device is grabbed by a handler
+ * @dev: input device being grabbed
+ * @handle: input handle doing the grab
+ *
+ * This tracepoint fires when an input device is grabbed exclusively
+ * by an input handler, typically for security or special processing.
+ */
+TRACE_EVENT(input_grab_device,
+
+ TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+ TP_ARGS(dev, handle),
+
+ TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+ __string(handler_name,
+ handle->handler->name)),
+
+ TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
+
+ TP_printk("dev=%s grabbed_by=%s", __get_str(dev_name),
+ __get_str(handler_name)));
+
+/**
+ * input_release_device - called when a grabbed device is released
+ * @dev: input device being released
+ * @handle: input handle releasing the grab
+ *
+ * This tracepoint fires when an input device grab is released.
+ */
+TRACE_EVENT(input_release_device,
+
+ TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+ TP_ARGS(dev, handle),
+
+ TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+ __string(handler_name,
+ handle->handler->name)),
+
+ TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
+
+ TP_printk("dev=%s released_by=%s", __get_str(dev_name),
+ __get_str(handler_name)));
+
+DECLARE_EVENT_CLASS(input_device_state,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev),
+
+ TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+ __field(unsigned int, users)
+ __field(bool, inhibited)),
+
+ TP_fast_assign(__assign_str(name);
+ __entry->users = dev->users;
+ __entry->inhibited = dev->inhibited;),
+
+ TP_printk("dev=%s users=%u inhibited=%s", __get_str(name),
+ __entry->users,
+ __entry->inhibited ? "true" : "false"));
+
+/**
+ * input_open_device - called when an input device is opened
+ * @dev: input device being opened
+ *
+ * This tracepoint fires when the user count for an input device increases,
+ * typically when a userspace application opens the device.
+ */
+DEFINE_EVENT(input_device_state, input_open_device,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev));
+
+/**
+ * input_close_device - called when an input device is closed
+ * @dev: input device being closed
+ *
+ * This tracepoint fires when the user count for an input device decreases,
+ * typically when a userspace application closes the device.
+ */
+DEFINE_EVENT(input_device_state, input_close_device,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev));
+
+/**
+ * input_inhibit_device - called when an input device is inhibited
+ * @dev: input device being inhibited
+ *
+ * This tracepoint fires when an input device is inhibited (disabled),
+ * usually for power management or security reasons.
+ */
+DEFINE_EVENT(input_device_state, input_inhibit_device,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev));
+
+/**
+ * input_uninhibit_device - called when an input device is uninhibited
+ * @dev: input device being uninhibited
+ *
+ * This tracepoint fires when an input device is uninhibited (re-enabled)
+ * after being previously inhibited.
+ */
+DEFINE_EVENT(input_device_state, input_uninhibit_device,
+
+ TP_PROTO(struct input_dev *dev),
+
+ TP_ARGS(dev));
+
+#endif /* _TRACE_INPUT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.50.0On Thu, 10 Jul 2025 15:31:38 +0800
WangYuli <wangyuli@uniontech.com> wrote:
> diff --git a/include/trace/events/input.h b/include/trace/events/input.h
> new file mode 100644
> index 000000000000..3c5ffcfb7c8d
> --- /dev/null
> +++ b/include/trace/events/input.h
> @@ -0,0 +1,251 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* input tracepoints
> + *
> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
> + */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM input
> +
> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_INPUT_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/input.h>
> +
> +/**
> + * input_event - called when an input event is processed
> + * @dev: input device that generated the event
> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
> + * @code: event code within the type
> + * @value: event value
> + *
> + * This tracepoint fires for every input event processed by the input core.
> + * It can be used to monitor input device activity and debug input issues.
> + */
> +TRACE_EVENT(
> + input_event,
> +
> + TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
> + int value),
> +
> + TP_ARGS(dev, type, code, value),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
> + unsigned int, type) __field(unsigned int, code)
> + __field(int, value) __field(u16, bustype)
> + __field(u16, vendor)
> + __field(u16, product)),
> +
The contents of the tracepoints in the subsystems are determined by the
subsystem maintainers, but please follow the tracepoint formatting. The
above is horrible. It should look like a structure layout. One wouldn't
write:
struct entry { char *name;
unsigned int type; unsigned int code;
int value; u16 bustype;
u16 vendor;
u16 product; };
That's what the above looks like. It should be instead:
TP_STRUCT__entry(
__string( name, dev->name ?: "unknown" )
__field( unsigned int, type )
__field( unsigned int, code )
__field( int, value )
__field( u16, bustype )
__field( u16, vendor )
__field( u16, product )
),
So the fields can be easily visible and easily reviewed.
> + TP_fast_assign(__assign_str(name); __entry->type = type;
> + __entry->code = code; __entry->value = value;
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;),
> +
> + TP_printk(
> + "dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
> + __get_str(name), __entry->type, __entry->code, __entry->value,
> + __entry->bustype, __entry->vendor, __entry->product));
> +
> +/**
> + * input_device_register - called when an input device is registered
> + * @dev: input device being registered
> + *
> + * This tracepoint fires when a new input device is registered with the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(
> + input_device_register,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> + phys, dev->phys ?: "") __field(u16, bustype)
> + __field(u16, vendor) __field(u16, product)
> + __field(u16, version)),
Same here.
> +
> + TP_fast_assign(__assign_str(name); __assign_str(phys);
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;
> + __entry->version = dev->id.version;),
> +
> + TP_printk(
> + "dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
> + __get_str(name), __get_str(phys), __entry->bustype,
> + __entry->vendor, __entry->product, __entry->version));
> +
> +/**
> + * input_device_unregister - called when an input device is unregistered
> + * @dev: input device being unregistered
> + *
> + * This tracepoint fires when an input device is unregistered from the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(input_device_unregister,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> + phys, dev->phys ?: "") __field(u16, bustype)
> + __field(u16, vendor)
> + __field(u16, product)),
ditto.
> +
> + TP_fast_assign(__assign_str(name); __assign_str(phys);
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;),
> +
> + TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04x",
> + __get_str(name), __get_str(phys), __entry->bustype,
> + __entry->vendor, __entry->product));
> +
> +/**
> + * input_handler_connect - called when an input handler connects to a device
> + * @handler: input handler
> + * @dev: input device
> + * @minor: assigned minor number (if applicable)
> + *
> + * This tracepoint fires when an input handler (like evdev, mousedev) connects
> + * to an input device, creating a new input handle.
> + */
> +TRACE_EVENT(input_handler_connect,
> +
> + TP_PROTO(struct input_handler *handler, struct input_dev *dev,
> + int minor),
> +
> + TP_ARGS(handler, dev, minor),
> +
> + TP_STRUCT__entry(__string(handler_name, handler->name)
> + __string(dev_name, dev->name ?: "unknown")
> + __field(int, minor)),
ditto.
> +
> + TP_fast_assign(__assign_str(handler_name); __assign_str(dev_name);
> + __entry->minor = minor;),
> +
> + TP_printk("handler=%s dev=%s minor=%d", __get_str(handler_name),
> + __get_str(dev_name), __entry->minor));
> +
> +/**
> + * input_grab_device - called when a device is grabbed by a handler
> + * @dev: input device being grabbed
> + * @handle: input handle doing the grab
> + *
> + * This tracepoint fires when an input device is grabbed exclusively
> + * by an input handler, typically for security or special processing.
> + */
> +TRACE_EVENT(input_grab_device,
> +
> + TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> + TP_ARGS(dev, handle),
> +
> + TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> + __string(handler_name,
> + handle->handler->name)),
ditto.
> +
> + TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> + TP_printk("dev=%s grabbed_by=%s", __get_str(dev_name),
> + __get_str(handler_name)));
> +
> +/**
> + * input_release_device - called when a grabbed device is released
> + * @dev: input device being released
> + * @handle: input handle releasing the grab
> + *
> + * This tracepoint fires when an input device grab is released.
> + */
> +TRACE_EVENT(input_release_device,
> +
> + TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> + TP_ARGS(dev, handle),
> +
> + TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> + __string(handler_name,
> + handle->handler->name)),
ditto.
> +
> + TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> + TP_printk("dev=%s released_by=%s", __get_str(dev_name),
> + __get_str(handler_name)));
> +
> +DECLARE_EVENT_CLASS(input_device_state,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
> + __field(unsigned int, users)
> + __field(bool, inhibited)),
ditto.
-- Steve
> +
> + TP_fast_assign(__assign_str(name);
> + __entry->users = dev->users;
> + __entry->inhibited = dev->inhibited;),
> +
> + TP_printk("dev=%s users=%u inhibited=%s", __get_str(name),
> + __entry->users,
> + __entry->inhibited ? "true" : "false"));
> +
Hi Steve,
On 2025/7/23 08:25, Steven Rostedt wrote:
>> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
>> + unsigned int, type) __field(unsigned int, code)
>> + __field(int, value) __field(u16, bustype)
>> + __field(u16, vendor)
>> + __field(u16, product)),
>> +
> The contents of the tracepoints in the subsystems are determined by the
> subsystem maintainers, but please follow the tracepoint formatting. The
> above is horrible. It should look like a structure layout. One wouldn't
> write:
>
> struct entry { char *name;
> unsigned int type; unsigned int code;
> int value; u16 bustype;
> u16 vendor;
> u16 product; };
>
> That's what the above looks like. It should be instead:
>
> TP_STRUCT__entry(
> __string( name, dev->name ?: "unknown" )
> __field( unsigned int, type )
> __field( unsigned int, code )
> __field( int, value )
> __field( u16, bustype )
> __field( u16, vendor )
> __field( u16, product )
> ),
>
> So the fields can be easily visible and easily reviewed.
My apologies.
Since this was a new file I added, I didn't carefully check it after
applying clang-format, which led to this issue.
I'll fix the code formatting and send a patch v2.
--
WangYuli*
*
On 2025-07-22 20:25, Steven Rostedt wrote: > On Thu, 10 Jul 2025 15:31:38 +0800 > WangYuli <wangyuli@uniontech.com> wrote: > >> diff --git a/include/trace/events/input.h b/include/trace/events/input.h >> new file mode 100644 >> index 000000000000..3c5ffcfb7c8d >> --- /dev/null >> +++ b/include/trace/events/input.h >> @@ -0,0 +1,251 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* input tracepoints >> + * >> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com> >> + */ >> +#undef TRACE_SYSTEM >> +#define TRACE_SYSTEM input >> + >> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ) >> +#define _TRACE_INPUT_H >> + >> +#include <linux/tracepoint.h> >> +#include <linux/input.h> >> + >> +/** >> + * input_event - called when an input event is processed >> + * @dev: input device that generated the event >> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.) >> + * @code: event code within the type >> + * @value: event value >> + * >> + * This tracepoint fires for every input event processed by the input core. >> + * It can be used to monitor input device activity and debug input issues. >> + */ I've always been worried about adding tracepoint instrumentation of the input subsystem that includes the actual keystrokes into the event payload. What I'm trying to avoid here is people leaking their password by mistake just because they happened to record a trace while typing on their keyboard. I don't mind if this gets enabled with a new kernel command line options "tracing_leak_my_credentials=yes" or such, but I'd try to avoid making it easy to enable by mistake unless this information is specifically needed. But maybe I'm being too careful and people should really learn not to share kernel traces with others. Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
Hi Mathieu, On 2025/7/23 09:24, Mathieu Desnoyers wrote: > I've always been worried about adding tracepoint instrumentation of the > input subsystem that includes the actual keystrokes into the event > payload. What I'm trying to avoid here is people leaking their password > by mistake just because they happened to record a trace while > typing on their keyboard. > The evtest tool can also do this. However, it doesn't fully report all events from the input subsystem. From a debugging perspective, adding tracepoints to the input subsystem is still more convenient for debugging. > I don't mind if this gets enabled with a new kernel command line > options "tracing_leak_my_credentials=yes" or such, but I'd try to > avoid making it easy to enable by mistake unless this information > is specifically needed. > I'm not sure if this is over-engineering... I feel that adding too many command-line parameters will increase the user's cognitive load. However, the leakage of keyboard input records is indeed a very, very significant risk. As a compromise, would it be better if we added a separate Kconfig option specifically for the input subsystem's tracepoints to decide whether to enable them at compile time, and then documented the potential risks within that Kconfig's description? > But maybe I'm being too careful and people should really learn not > to share kernel traces with others. > > Thoughts ? > Thanks, -- WangYuli
On 2025-07-28 03:07, WangYuli wrote: > Hi Mathieu, > > On 2025/7/23 09:24, Mathieu Desnoyers wrote: >> I've always been worried about adding tracepoint instrumentation of the >> input subsystem that includes the actual keystrokes into the event >> payload. What I'm trying to avoid here is people leaking their password >> by mistake just because they happened to record a trace while >> typing on their keyboard. >> > The evtest tool can also do this. > > However, it doesn't fully report all events from the input subsystem. > > From a debugging perspective, adding tracepoints to the input subsystem > is still more convenient for debugging. > >> I don't mind if this gets enabled with a new kernel command line >> options "tracing_leak_my_credentials=yes" or such, but I'd try to >> avoid making it easy to enable by mistake unless this information >> is specifically needed. >> > I'm not sure if this is over-engineering... > > I feel that adding too many command-line parameters will increase the > user's cognitive load. > > However, the leakage of keyboard input records is indeed a very, very > significant risk. > > As a compromise, would it be better if we added a separate Kconfig > option specifically for the input subsystem's tracepoints to decide > whether to enable them at compile time, and then documented the > potential risks within that Kconfig's description? In term of mechanism to select keylogger enabling/disabling, I can think of the following options: - Kconfig option, - kernel command line parameter, - sysctl Here is a possibly incomplete list of desiderata for this: - Keylogger should be disabled by default. - System administrator should be able to enable keylogger at boot. - Users should be able to query the state of keylogger (enabled/disabled) during kernel execution, and this should be invariant until reboot, - Selecting whether this option is enabled or not should be decided by the system administrator, not by the distribution vendors who compile the distro kernels. - Prevent use of a kernel tracer as a keylogger by mistake without having the system administrator explicitly enable keylogging. The most flexible approach would be a sysctl, because it would allow a system administrator to enable this while the system runs. But it is somewhat redundant with the fact that the tracers allow disabling specific events dynamically. Also I don't think we would want to allow changing this configuration after system boots, so users interacting with a production system can check whether this is enabled or not to learn whether they can trust that this keylogger feature is disabled. This leaves Kconfig option and kernel command line. The downside of the Kconfig option is that it requires to choose the configuration up front for a distro kernel, not allowing the system admin to select the behavior at boot time without recompiling a custom kernel. This leaves the kernel command line option, which I think is a good tradeoff. It allows sysadmins to enable keylogging at boot from a distro kernel without recompiling their own kernel. It also prevents enabling keylogging in a production system by mistake after bootup. It allows users to inspect the status of this knob by looking at the kernel command line to know whether they are interacting with a system that has this keylogging enabled. Thoughts ? Thanks, Mathieu >> But maybe I'm being too careful and people should really learn not >> to share kernel traces with others. >> >> Thoughts ? >> > Thanks, -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
© 2016 - 2025 Red Hat, Inc.