Hi Roderick,
Hi Daniel,
Thank you for all the suggestions, Roderick.
I fixed the typo in your patch, backported to hid-sony and tested both of them.
They fix the issue and the DS4 can be used even without calibration data.
I cannot test if everything works well with a DualSense because I do
not own one.
Hi Daniel, I've seen that in the hid-nintendo driver, the calibration
denominator is not sanitized if it's zero.
If I'm not mistaken, a device that disguises itself as a joycon with
denominator zero can crash the kernel
when this is used in the mult_frac(). I had this behaviour with the
hid-sony driver.
You can find attached the patch that should solve the problem.
Thanks,
Alain
From 5f7aa1612a10123b9630701d29e39f8e55a74b5e Mon Sep 17 00:00:00 2001
From: Alain Carlucci <carpikemail@xxxxxxxxx>
Date: Fri, 30 Dec 2022 18:53:53 +0100
Subject: [PATCH 3/3] HID: nintendo: sanity check for denominators
This patch prevents a crash if the device reports denominator equal to
zero. In that case, it is substituted with one, so that the driver does
not do a division by zero.
---
drivers/hid/hid-nintendo.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 5bfc0c450..f050754e2 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -866,6 +866,19 @@ static void joycon_calc_imu_cal_divisors(struct joycon_ctlr *ctlr)
ctlr->accel_cal.offset[i];
ctlr->imu_cal_gyro_divisor[i] = ctlr->gyro_cal.scale[i] -
ctlr->gyro_cal.offset[i];
+
+
+ /* Sanity check accelerometer calibration data. */
+ if (ctlr->imu_cal_accel_divisor[i] == 0) {
+ hid_warn(ctlr->hdev, "Invalid divisor for accelerometer %d.", i);
+ ctlr->imu_cal_accel_divisor[i] = 1;
+ }
+
+ /* Sanity check gyroscope calibration data. */
+ if (ctlr->imu_cal_gyro_divisor[i] == 0) {
+ hid_warn(ctlr->hdev, "Invalid divisor for gyroscope %d.", i);
+ ctlr->imu_cal_gyro_divisor[i] = 1;
+ }
}
}
--
2.39.0
From d751911f23aca413b2bf831ea301219c533081e6 Mon Sep 17 00:00:00 2001
From: Alain Carlucci <carpikemail@xxxxxxxxx>
Date: Fri, 30 Dec 2022 18:47:54 +0100
Subject: [PATCH 1/3] HID: playstation: sanity check calibration data.
This patch prevents a division by zero if the the device reports invalid
calibration parameters. In that case, fallback to default values.
---
drivers/hid/hid-playstation.c | 61 +++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f399bf0d3..a249f3bb0 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -954,6 +954,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
int speed_2x;
int range_2g;
int ret = 0;
+ int i;
uint8_t *buf;
buf = kzalloc(DS_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
@@ -1005,6 +1006,23 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
ds->gyro_calib_data[2].sens_numer = speed_2x*DS_GYRO_RES_PER_DEG_S;
ds->gyro_calib_data[2].sens_denom = gyro_roll_plus - gyro_roll_minus;
+ /* Sanity check gyro calibration data. This is needed for clone devices,
+ * which poorly implement support. Some devices appear to return zeros
+ * for all data, while some hardcode all the same values and for some
+ * use the wrong sign (the delta is then 0).
+ * When we detect something fishy, just disable the calibration logic
+ * altogether as nothing can be trusted for that axis.
+ */
+ for (i = 0; i < ARRAY_SIZE(ds->gyro_calib_data); i++) {
+ if (ds->gyro_calib_data[i].sens_denom == 0) {
+ hid_warn(ds->base.hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
+ ds->gyro_calib_data[i].abs_code);
+ ds->gyro_calib_data[i].bias = 0;
+ ds->gyro_calib_data[i].sens_numer = DS_GYRO_RES_PER_DEG_S;
+ ds->gyro_calib_data[i].sens_denom = 1;
+ }
+ }
+
/*
* Set accelerometer calibration and normalization parameters.
* Data values will be normalized to 1/DS_ACC_RES_PER_G g.
@@ -1027,6 +1045,18 @@ static int dualsense_get_calibration_data(struct dualsense *ds)
ds->accel_calib_data[2].sens_numer = 2*DS_ACC_RES_PER_G;
ds->accel_calib_data[2].sens_denom = range_2g;
+ /* Sanity check accelerometer calibration data. This is needed for clone devices,
+ * which poorly implement support.
+ */
+ for (i = 0; i < ARRAY_SIZE(ds->accel_calib_data); i++) {
+ if (ds->accel_calib_data[i].sens_denom == 0) {
+ hid_warn(ds->base.hdev, "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
+ ds->accel_calib_data[i].abs_code);
+ ds->accel_calib_data[i].bias = 0;
+ ds->accel_calib_data[i].sens_numer = DS_ACC_RES_PER_G;
+ ds->accel_calib_data[i].sens_denom = 1;
+ }
+ }
err_free:
kfree(buf);
return ret;
@@ -1737,6 +1767,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
int speed_2x;
int range_2g;
int ret = 0;
+ int i;
uint8_t *buf;
if (ds4->base.hdev->bus == BUS_USB) {
@@ -1830,6 +1861,23 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S;
ds4->gyro_calib_data[2].sens_denom = gyro_roll_plus - gyro_roll_minus;
+ /* Sanity check gyro calibration data. This is needed for clone devices,
+ * which poorly implement support. Some devices appear to return zeros
+ * for all data, while some hardcode all the same values and for some
+ * use the wrong sign (the delta is then 0).
+ * When we detect something fishy, just disable the calibration logic
+ * altogether as nothing can be trusted for that axis.
+ */
+ for (i = 0; i < ARRAY_SIZE(ds4->gyro_calib_data); i++) {
+ if (ds4->gyro_calib_data[i].sens_denom == 0) {
+ hid_warn(hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
+ ds4->gyro_calib_data[i].abs_code);
+ ds4->gyro_calib_data[i].bias = 0;
+ ds4->gyro_calib_data[i].sens_numer = DS4_GYRO_RES_PER_DEG_S;
+ ds4->gyro_calib_data[i].sens_denom = 1;
+ }
+ }
+
/*
* Set accelerometer calibration and normalization parameters.
* Data values will be normalized to 1/DS4_ACC_RES_PER_G g.
@@ -1852,6 +1900,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
ds4->accel_calib_data[2].sens_denom = range_2g;
+ /* Sanity check accelerometer calibration data. This is needed for clone devices,
+ * which poorly implement support.
+ */
+ for (i = 0; i < ARRAY_SIZE(ds4->accel_calib_data); i++) {
+ if (ds4->accel_calib_data[i].sens_denom == 0) {
+ hid_warn(hdev, "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
+ ds4->accel_calib_data[i].abs_code);
+ ds4->accel_calib_data[i].bias = 0;
+ ds4->accel_calib_data[i].sens_numer = DS4_ACC_RES_PER_G;
+ ds4->accel_calib_data[i].sens_denom = 1;
+ }
+ }
+
err_free:
kfree(buf);
return ret;
--
2.39.0
From dc92f0be1352ba5a27239f451e5f0161e48f14e6 Mon Sep 17 00:00:00 2001
From: Alain Carlucci <carpikemail@xxxxxxxxx>
Date: Fri, 30 Dec 2022 18:52:06 +0100
Subject: [PATCH 2/3] HID: sony: sanity check DS4 calibration data
This patch prevents a crash when the DS4 reports invalid calibration
data for the IMUs. It checks if the denominator is zero, and in that
case fallback to a default value.
---
drivers/hid/hid-sony.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 13125997a..d25e84e35 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1714,6 +1714,7 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc)
short acc_z_plus, acc_z_minus;
int speed_2x;
int range_2g;
+ int i;
/* For Bluetooth we use a different request, which supports CRC.
* Note: in Bluetooth mode feature report 0x02 also changes the state
@@ -1858,6 +1859,30 @@ static int dualshock4_get_calibration_data(struct sony_sc *sc)
sc->ds4_calib_data[5].sens_numer = 2*DS4_ACC_RES_PER_G;
sc->ds4_calib_data[5].sens_denom = range_2g;
+ /* Sanity check gyro calibration data. This is needed for clone devices,
+ * which poorly implement support. Some devices appear to return zeros
+ * for all data, while some hardcode all the same values and for some
+ * use the wrong sign (the delta is then 0).
+ * When we detect something fishy, just disable the calibration logic
+ * altogether as nothing can be trusted for that axis.
+ */
+ for (i = 0; i < 6; i++) {
+ if (sc->ds4_calib_data[i].sens_denom == 0) {
+ hid_warn(sc->hdev, "Invalid calibration data for axis (%d), disabling calibration.",
+ sc->ds4_calib_data[i].abs_code);
+
+ if (i < 3) {
+ /* Gyroscope */
+ sc->ds4_calib_data[i].sens_numer = DS4_GYRO_RES_PER_DEG_S;
+ } else {
+ /* Accelerometer */
+ sc->ds4_calib_data[i].sens_numer = DS4_ACC_RES_PER_G;
+ }
+ sc->ds4_calib_data[i].bias = 0;
+ sc->ds4_calib_data[i].sens_denom = 1;
+ }
+ }
+
err_stop:
kfree(buf);
return ret;
--
2.39.0
