* [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver
@ 2014-02-20 16:35 Frank Praznik
2014-02-20 16:36 ` [PATCH v2 1/5] HID: sony: Fix multi-line comment styling Frank Praznik
` (6 more replies)
0 siblings, 7 replies; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:35 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
v2 of this patch set addresses the code review issues raised in v1 as well as
adding a patch that fixes the styling of multi-line comments to conform to
kernel coding standards, adds a check to protect against a potential
out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
that a device module now depends on the current behavior of the uniq string.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/5] HID: sony: Fix multi-line comment styling
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
@ 2014-02-20 16:36 ` Frank Praznik
2014-02-20 16:36 ` [PATCH v2 2/5] HID: sony: Fix work queue issues Frank Praznik
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:36 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
Convert multi-line comments to comply with the kernel coding style.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 45 ++++++++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 526705f..26992e1 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -17,7 +17,8 @@
* any later version.
*/
-/* NOTE: in order for the Sony PS3 BD Remote Control to be found by
+/*
+ * NOTE: in order for the Sony PS3 BD Remote Control to be found by
* a Bluetooth host, the key combination Start+Enter has to be kept pressed
* for about 7 seconds with the Bluetooth Host Controller in discovering mode.
*
@@ -81,7 +82,8 @@ static const u8 sixaxis_rdesc_fixup2[] = {
0xb1, 0x02, 0xc0, 0xc0,
};
-/* The default descriptor doesn't provide mapping for the accelerometers
+/*
+ * The default descriptor doesn't provide mapping for the accelerometers
* or orientation sensors. This fixed descriptor maps the accelerometers
* to usage values 0x40, 0x41 and 0x42 and maps the orientation sensors
* to usage values 0x43, 0x44 and 0x45.
@@ -340,7 +342,8 @@ static u8 dualshock4_usb_rdesc[] = {
0xC0 /* End Collection */
};
-/* The default behavior of the Dualshock 4 is to send reports using report
+/*
+ * The default behavior of the Dualshock 4 is to send reports using report
* type 1 when running over Bluetooth. However, as soon as it receives a
* report of type 17 to set the LEDs or rumble it starts returning it's state
* in report 17 instead of 1. Since report 17 is undefined in the default HID
@@ -667,7 +670,8 @@ static const unsigned int ps3remote_keymap_remote_buttons[] = {
};
static const unsigned int buzz_keymap[] = {
- /* The controller has 4 remote buzzers, each with one LED and 5
+ /*
+ * The controller has 4 remote buzzers, each with one LED and 5
* buttons.
*
* We use the mapping chosen by the controller, which is:
@@ -838,7 +842,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
unsigned long flags;
__u8 cable_state, battery_capacity, battery_charging;
- /* The sixaxis is charging if the battery value is 0xee
+ /*
+ * The sixaxis is charging if the battery value is 0xee
* and it is fully charged if the value is 0xef.
* It does not report the actual level while charging so it
* is set to 100% while charging is in progress.
@@ -868,18 +873,21 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
int n, offset;
__u8 cable_state, battery_capacity, battery_charging;
- /* Battery and touchpad data starts at byte 30 in the USB report and
+ /*
+ * Battery and touchpad data starts at byte 30 in the USB report and
* 32 in Bluetooth report.
*/
offset = (sc->quirks & DUALSHOCK4_CONTROLLER_USB) ? 30 : 32;
- /* The lower 4 bits of byte 30 contain the battery level
+ /*
+ * The lower 4 bits of byte 30 contain the battery level
* and the 5th bit contains the USB cable state.
*/
cable_state = (rd[offset] >> 4) & 0x01;
battery_capacity = rd[offset] & 0x0F;
- /* When a USB power source is connected the battery level ranges from
+ /*
+ * When a USB power source is connected the battery level ranges from
* 0 to 10, and when running on battery power it ranges from 0 to 9.
* A battery level above 10 when plugged in means charge completed.
*/
@@ -903,7 +911,8 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
offset += 5;
- /* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB
+ /*
+ * The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB
* and 37 on Bluetooth.
* The first 7 bits of the first byte is a counter and bit 8 is a touch
* indicator that is 0 when pressed and 1 when not pressed.
@@ -932,7 +941,8 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
{
struct sony_sc *sc = hid_get_drvdata(hdev);
- /* Sixaxis HID report has acclerometers/gyro with MSByte first, this
+ /*
+ * Sixaxis HID report has acclerometers/gyro with MSByte first, this
* has to be BYTE_SWAPPED before passing up to joystick interface
*/
if ((sc->quirks & SIXAXIS_CONTROLLER) && rd[0] == 0x01 && size == 49) {
@@ -1057,7 +1067,8 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
HID_FEATURE_REPORT);
}
-/* Requesting feature report 0x02 in Bluetooth mode changes the state of the
+/*
+ * Requesting feature report 0x02 in Bluetooth mode changes the state of the
* controller so that it sends full input reports of type 0x11.
*/
static int dualshock4_set_operational_bt(struct hid_device *hdev)
@@ -1211,9 +1222,11 @@ static int sony_leds_init(struct hid_device *hdev)
name_fmt = "%s::sony%d";
}
- /* Clear LEDs as we have no way of reading their initial state. This is
+ /*
+ * Clear LEDs as we have no way of reading their initial state. This is
* only relevant if the driver is loaded after somebody actively set the
- * LEDs to on */
+ * LEDs to on
+ */
sony_set_leds(hdev, initial_values, drv_data->led_count);
name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
@@ -1420,7 +1433,8 @@ static int sony_battery_probe(struct sony_sc *sc)
struct hid_device *hdev = sc->hdev;
int ret;
- /* Set the default battery level to 100% to avoid low battery warnings
+ /*
+ * Set the default battery level to 100% to avoid low battery warnings
* if the battery is polled before the first device report is received.
*/
sc->battery_capacity = 100;
@@ -1533,7 +1547,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_stop;
}
}
- /* The Dualshock 4 touchpad supports 2 touches and has a
+ /*
+ * The Dualshock 4 touchpad supports 2 touches and has a
* resolution of 1920x940.
*/
ret = sony_register_touchpad(sc, 2, 1920, 940);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/5] HID: sony: Fix work queue issues
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
2014-02-20 16:36 ` [PATCH v2 1/5] HID: sony: Fix multi-line comment styling Frank Praznik
@ 2014-02-20 16:36 ` Frank Praznik
2014-02-20 16:36 ` [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index Frank Praznik
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:36 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
Only initialize force feedback for devices that actually support it (Sixaxis and
Dualshock 4) to prevent calls to schedule_work() with an uninitialized work
queue.
Move the cancel_work_sync() call out of sony_destroy_ff() since the state worker
is used for the LEDs even when force-feedback is disabled.
Remove the sony_destroy_ff() function since it is no longer used.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
This is functionally the same as the 3.14 fix but updated for the 3.15 codebase
drivers/hid/hid-sony.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 26992e1..a51a9c0 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -51,6 +51,7 @@
#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER | BUZZ_CONTROLLER |\
DUALSHOCK4_CONTROLLER)
#define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER)
+#define SONY_FF_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER)
#define MAX_LEDS 4
@@ -729,6 +730,7 @@ struct sony_sc {
__u8 right;
#endif
+ __u8 worker_initialized;
__u8 cable_state;
__u8 battery_charging;
__u8 battery_capacity;
@@ -1367,22 +1369,12 @@ static int sony_init_ff(struct hid_device *hdev)
return input_ff_create_memless(input_dev, NULL, sony_play_effect);
}
-static void sony_destroy_ff(struct hid_device *hdev)
-{
- struct sony_sc *sc = hid_get_drvdata(hdev);
-
- cancel_work_sync(&sc->state_worker);
-}
-
#else
static int sony_init_ff(struct hid_device *hdev)
{
return 0;
}
-static void sony_destroy_ff(struct hid_device *hdev)
-{
-}
#endif
static int sony_battery_get_property(struct power_supply *psy,
@@ -1535,9 +1527,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
ret = sixaxis_set_operational_usb(hdev);
+ sc->worker_initialized = 1;
INIT_WORK(&sc->state_worker, sixaxis_state_worker);
} else if (sc->quirks & SIXAXIS_CONTROLLER_BT) {
ret = sixaxis_set_operational_bt(hdev);
+ sc->worker_initialized = 1;
INIT_WORK(&sc->state_worker, sixaxis_state_worker);
} else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) {
@@ -1555,6 +1549,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret < 0)
goto err_stop;
+ sc->worker_initialized = 1;
INIT_WORK(&sc->state_worker, dualshock4_state_worker);
} else {
ret = 0;
@@ -1582,9 +1577,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
}
- ret = sony_init_ff(hdev);
- if (ret < 0)
- goto err_close;
+ if (sc->quirks & SONY_FF_SUPPORT) {
+ ret = sony_init_ff(hdev);
+ if (ret < 0)
+ goto err_close;
+ }
return 0;
err_close:
@@ -1594,6 +1591,8 @@ err_stop:
sony_leds_remove(hdev);
if (sc->quirks & SONY_BATTERY_SUPPORT)
sony_battery_remove(sc);
+ if (sc->worker_initialized)
+ cancel_work_sync(&sc->state_worker);
hid_hw_stop(hdev);
return ret;
}
@@ -1610,7 +1609,8 @@ static void sony_remove(struct hid_device *hdev)
sony_battery_remove(sc);
}
- sony_destroy_ff(hdev);
+ if (sc->worker_initialized)
+ cancel_work_sync(&sc->state_worker);
hid_hw_stop(hdev);
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index.
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
2014-02-20 16:36 ` [PATCH v2 1/5] HID: sony: Fix multi-line comment styling Frank Praznik
2014-02-20 16:36 ` [PATCH v2 2/5] HID: sony: Fix work queue issues Frank Praznik
@ 2014-02-20 16:36 ` Frank Praznik
2014-02-24 10:23 ` David Herrmann
2014-02-20 16:36 ` [PATCH v2 4/5] HID: sony: Prevent duplicate controller connections Frank Praznik
` (3 subsequent siblings)
6 siblings, 1 reply; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:36 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
Make sure that an out-of-bounds read doesn't occur in the Sixaxis battery level
lookup table in the event that the controller sends an invalid battery status
value in the report.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a51a9c0..b39e3ab 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -854,7 +854,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
battery_capacity = 100;
battery_charging = !(rd[30] & 0x01);
} else {
- battery_capacity = sixaxis_battery_capacity[rd[30]];
+ __u8 index = rd[30] <= 5 ? rd[30] : 5;
+ battery_capacity = sixaxis_battery_capacity[index];
battery_charging = 0;
}
cable_state = !((rd[31] >> 4) & 0x01);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/5] HID: sony: Prevent duplicate controller connections.
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
` (2 preceding siblings ...)
2014-02-20 16:36 ` [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index Frank Praznik
@ 2014-02-20 16:36 ` Frank Praznik
2014-02-20 16:36 ` [PATCH v2 5/5] HID: hidp: Add a comment that some devices depend on the current behavior of uniq Frank Praznik
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:36 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
If a Sixaxis or Dualshock 4 controller is connected via USB while already
connected via Bluetooth it will cause duplicate devices to be added to the
input device list.
To prevent this a global list of controllers and their MAC addresses is
maintained and new controllers are checked against this list. If a duplicate
is found, the probe function will exit with -EEXIST.
On USB the MAC is retrieved via a feature report. On Bluetooth neither
controller reports the MAC address in a feature report so the MAC is parsed from
the uniq string. As uniq cannot be guaranteed to be a MAC address in every case
(uHID or the behavior of HIDP changing) a parsing failure will not prevent the
connection.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v2 addresses the issues mentioned in the v1 code review.
To preempt a potential question, byte-swapping the Sixaxis MAC is correct
behavior since addresses in the Bluetooth protocol are natively little-endian.
drivers/hid/hid-sony.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index b39e3ab..5dfa45c 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,6 +33,7 @@
#include <linux/leds.h>
#include <linux/power_supply.h>
#include <linux/spinlock.h>
+#include <linux/list.h>
#include <linux/input/mt.h>
#include "hid-ids.h"
@@ -717,8 +718,12 @@ static enum power_supply_property sony_battery_props[] = {
POWER_SUPPLY_PROP_STATUS,
};
+static spinlock_t sony_dev_list_lock;
+static LIST_HEAD(sony_device_list);
+
struct sony_sc {
spinlock_t lock;
+ struct list_head list_node;
struct hid_device *hdev;
struct led_classdev *leds[MAX_LEDS];
unsigned long quirks;
@@ -730,6 +735,7 @@ struct sony_sc {
__u8 right;
#endif
+ __u8 mac_address[6];
__u8 worker_initialized;
__u8 cable_state;
__u8 battery_charging;
@@ -1489,6 +1495,133 @@ static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
return 0;
}
+/*
+ * If a controller is plugged in via USB while already connected via Bluetooth
+ * it will show up as two devices. A global list of connected controllers and
+ * their MAC addresses is maintained to ensure that a device is only connected
+ * once.
+ */
+static int sony_check_add_dev_list(struct sony_sc *sc)
+{
+ struct sony_sc *entry;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&sony_dev_list_lock, flags);
+
+ list_for_each_entry(entry, &sony_device_list, list_node) {
+ ret = memcmp(sc->mac_address, entry->mac_address,
+ sizeof(sc->mac_address));
+ if (!ret) {
+ ret = -EEXIST;
+ hid_info(sc->hdev, "controller with MAC address %pMR already connected\n",
+ sc->mac_address);
+ goto unlock;
+ }
+ }
+
+ ret = 0;
+ list_add(&(sc->list_node), &sony_device_list);
+
+unlock:
+ spin_unlock_irqrestore(&sony_dev_list_lock, flags);
+ return ret;
+}
+
+static void sony_remove_dev_list(struct sony_sc *sc)
+{
+ unsigned long flags;
+
+ if (sc->list_node.next) {
+ spin_lock_irqsave(&sony_dev_list_lock, flags);
+ list_del(&(sc->list_node));
+ spin_unlock_irqrestore(&sony_dev_list_lock, flags);
+ }
+}
+
+static int sony_get_bt_devaddr(struct sony_sc *sc)
+{
+ int ret;
+
+ /* HIDP stores the device MAC address as a string in the uniq field. */
+ ret = strlen(sc->hdev->uniq);
+ if (ret != 17)
+ return -EINVAL;
+
+ ret = sscanf(sc->hdev->uniq,
+ "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+ &sc->mac_address[5], &sc->mac_address[4], &sc->mac_address[3],
+ &sc->mac_address[2], &sc->mac_address[1], &sc->mac_address[0]);
+
+ if (ret != 6)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int sony_check_add(struct sony_sc *sc)
+{
+ int n, ret;
+
+ if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
+ (sc->quirks & SIXAXIS_CONTROLLER_BT)) {
+ /*
+ * sony_get_bt_devaddr() attempts to parse the Bluetooth MAC
+ * address from the uniq string where HIDP stores it.
+ * As uniq cannot be guaranteed to be a MAC address in all cases
+ * a failure of this function should not prevent the connection.
+ */
+ if (sony_get_bt_devaddr(sc) < 0) {
+ hid_warn(sc->hdev, "UNIQ does not contain a MAC address; duplicate check skipped\n");
+ return 0;
+ }
+ } else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ __u8 buf[7];
+
+ /*
+ * The MAC address of a DS4 controller connected via USB can be
+ * retrieved with feature report 0x81. The address begins at
+ * offset 1.
+ */
+ ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+ if (ret != 7) {
+ hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
+ return ret < 0 ? ret : -EINVAL;
+ }
+
+ memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
+ } else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+ __u8 buf[18];
+
+ /*
+ * The MAC address of a Sixaxis controller connected via USB can
+ * be retrieved with feature report 0xf2. The address begins at
+ * offset 4.
+ */
+ ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+ if (ret != 18) {
+ hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
+ return ret < 0 ? ret : -EINVAL;
+ }
+
+ /*
+ * The Sixaxis device MAC in the report is big-endian and must
+ * be byte-swapped.
+ */
+ for (n = 0; n < 6; n++)
+ sc->mac_address[5-n] = buf[4+n];
+ } else {
+ return 0;
+ }
+
+ return sony_check_add_dev_list(sc);
+}
+
+
static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
int ret;
@@ -1559,6 +1692,10 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret < 0)
goto err_stop;
+ ret = sony_check_add(sc);
+ if (ret < 0)
+ goto err_stop;
+
if (sc->quirks & SONY_LED_SUPPORT) {
ret = sony_leds_init(hdev);
if (ret < 0)
@@ -1594,6 +1731,7 @@ err_stop:
sony_battery_remove(sc);
if (sc->worker_initialized)
cancel_work_sync(&sc->state_worker);
+ sony_remove_dev_list(sc);
hid_hw_stop(hdev);
return ret;
}
@@ -1613,6 +1751,8 @@ static void sony_remove(struct hid_device *hdev)
if (sc->worker_initialized)
cancel_work_sync(&sc->state_worker);
+ sony_remove_dev_list(sc);
+
hid_hw_stop(hdev);
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 5/5] HID: hidp: Add a comment that some devices depend on the current behavior of uniq
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
` (3 preceding siblings ...)
2014-02-20 16:36 ` [PATCH v2 4/5] HID: sony: Prevent duplicate controller connections Frank Praznik
@ 2014-02-20 16:36 ` Frank Praznik
2014-02-24 10:25 ` [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver David Herrmann
2014-02-24 22:31 ` Jiri Kosina
6 siblings, 0 replies; 9+ messages in thread
From: Frank Praznik @ 2014-02-20 16:36 UTC (permalink / raw)
To: linux-input; +Cc: dh.herrmann, jkosina, Frank Praznik
Add a comment noting that some devices depend on the destination address being
stored in uniq.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
net/bluetooth/hidp/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 77c4bad..98e4840 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -767,6 +767,9 @@ static int hidp_setup_hid(struct hidp_session *session,
snprintf(hid->phys, sizeof(hid->phys), "%pMR",
&l2cap_pi(session->ctrl_sock->sk)->chan->src);
+ /* NOTE: Some device modules depend on the dst address being stored in
+ * uniq. Please be aware of this before making changes to this behavior.
+ */
snprintf(hid->uniq, sizeof(hid->uniq), "%pMR",
&l2cap_pi(session->ctrl_sock->sk)->chan->dst);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index.
2014-02-20 16:36 ` [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index Frank Praznik
@ 2014-02-24 10:23 ` David Herrmann
0 siblings, 0 replies; 9+ messages in thread
From: David Herrmann @ 2014-02-24 10:23 UTC (permalink / raw)
To: Frank Praznik; +Cc: open list:HID CORE LAYER, Jiri Kosina
Hi
On Thu, Feb 20, 2014 at 5:36 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Make sure that an out-of-bounds read doesn't occur in the Sixaxis battery level
> lookup table in the event that the controller sends an invalid battery status
> value in the report.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> drivers/hid/hid-sony.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index a51a9c0..b39e3ab 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -854,7 +854,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
> battery_capacity = 100;
> battery_charging = !(rd[30] & 0x01);
> } else {
> - battery_capacity = sixaxis_battery_capacity[rd[30]];
> + __u8 index = rd[30] <= 5 ? rd[30] : 5;
> + battery_capacity = sixaxis_battery_capacity[index];
Does it make sense to read something else on invalid reports? Sounds
weird to me to just read at a lower index in case it's too short.
Shouldn't you just bail out? But the worst that can happen is wrong
battery values reported to user-space, so I'm fine with it.
Thanks
David
> battery_charging = 0;
> }
> cable_state = !((rd[31] >> 4) & 0x01);
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
` (4 preceding siblings ...)
2014-02-20 16:36 ` [PATCH v2 5/5] HID: hidp: Add a comment that some devices depend on the current behavior of uniq Frank Praznik
@ 2014-02-24 10:25 ` David Herrmann
2014-02-24 22:31 ` Jiri Kosina
6 siblings, 0 replies; 9+ messages in thread
From: David Herrmann @ 2014-02-24 10:25 UTC (permalink / raw)
To: Frank Praznik; +Cc: open list:HID CORE LAYER, Jiri Kosina
Hi
On Thu, Feb 20, 2014 at 5:35 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> v2 of this patch set addresses the code review issues raised in v1 as well as
> adding a patch that fixes the styling of multi-line comments to conform to
> kernel coding standards, adds a check to protect against a potential
> out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
> that a device module now depends on the current behavior of the uniq string.
Besides some minor comments, the series looks good to me:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
` (5 preceding siblings ...)
2014-02-24 10:25 ` [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver David Herrmann
@ 2014-02-24 22:31 ` Jiri Kosina
6 siblings, 0 replies; 9+ messages in thread
From: Jiri Kosina @ 2014-02-24 22:31 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, dh.herrmann
On Thu, 20 Feb 2014, Frank Praznik wrote:
> v2 of this patch set addresses the code review issues raised in v1 as well as
> adding a patch that fixes the styling of multi-line comments to conform to
> kernel coding standards, adds a check to protect against a potential
> out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
> that a device module now depends on the current behavior of the uniq string.
Queued in for-3.15/sony, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-02-24 22:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-20 16:35 [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver Frank Praznik
2014-02-20 16:36 ` [PATCH v2 1/5] HID: sony: Fix multi-line comment styling Frank Praznik
2014-02-20 16:36 ` [PATCH v2 2/5] HID: sony: Fix work queue issues Frank Praznik
2014-02-20 16:36 ` [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index Frank Praznik
2014-02-24 10:23 ` David Herrmann
2014-02-20 16:36 ` [PATCH v2 4/5] HID: sony: Prevent duplicate controller connections Frank Praznik
2014-02-20 16:36 ` [PATCH v2 5/5] HID: hidp: Add a comment that some devices depend on the current behavior of uniq Frank Praznik
2014-02-24 10:25 ` [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver David Herrmann
2014-02-24 22:31 ` Jiri Kosina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.