| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6ebeb4e7-e446-45aa-a0bb-fc96d1431580@kernel.org>
Date: Sat, 29 Nov 2025 15:48:57 +0100
From: Hans de Goede <hansg@...nel.org>
To: Vishnu Sankar <vishnuocv@...il.com>, corbet@....net,
dmitry.torokhov@...il.com, hmh@....eng.br, derekjohn.clark@...il.com,
ilpo.jarvinen@...ux.intel.com
Cc: mpearson-lenovo@...ebb.ca, linux-doc@...r.kernel.org,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
ibm-acpi-devel@...ts.sourceforge.net, platform-driver-x86@...r.kernel.org,
vsankar@...ovo.com
Subject: Re: [PATCH v4 2/3] platform/x86: thinkpad_acpi - Add doubletap_filter
sysfs interface
Hi Vishnu,
On 29-Nov-25 1:25 AM, Vishnu Sankar wrote:
> Add sysfs interface for controlling TrackPoint doubletap event filtering.
> This allows userspace to enable/disable doubletap functionality and
> query the current state.
>
> The attribute is available at:
> /sys/devices/platform/thinkpad_acpi/doubletap_filter
>
> When set to 1, doubletap events are filtered out (ignored).
> When set to 0, doubletap events are processed (default).
>
> This complements the automatic hardware enablement in the trackpoint
> driver by providing user control over event processing at the kernel level.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@...il.com>
> Suggested-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
> ---
> Changes in v2:
> - Updated commit message to clarify dependency on trackpoint driver
> - Now handling sysfs read/write of trackpoint driver using file read/write
> - Removed sysfs attribute creation of trackpoint double tap here
> - Reversed the logic and return false right away
> - Dropped unnecessary debug messages
> - Using dev_dbg() instead of pr_xxxx()
>
> Changes in v3:
> - No changes
>
> Changes in v4:
> - Simplified approach: single sysfs attribute for user control
> - Clear naming: doubletap_filter instead of doubletap_enabled
> - Intuitive behavior: 0=process events, 1=filter events
> - No cross-driver dependencies or complex interactions
> - Minimal code changes using existing thinkpad_acpi infrastructure
> ---
> ---
> drivers/platform/x86/lenovo/thinkpad_acpi.c | 54 +++++++++++++++++++--
> 1 file changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index cc19fe520ea9..9b646ecff452 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -373,7 +373,7 @@ static struct {
> u32 hotkey_poll_active:1;
> u32 has_adaptive_kbd:1;
> u32 kbd_lang:1;
> - u32 trackpoint_doubletap:1;
> + u32 trackpoint_doubletap_filter:1;
Why the rename? The new name suggests that events will be filtered
when trackpoint_doubletap_filter = 1, so inverted of the old name
but when processing events events are let through when
trackpoint_doubletap_filter == 1 so 1 means _not_ filtering which
makes the name weird:
> + /* Only send the event if kernel-level filtering allows it */
> + if (tp_features.trackpoint_doubletap_filter)
> tpacpi_input_send_key(hkey, send_acpi_ev);
Maybe you forgot to add an '!' here, iow this should become:
/* Only send the event if kernel-level filtering allows it */
if (!tp_features.trackpoint_doubletap_filter)
tpacpi_input_send_key(hkey, send_acpi_ev);
?
> struct quirk_entry *quirks;
> } tp_features;
>
> @@ -3104,8 +3104,35 @@ static void tpacpi_send_radiosw_update(void)
> hotkey_radio_sw_notify_change();
> }
>
> +static ssize_t doubletap_filter_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%d\n", tp_features.trackpoint_doubletap_filter);
> +}
> +
> +static ssize_t doubletap_filter_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + bool filter;
> + int err;
> +
> + err = kstrtobool(buf, &filter);
> + if (err)
> + return err;
> +
> + tp_features.trackpoint_doubletap_filter = filter;
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(doubletap_filter);
> +
> static void hotkey_exit(void)
> {
> + if (tpacpi_pdev)
> + device_remove_file(&tpacpi_pdev->dev, &dev_attr_doubletap_filter);
> +
New code should not use device_create_file() / device_remove_file() instead
the attribute should be added to hotkey_attributes[] and then the driver
core will automatically add/remove it.
> mutex_lock(&hotkey_mutex);
> hotkey_poll_stop_sync();
> dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
> @@ -3557,8 +3584,22 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>
> hotkey_poll_setup_safe(true);
>
> - /* Enable doubletap by default */
> - tp_features.trackpoint_doubletap = 1;
> + /*
> + * Enable kernel-level doubletap event filtering by default.
> + * This allows doubletap events to reach userspace.
> + */
> + tp_features.trackpoint_doubletap_filter = 1;
Again this seems to be inverted, or if you really want
trackpoint_doubletap_filter to work as an enable flag
then please rename it to trackpoint_doubletap_enable.
> +
> + /* Create doubletap_filter sysfs attribute */
> + if (tpacpi_pdev) {
> + int err = device_create_file(&tpacpi_pdev->dev, &dev_attr_doubletap_filter);
> + if (err) {
> + pr_warn("Unable to create doubletap_filter sysfs attribute\n");
> + /* Continue even if sysfs creation fails */
> + } else {
> + pr_info("ThinkPad ACPI doubletap_filter sysfs attribute created\n");
> + }
> + }
This can be dropped (replaced by adding the attr to hotkey_attributes[].
Regards,
Hans
Powered by blists - more mailing lists