| 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: <996ee62d-7a0e-5058-6ebe-0a9c51d3ea99@linux.intel.com>
Date: Fri, 10 Jan 2025 16:38:56 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Armin Wolf <W_Armin@....de>
cc: dmitry.torokhov@...il.com, o2g.org.ru@...il.com,
Hans de Goede <hdegoede@...hat.com>, corentin.chary@...il.com,
luke@...nes.dev, mjg59@...f.ucam.org, pali@...nel.org,
eric.piel@...mplin-utc.net, jlee@...e.com, kenneth.t.chan@...il.com,
coproscefalo@...il.com, linux-input@...r.kernel.org,
platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
josh@...huagrisham.com
Subject: Re: [PATCH v2] Input: i8042 - Add support for platform filter
contexts
On Thu, 2 Jan 2025, Armin Wolf wrote:
> Am 22.12.24 um 22:50 schrieb Armin Wolf:
>
> > Currently the platform filter cannot access any driver-specific state
> > which forces drivers installing a i8042 filter to have at least some
> > kind of global pointer for their filter.
> >
> > This however might cause issues should such a driver probe multiple
> > devices. Fix this by allowing callers of i8042_install_filter() to
> > submit a context pointer which is then passed to the i8042 filter.
> >
> > Also introduce a separate type for the i8042 filter (i8042_filter_t)
> > so that the function definitions can stay compact.
> >
> > Tested on a Dell Inspiron 3505.
>
> Any updates on this?
We haven't heard what Dimitry thinks of the reasonale you gave in v1.
I'd like to have this as it allows us clean up the globals on pdx86 side
even if i8042 is still limited to a single filter (and uses globals
itself).
--
i.
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > Signed-off-by: Armin Wolf <W_Armin@....de>
> > ---
> > Changes since v1:
> > - add kerneldoc for new typedef
> > - add Reviewed-by tag
> > ---
> > drivers/input/misc/ideapad_slidebar.c | 4 ++--
> > drivers/input/serio/i8042.c | 17 ++++++++-------
> > drivers/platform/x86/asus-nb-wmi.c | 3 ++-
> > drivers/platform/x86/asus-wmi.c | 2 +-
> > drivers/platform/x86/asus-wmi.h | 3 +--
> > drivers/platform/x86/dell/dell-laptop.c | 6 +++---
> > drivers/platform/x86/hp/hp_accel.c | 4 ++--
> > drivers/platform/x86/msi-laptop.c | 6 +++---
> > drivers/platform/x86/panasonic-laptop.c | 4 ++--
> > drivers/platform/x86/toshiba_acpi.c | 4 ++--
> > include/linux/i8042.h | 28 ++++++++++++++++++-------
> > 11 files changed, 48 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/input/misc/ideapad_slidebar.c
> > b/drivers/input/misc/ideapad_slidebar.c
> > index f6e5fc807b4d..ab2e0a401904 100644
> > --- a/drivers/input/misc/ideapad_slidebar.c
> > +++ b/drivers/input/misc/ideapad_slidebar.c
> > @@ -121,7 +121,7 @@ static void slidebar_mode_set(u8 mode)
> > }
> >
> > static bool slidebar_i8042_filter(unsigned char data, unsigned char str,
> > - struct serio *port)
> > + struct serio *port, void *context)
> > {
> > static bool extended = false;
> >
> > @@ -219,7 +219,7 @@ static int __init ideapad_probe(struct platform_device*
> > pdev)
> > input_set_capability(slidebar_input_dev, EV_ABS, ABS_X);
> > input_set_abs_params(slidebar_input_dev, ABS_X, 0, 0xff, 0, 0);
> >
> > - err = i8042_install_filter(slidebar_i8042_filter);
> > + err = i8042_install_filter(slidebar_i8042_filter, NULL);
> > if (err) {
> > dev_err(&pdev->dev,
> > "Failed to install i8042 filter: %d\n", err);
> > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> > index 509330a27880..cab5a4c5baf5 100644
> > --- a/drivers/input/serio/i8042.c
> > +++ b/drivers/input/serio/i8042.c
> > @@ -179,8 +179,8 @@ static struct platform_device *i8042_platform_device;
> > static struct notifier_block i8042_kbd_bind_notifier_block;
> >
> > static bool i8042_handle_data(int irq);
> > -static bool (*i8042_platform_filter)(unsigned char data, unsigned char str,
> > - struct serio *serio);
> > +static i8042_filter_t i8042_platform_filter;
> > +static void *i8042_platform_filter_context;
> >
> > void i8042_lock_chip(void)
> > {
> > @@ -194,8 +194,7 @@ void i8042_unlock_chip(void)
> > }
> > EXPORT_SYMBOL(i8042_unlock_chip);
> >
> > -int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char
> > str,
> > - struct serio *serio))
> > +int i8042_install_filter(i8042_filter_t filter, void *context)
> > {
> > guard(spinlock_irqsave)(&i8042_lock);
> >
> > @@ -203,12 +202,12 @@ int i8042_install_filter(bool (*filter)(unsigned char
> > data, unsigned char str,
> > return -EBUSY;
> >
> > i8042_platform_filter = filter;
> > + i8042_platform_filter_context = context;
> > return 0;
> > }
> > EXPORT_SYMBOL(i8042_install_filter);
> >
> > -int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char
> > str,
> > - struct serio *port))
> > +int i8042_remove_filter(i8042_filter_t filter)
> > {
> > guard(spinlock_irqsave)(&i8042_lock);
> >
> > @@ -216,6 +215,7 @@ int i8042_remove_filter(bool (*filter)(unsigned char
> > data, unsigned char str,
> > return -EINVAL;
> >
> > i8042_platform_filter = NULL;
> > + i8042_platform_filter_context = NULL;
> > return 0;
> > }
> > EXPORT_SYMBOL(i8042_remove_filter);
> > @@ -480,7 +480,10 @@ static bool i8042_filter(unsigned char data, unsigned
> > char str,
> > }
> > }
> >
> > - if (i8042_platform_filter && i8042_platform_filter(data, str, serio))
> > {
> > + if (!i8042_platform_filter)
> > + return false;
> > +
> > + if (i8042_platform_filter(data, str, serio,
> > i8042_platform_filter_context)) {
> > dbg("Filtered out by platform filter\n");
> > return true;
> > }
> > diff --git a/drivers/platform/x86/asus-nb-wmi.c
> > b/drivers/platform/x86/asus-nb-wmi.c
> > index ef04d396f61c..a3d4b98045f8 100644
> > --- a/drivers/platform/x86/asus-nb-wmi.c
> > +++ b/drivers/platform/x86/asus-nb-wmi.c
> > @@ -50,7 +50,8 @@ MODULE_PARM_DESC(tablet_mode_sw, "Tablet mode detect:
> > -1:auto 0:disable 1:kbd-do
> > static struct quirk_entry *quirks;
> > static bool atkbd_reports_vol_keys;
> >
> > -static bool asus_i8042_filter(unsigned char data, unsigned char str, struct
> > serio *port)
> > +static bool asus_i8042_filter(unsigned char data, unsigned char str, struct
> > serio *port,
> > + void *context)
> > {
> > static bool extended_e0;
> > static bool extended_e1;
> > diff --git a/drivers/platform/x86/asus-wmi.c
> > b/drivers/platform/x86/asus-wmi.c
> > index fdeebab96fc0..6c674de60ec0 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -4824,7 +4824,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> > }
> >
> > if (asus->driver->i8042_filter) {
> > - err = i8042_install_filter(asus->driver->i8042_filter);
> > + err = i8042_install_filter(asus->driver->i8042_filter, NULL);
> > if (err)
> > pr_warn("Unable to install key filter - %d\n", err);
> > }
> > diff --git a/drivers/platform/x86/asus-wmi.h
> > b/drivers/platform/x86/asus-wmi.h
> > index d02f15fd3482..018dfde4025e 100644
> > --- a/drivers/platform/x86/asus-wmi.h
> > +++ b/drivers/platform/x86/asus-wmi.h
> > @@ -73,8 +73,7 @@ struct asus_wmi_driver {
> > void (*key_filter) (struct asus_wmi_driver *driver, int *code,
> > unsigned int *value, bool *autorelease);
> > /* Optional standard i8042 filter */
> > - bool (*i8042_filter)(unsigned char data, unsigned char str,
> > - struct serio *serio);
> > + i8042_filter_t i8042_filter;
> >
> > int (*probe) (struct platform_device *device);
> > void (*detect_quirks) (struct asus_wmi_driver *driver);
> > diff --git a/drivers/platform/x86/dell/dell-laptop.c
> > b/drivers/platform/x86/dell/dell-laptop.c
> > index 5671bd0deee7..58b860b88fff 100644
> > --- a/drivers/platform/x86/dell/dell-laptop.c
> > +++ b/drivers/platform/x86/dell/dell-laptop.c
> > @@ -725,8 +725,8 @@ static void dell_update_rfkill(struct work_struct
> > *ignored)
> > }
> > static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
> >
> > -static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
> > - struct serio *port)
> > +static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
> > struct serio *port,
> > + void *context)
> > {
> > static bool extended;
> >
> > @@ -884,7 +884,7 @@ static int __init dell_setup_rfkill(void)
> > pr_warn("Unable to register dell rbtn notifier\n");
> > goto err_filter;
> > } else {
> > - ret = i8042_install_filter(dell_laptop_i8042_filter);
> > + ret = i8042_install_filter(dell_laptop_i8042_filter, NULL);
> > if (ret) {
> > pr_warn("Unable to install key filter\n");
> > goto err_filter;
> > diff --git a/drivers/platform/x86/hp/hp_accel.c
> > b/drivers/platform/x86/hp/hp_accel.c
> > index 39a6530f5072..10d5af18d639 100644
> > --- a/drivers/platform/x86/hp/hp_accel.c
> > +++ b/drivers/platform/x86/hp/hp_accel.c
> > @@ -267,7 +267,7 @@ static struct delayed_led_classdev hpled_led = {
> > };
> >
> > static bool hp_accel_i8042_filter(unsigned char data, unsigned char str,
> > - struct serio *port)
> > + struct serio *port, void *context)
> > {
> > static bool extended;
> >
> > @@ -326,7 +326,7 @@ static int lis3lv02d_probe(struct platform_device
> > *device)
> > /* filter to remove HPQ6000 accelerometer data
> > * from keyboard bus stream */
> > if (strstr(dev_name(&device->dev), "HPQ6000"))
> > - i8042_install_filter(hp_accel_i8042_filter);
> > + i8042_install_filter(hp_accel_i8042_filter, NULL);
> >
> > INIT_WORK(&hpled_led.work, delayed_set_status_worker);
> > ret = led_classdev_register(NULL, &hpled_led.led_classdev);
> > diff --git a/drivers/platform/x86/msi-laptop.c
> > b/drivers/platform/x86/msi-laptop.c
> > index e5391a37014d..c4b150fa093f 100644
> > --- a/drivers/platform/x86/msi-laptop.c
> > +++ b/drivers/platform/x86/msi-laptop.c
> > @@ -806,8 +806,8 @@ static void msi_send_touchpad_key(struct work_struct
> > *ignored)
> > }
> > static DECLARE_DELAYED_WORK(msi_touchpad_dwork, msi_send_touchpad_key);
> >
> > -static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> > - struct serio *port)
> > +static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> > struct serio *port,
> > + void *context)
> > {
> > static bool extended;
> >
> > @@ -996,7 +996,7 @@ static int __init load_scm_model_init(struct
> > platform_device *sdev)
> > if (result)
> > goto fail_input;
> >
> > - result = i8042_install_filter(msi_laptop_i8042_filter);
> > + result = i8042_install_filter(msi_laptop_i8042_filter, NULL);
> > if (result) {
> > pr_err("Unable to install key filter\n");
> > goto fail_filter;
> > diff --git a/drivers/platform/x86/panasonic-laptop.c
> > b/drivers/platform/x86/panasonic-laptop.c
> > index 22ca70eb8227..2987b4db6009 100644
> > --- a/drivers/platform/x86/panasonic-laptop.c
> > +++ b/drivers/platform/x86/panasonic-laptop.c
> > @@ -260,7 +260,7 @@ struct pcc_acpi {
> > * keypress events over the PS/2 kbd interface, filter these out.
> > */
> > static bool panasonic_i8042_filter(unsigned char data, unsigned char str,
> > - struct serio *port)
> > + struct serio *port, void *context)
> > {
> > static bool extended;
> >
> > @@ -1100,7 +1100,7 @@ static int acpi_pcc_hotkey_add(struct acpi_device
> > *device)
> > pcc->platform = NULL;
> > }
> >
> > - i8042_install_filter(panasonic_i8042_filter);
> > + i8042_install_filter(panasonic_i8042_filter, NULL);
> > return 0;
> >
> > out_platform:
> > diff --git a/drivers/platform/x86/toshiba_acpi.c
> > b/drivers/platform/x86/toshiba_acpi.c
> > index 78a5aac2dcfd..5ad3a7183d33 100644
> > --- a/drivers/platform/x86/toshiba_acpi.c
> > +++ b/drivers/platform/x86/toshiba_acpi.c
> > @@ -2755,7 +2755,7 @@ static int toshiba_acpi_enable_hotkeys(struct
> > toshiba_acpi_dev *dev)
> > }
> >
> > static bool toshiba_acpi_i8042_filter(unsigned char data, unsigned char
> > str,
> > - struct serio *port)
> > + struct serio *port, void *context)
> > {
> > if (str & I8042_STR_AUXDATA)
> > return false;
> > @@ -2915,7 +2915,7 @@ static int toshiba_acpi_setup_keyboard(struct
> > toshiba_acpi_dev *dev)
> > if (ec_handle && acpi_has_method(ec_handle, "NTFY")) {
> > INIT_WORK(&dev->hotkey_work, toshiba_acpi_hotkey_work);
> >
> > - error = i8042_install_filter(toshiba_acpi_i8042_filter);
> > + error = i8042_install_filter(toshiba_acpi_i8042_filter, NULL);
> > if (error) {
> > pr_err("Error installing key filter\n");
> > goto err_free_dev;
> > diff --git a/include/linux/i8042.h b/include/linux/i8042.h
> > index 95b07f8b77fe..00037c13abc8 100644
> > --- a/include/linux/i8042.h
> > +++ b/include/linux/i8042.h
> > @@ -54,15 +54,29 @@
> >
> > struct serio;
> >
> > +/**
> > + * typedef i8042_filter_t - i8042 filter callback
> > + * @data: Data received by the i8042 controller
> > + * @str: Status register of the i8042 controller
> > + * @serio: Serio of the i8042 controller
> > + * @context: Context pointer associated with this callback
> > + *
> > + * This represents a i8042 filter callback which can be used with
> > i8042_install_filter()
> > + * and i8042_remove_filter() to filter the i8042 input for
> > platform-specific key codes.
> > + *
> > + * Context: Interrupt context.
> > + * Returns: true if the data should be filtered out, false if otherwise.
> > + */
> > +typedef bool (*i8042_filter_t)(unsigned char data, unsigned char str,
> > struct serio *serio,
> > + void *context);
> > +
> > #if defined(CONFIG_SERIO_I8042) || defined(CONFIG_SERIO_I8042_MODULE)
> >
> > void i8042_lock_chip(void);
> > void i8042_unlock_chip(void);
> > int i8042_command(unsigned char *param, int command);
> > -int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char
> > str,
> > - struct serio *serio));
> > -int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char
> > str,
> > - struct serio *serio));
> > +int i8042_install_filter(i8042_filter_t filter, void *context);
> > +int i8042_remove_filter(i8042_filter_t filter);
> >
> > #else
> >
> > @@ -79,14 +93,12 @@ static inline int i8042_command(unsigned char *param,
> > int command)
> > return -ENODEV;
> > }
> >
> > -static inline int i8042_install_filter(bool (*filter)(unsigned char data,
> > unsigned char str,
> > - struct serio *serio))
> > +static inline int i8042_install_filter(i8042_filter_t filter, void
> > *context)
> > {
> > return -ENODEV;
> > }
> >
> > -static inline int i8042_remove_filter(bool (*filter)(unsigned char data,
> > unsigned char str,
> > - struct serio *serio))
> > +static inline int i8042_remove_filter(i8042_filter_t filter)
> > {
> > return -ENODEV;
> > }
> > --
> > 2.39.5
> >
> >
>
Powered by blists - more mailing lists