Skip to content

Commit

Permalink
platform/x86: intel-vbtn: Protect ACPI notify handler against recursion
Browse files Browse the repository at this point in the history
Since commit e2ffcda ("ACPI: OSL: Allow Notify () handlers to run on
all CPUs") ACPI notify handlers like the intel-vbtn notify_handler() may
run on multiple CPU cores racing with themselves.

This race gets hit on Dell Venue 7140 tablets when undocking from
the keyboard, causing the handler to try and register priv->switches_dev
twice, as can be seen from the dev_info() message getting logged twice:

[ 83.861800] intel-vbtn INT33D6:00: Registering Intel Virtual Switches input-dev after receiving a switch event
[ 83.861858] input: Intel Virtual Switches as /devices/pci0000:00/0000:00:1f.0/PNP0C09:00/INT33D6:00/input/input17
[ 83.861865] intel-vbtn INT33D6:00: Registering Intel Virtual Switches input-dev after receiving a switch event

After which things go seriously wrong:
[ 83.861872] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:1f.0/PNP0C09:00/INT33D6:00/input/input17'
...
[ 83.861967] kobject: kobject_add_internal failed for input17 with -EEXIST, don't try to register things with the same name in the same directory.
[ 83.877338] BUG: kernel NULL pointer dereference, address: 0000000000000018
...

Protect intel-vbtn notify_handler() from racing with itself with a mutex
to fix this.

Fixes: e2ffcda ("ACPI: OSL: Allow Notify () handlers to run on all CPUs")
Reported-by: En-Wei Wu <en-wei.wu@canonical.com>
Closes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2073001
Tested-by: Kostadin Stoilov <kmstoilov@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20240729120443.14779-1-hdegoede@redhat.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
  • Loading branch information
Hans de Goede authored and Ilpo Järvinen committed Jul 30, 2024
1 parent 8400291 commit e075c3b
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions drivers/platform/x86/intel/vbtn.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
*/

#include <linux/acpi.h>
#include <linux/cleanup.h>
#include <linux/dmi.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/suspend.h>
#include "../dual_accel_detect.h"
Expand Down Expand Up @@ -66,6 +68,7 @@ static const struct key_entry intel_vbtn_switchmap[] = {
};

struct intel_vbtn_priv {
struct mutex mutex; /* Avoid notify_handler() racing with itself */
struct input_dev *buttons_dev;
struct input_dev *switches_dev;
bool dual_accel;
Expand Down Expand Up @@ -155,6 +158,8 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
bool autorelease;
int ret;

guard(mutex)(&priv->mutex);

if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
if (!priv->has_buttons) {
dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
Expand Down Expand Up @@ -290,6 +295,10 @@ static int intel_vbtn_probe(struct platform_device *device)
return -ENOMEM;
dev_set_drvdata(&device->dev, priv);

err = devm_mutex_init(&device->dev, &priv->mutex);
if (err)
return err;

priv->dual_accel = dual_accel;
priv->has_buttons = has_buttons;
priv->has_switches = has_switches;
Expand Down

0 comments on commit e075c3b

Please sign in to comment.