From: Aaron Armstrong Skomra <skomra@gmail.com>
To: linux-input@vger.kernel.org, jkosina@suse.cz,
pinglinux@gmail.com, killertofu@gmail.com,
benjamin.tissoires@redhat.com
Cc: Aaron Armstrong Skomra <skomra@gmail.com>,
"stable #4 . 9" <stable@vger.kernel.org>,
Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Subject: [PATCH] HID: wacom: EKR: ensure devres groups at higher indexes are released
Date: Thu, 7 Dec 2017 12:31:56 -0800 [thread overview]
Message-ID: <1512678716-31779-1-git-send-email-skomra@gmail.com> (raw)
Background: ExpressKey Remotes communicate their events via usb dongle.
Each dongle can hold up to 5 pairings at one time and one EKR (identified
by its serial number) can unfortunately be paired with its dongle
more than once. The pairing takes place in a round-robin fashion.
Input devices are only created once per EKR, when a new serial number
is seen in the list of pairings. However, if a device is created for
a "higher" paring index and subsequently a second pairing occurs at a
lower pairing index, unpairing the remote with that serial number from
any pairing index will currently cause a driver crash. This occurs
infrequently, as two remotes are necessary to trigger this bug and most
users have only one remote.
As an illustration, to trigger the bug you need to have two remotes,
and pair them in this order:
1. slot 0 -> remote 1 (input device created for remote 1)
2. slot 1 -> remote 1 (duplicate pairing - no device created)
3. slot 2 -> remote 1 (duplicate pairing - no device created)
4. slot 3 -> remote 1 (duplicate pairing - no device created)
5. slot 4 -> remote 2 (input device created for remote 2)
6. slot 0 -> remote 2 (1 destroyed and recreated at slot 1)
7. slot 1 -> remote 2 (1 destroyed and recreated at slot 2)
8. slot 2 -> remote 2 (1 destroyed and recreated at slot 3)
9. slot 3 -> remote 2 (1 destroyed and not recreated)
10. slot 4 -> remote 2 (2 was already in this slot so no changes)
11. slot 0 -> remote 1 (The current code sees remote 2 was paired over in
one of the dongle slots it occupied and attempts
to remove all information about remote 2 [1]. It
calls wacom_remote_destroy_one for remote 2, but
the destroy function assumes the lowest index is
where the remote's input device was created. The
code "cleans up" the other remote 2 pairings
including the one which the input device was based
on, assuming they were were just duplicate
pairings. However, the cleanup doesn't call the
devres release function for the input device that
was created in slot 4).
This issue is fixed by this commit.
[1] Remote 2 should subsequently be re-created on the next packet from the
EKR at the lowest numbered slot that it occupies (here slot 1).
Fixes: f9036bd43602 ("HID: wacom: EKR: use devres groups to manage resources")
Cc: stable <stable@vger.kernel.org> #4.9
Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
---
drivers/hid/wacom_sys.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index ee71ad9b6cc1..76531796bd3c 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2347,23 +2347,23 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
int i;
unsigned long flags;
- spin_lock_irqsave(&remote->remote_lock, flags);
- remote->remotes[index].registered = false;
- spin_unlock_irqrestore(&remote->remote_lock, flags);
+ for (i = 0; i < WACOM_MAX_REMOTES; i++) {
+ if (remote->remotes[i].serial == serial) {
- if (remote->remotes[index].battery.battery)
- devres_release_group(&wacom->hdev->dev,
- &remote->remotes[index].battery.bat_desc);
+ spin_lock_irqsave(&remote->remote_lock, flags);
+ remote->remotes[i].registered = false;
+ spin_unlock_irqrestore(&remote->remote_lock, flags);
- if (remote->remotes[index].group.name)
- devres_release_group(&wacom->hdev->dev,
- &remote->remotes[index]);
+ if (remote->remotes[i].battery.battery)
+ devres_release_group(&wacom->hdev->dev,
+ &remote->remotes[i].battery.bat_desc);
+
+ if (remote->remotes[i].group.name)
+ devres_release_group(&wacom->hdev->dev,
+ &remote->remotes[i]);
- for (i = 0; i < WACOM_MAX_REMOTES; i++) {
- if (remote->remotes[i].serial == serial) {
remote->remotes[i].serial = 0;
remote->remotes[i].group.name = NULL;
- remote->remotes[i].registered = false;
remote->remotes[i].battery.battery = NULL;
wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
}
--
2.7.4
next reply other threads:[~2017-12-07 20:31 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-07 20:31 Aaron Armstrong Skomra [this message]
2018-01-23 14:30 ` [PATCH] HID: wacom: EKR: ensure devres groups at higher indexes are released Jiri Kosina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1512678716-31779-1-git-send-email-skomra@gmail.com \
--to=skomra@gmail.com \
--cc=aaron.skomra@wacom.com \
--cc=benjamin.tissoires@redhat.com \
--cc=jkosina@suse.cz \
--cc=killertofu@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=pinglinux@gmail.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).