* Webmail Account Warning.
From: Administrator:: Account Warning. @ 2013-09-10 10:54 UTC (permalink / raw)
Administrator Webmail Account Warning.
This mail is from All Administrator Webmail Account; we wish to bring to
your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of
500 MB quota and your email IP is causing conflict because it is been
accessed in different server location. You need to Upgrade and expand your
email quota limit before you can continue to use your email.
Update your email quota limit to 2.6 GB, use the below web link:
http://accountwarning.yolasite.com/contact-us.php
Failure to do this will result to email deactivation within 24 hours
Thank you for your understanding.
Copyright 2013 Help Desk
Administrator Webmail Support.
^ permalink raw reply
* Re: [PATCH 4/7] HID: steelseries: validate output report details
From: Benjamin Tissoires @ 2013-09-10 11:45 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Henrik Rydberg
In-Reply-To: <CAN+gG=H6Pib-kzWKPH_Hj_vcw7q_agHZ7_Qi-pEde9tbTB4EkA@mail.gmail.com>
On Mon, Sep 9, 2013 at 3:02 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Wed, Sep 4, 2013 at 6:37 PM, Kees Cook <keescook@chromium.org> wrote:
>> A HID device could send a malicious output report that would cause the
>> steelseries HID driver to write beyond the output report allocation
>> during initialization, causing a heap overflow:
>>
>> [ 167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410
>> ...
>> [ 182.050547] BUG kmalloc-256 (Tainted: G W ): Redzone overwritten
>>
>> CVE-2013-2891
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Cc: stable@kernel.org
>> ---
>> drivers/hid/hid-steelseries.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
>> index d164911..29f328f 100644
>> --- a/drivers/hid/hid-steelseries.c
>> +++ b/drivers/hid/hid-steelseries.c
>> @@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
>> goto err_free;
>> }
>>
>> + if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 16)) {
>> + ret = -ENODEV;
>> + goto err_free;
>
> There is a problem here in case of a failure:
> hid_parse() allocates a lot of memory and sets the flag
> HID_STAT_PARSED, but we are leaving the probe() without clearing all
> this stuff.
> In hid_parse(), when a failure is detected, we call
> hid_close_report(), so we also should close the report in the same way
> here (but hid_close_report() is a static function).
>
Sorry, my bad. hid_close_report() is called upon ->probe() failure. So
this is perfectly good.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* 答复: [PATCH 2/2] Input: ALPS Touchpad- Improve the performance of alps v5-protocol's touchpad
From: yunkang.tang @ 2013-09-10 11:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input@vger.kernel.org
In-Reply-To: <20130909162734.GC25107@core.coreip.homeip.net>
Hi Dmitry,
>Please do not separate changes in header files form changes in .c files
>because patches need to be complete. If I were to apply this patch alone
>it would break the compilation.
Thanks for your expanation. I got that.
Thanks
Best Regards
------------------------------------------------------
Tommy
On Thu, Sep 05, 2013 at 05:50:55AM +0000, yunkang.tang@cn.alps.com wrote:
> <Change list>
> - Add the macro definition for v5 device.
>
> --- linux-3.11/drivers/input/mouse/alps.h.orig 2013-09-04 19:51:33.837135870 +0800
> +++ linux-3.11/drivers/input/mouse/alps.h 2013-09-05 21:01:57.074314890 +0800
> @@ -18,6 +18,10 @@
> #define ALPS_PROTO_V4 4
> #define ALPS_PROTO_V5 5
>
> +#define DOLPHIN_COUNT_PER_ELECTRODE 64
> +#define DOLPHIN_PROFILE_XOFFSET 8 /* [DOLPHIN] The number of the x-electrode offset value */
> +#define DOLPHIN_PROFILE_YOFFSET 1 /* [DOLPHIN] The number of the y-electrode offset value */
> +
> /**
> * struct alps_model_info - touchpad ID table
> * @signature: E7 response string to match.
> @@ -69,7 +73,7 @@ struct alps_nibble_commands {
> * @y: Y position for ST.
> * @z: Z position for ST.
> * @first_mp: Packet is the first of a multi-packet report.
> - * @is_mp: Packet is part of a multi-packet report.
> + * @is_mp: Packet is the last of a multi-packet report.
> * @left: Left touchpad button is active.
> * @right: Right touchpad button is active.
> * @middle: Middle touchpad button is active.
> @@ -145,7 +149,7 @@ struct alps_data {
>
> int (*hw_init)(struct psmouse *psmouse);
> void (*process_packet)(struct psmouse *psmouse);
> - void (*decode_fields)(struct alps_fields *f, unsigned char *p);
> + void (*decode_fields)(struct alps_fields *f, unsigned char *p, struct psmouse *psmouse);
> void (*set_abs_params)(struct alps_data *priv, struct input_dev *dev1);
>
> int prev_fin;
Please do not separate changes in header files form changes in .c files
because patches need to be complete. If I were to apply this patch alone
it would break the compilation.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Atmel updates to atmel_mxt_ts touch controller driver - v6
From: Nick Dyer @ 2013-09-10 13:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: rydberg, Daniel Kurtz, Joonyoung Shim, Alan Bowens, linux-input,
linux-kernel, Peter Meerwald, Benson Leung, Olof Johansson
In-Reply-To: <20130815160734.GB12196@core.coreip.homeip.net>
Dmitry Torokhov wrote:
> On Thu, Aug 15, 2013 at 04:55:57PM +0100, Nick Dyer wrote:
>> rydberg@euromail.se wrote:
>>> First: thanks for the patches and you work on this driver.
>>
>> Thank you for your time in looking at these changes.
>>
>>> Now, I don't swear much, but I would like to emphasize line 161 of
>>> Documentation/SubmittingPatches:
>>>
>>> **Do not send more than 15 patches at once to the vger mailing lists!!!***
>>>
>>> One reason that should be obvious by now is that your work will be
>>> attended to much quicker. One may think that it is more efficient to
>>> send the whole backlog at once, but in fact, the time it takes to get
>>> a patchset accepted is inversely proportional to the length of the
>>> patchset. So please, keep it small and simple next time,
>>
>> Apologies. I will split this lot up into several smaller series of patches.
>
> Wait, I am in the process of applying it actually...
I wonder, have you made any progress with this? Is there any way I can assist?
Thanks
--
Nick Dyer
^ permalink raw reply
* [PATCH 3.12-rc1] USB: input: cm109.c: Convert high volume dev_err() to dev_err_ratelimited()
From: Tim Gardner @ 2013-09-10 16:23 UTC (permalink / raw)
To: linux-input, linux-kernel; +Cc: Tim Gardner, Dmitry Torokhov
BugLink: http://bugs.launchpad.net/bugs/1222850
This input device can get into a state that produces a high
volume of device status errors. Attempt to throttle these
error messages such that the kernel log is not flooded.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/input/misc/cm109.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 082684e..8ec72bc 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -351,7 +351,8 @@ static void cm109_urb_irq_callback(struct urb *urb)
if (status) {
if (status == -ESHUTDOWN)
return;
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
}
/* Special keys */
@@ -419,7 +420,8 @@ static void cm109_urb_ctl_callback(struct urb *urb)
dev->ctl_data->byte[3]);
if (status)
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
spin_lock(&dev->ctl_submit_lock);
@@ -436,7 +438,7 @@ static void cm109_urb_ctl_callback(struct urb *urb)
dev->irq_urb_pending = 1;
error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
if (error)
- dev_err(&dev->intf->dev,
+ dev_err_ratelimited(&dev->intf->dev,
"%s: usb_submit_urb (urb_irq) failed %d\n",
__func__, error);
}
@@ -480,7 +482,7 @@ static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on)
dev->ctl_data,
USB_PKT_LEN, USB_CTRL_SET_TIMEOUT);
if (error < 0 && error != -EINTR)
- dev_err(&dev->intf->dev, "%s: usb_control_msg() failed %d\n",
+ dev_err_ratelimited(&dev->intf->dev, "%s: usb_control_msg() failed %d\n",
__func__, error);
}
@@ -542,8 +544,9 @@ static int cm109_input_open(struct input_dev *idev)
error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
if (error)
- dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
- __func__, error);
+ dev_err_ratelimited(&dev->intf->dev,
+ "%s: usb_submit_urb (urb_ctl) failed %d\n", __func__,
+ error);
else
dev->open = 1;
@@ -716,7 +719,7 @@ static int cm109_usb_probe(struct usb_interface *intf,
pipe = usb_rcvintpipe(udev, endpoint->bEndpointAddress);
ret = usb_maxpacket(udev, pipe, usb_pipeout(pipe));
if (ret != USB_PKT_LEN)
- dev_err(&intf->dev, "invalid payload size %d, expected %d\n",
+ dev_err_ratelimited(&intf->dev, "invalid payload size %d, expected %d\n",
ret, USB_PKT_LEN);
/* initialise irq urb */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/3] HID: Delay opening HID device
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: jc23-KWPb1pKIrIJaa/9Udqfwiw, jkosina-AlSwsSmVLrQ,
holler-SXC+2es9fhnfWeYVQQPykw, Srinivas Pandruvada
Don't call hid_open_device till there is actually an user. This saves
power by not opening underlying transport for HID. Also close device
if there are no active mfd client using HID sensor hub.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/hid/hid-sensor-hub.c | 45 ++++++++++++++++++++++++++++++++----------
include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
2 files changed, 53 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 10e1581..8effdee 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
return 1;
}
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
+{
+ int ret;
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+
+ mutex_lock(&data->mutex);
+ if (!hsdev->ref_cnt) {
+ ret = hid_hw_open(hsdev->hdev);
+ if (ret) {
+ hid_err(hsdev->hdev, "failed to open hid device\n");
+ mutex_unlock(&data->mutex);
+ return ret;
+ }
+ }
+ hsdev->ref_cnt++;
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_open);
+
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
+{
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+
+ mutex_lock(&data->mutex);
+ hsdev->ref_cnt--;
+ if (!hsdev->ref_cnt)
+ hid_hw_close(hsdev->hdev);
+ mutex_unlock(&data->mutex);
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_close);
+
static int sensor_hub_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
hid_err(hdev, "hw start failed\n");
return ret;
}
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "failed to open input interrupt pipe\n");
- goto err_stop_hw;
- }
-
INIT_LIST_HEAD(&sd->dyn_callback_list);
sd->hid_sensor_client_cnt = 0;
report_enum = &hdev->report_enum[HID_INPUT_REPORT];
@@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (dev_cnt > HID_MAX_PHY_DEVICES) {
hid_err(hdev, "Invalid Physical device count\n");
ret = -EINVAL;
- goto err_close;
+ goto err_stop_hw;
}
sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
sizeof(struct mfd_cell),
@@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (sd->hid_sensor_hub_client_devs == NULL) {
hid_err(hdev, "Failed to allocate memory for mfd cells\n");
ret = -ENOMEM;
- goto err_close;
+ goto err_stop_hw;
}
list_for_each_entry(report, &report_enum->report_list, list) {
hid_dbg(hdev, "Report id:%x\n", report->id);
@@ -565,8 +592,6 @@ err_free_names:
for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
kfree(sd->hid_sensor_hub_client_devs[i].name);
kfree(sd->hid_sensor_hub_client_devs);
-err_close:
- hid_hw_close(hdev);
err_stop_hw:
hid_hw_stop(hdev);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 32ba451..a265af2 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
* @hdev: Stores the hid instance.
* @vendor_id: Vendor id of hub device.
* @product_id: Product id of hub device.
+ * @ref_cnt: Number of MFD clients have opened this device
*/
struct hid_sensor_hub_device {
struct hid_device *hdev;
u32 vendor_id;
u32 product_id;
+ int ref_cnt;
};
/**
@@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
void *priv);
};
+/**
+* sensor_hub_device_open() - Open hub device
+* @hsdev: Hub device instance.
+*
+* Used to open hid device for sensor hub.
+*/
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
+
+/**
+* sensor_hub_device_clode() - Close hub device
+* @hsdev: Hub device instance.
+*
+* Used to clode hid device for sensor hub.
+*/
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
+
/* Registration functions */
/**
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] IIO: call sensor hub open close function
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input, linux-iio; +Cc: jc23, jkosina, holler, Srinivas Pandruvada
In-Reply-To: <1378843432-5113-1-git-send-email-srinivas.pandruvada@linux.intel.com>
Call hid_sensor_hub_device_open when user space opens device and call
hid_sensor_hub_device_close when device is closed. This helps in
saving power.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 87419c4..b6e77e0 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -34,6 +34,12 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
int state_val;
+ if (state) {
+ if (sensor_hub_device_open(st->hsdev))
+ return -EIO;
+ } else
+ sensor_hub_device_close(st->hsdev);
+
state_val = state ? 1 : 0;
if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
++state_val;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3] HID RTC: Open sensor hub open close
From: Srinivas Pandruvada @ 2013-09-10 20:03 UTC (permalink / raw)
To: linux-input, linux-iio; +Cc: jc23, jkosina, holler, Srinivas Pandruvada
In-Reply-To: <1378843432-5113-1-git-send-email-srinivas.pandruvada@linux.intel.com>
Open sensor hub when module is loaded and close when module is removed.
This helps saving power by opening HID transport only when there is an
user.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/rtc/rtc-hid-sensor-time.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
index 7273b01..1fe170c 100644
--- a/drivers/rtc/rtc-hid-sensor-time.c
+++ b/drivers/rtc/rtc-hid-sensor-time.c
@@ -279,15 +279,28 @@ static int hid_time_probe(struct platform_device *pdev)
return ret;
}
+ ret = sensor_hub_device_open(hsdev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to open sensor hub device!\n");
+ goto err_open;
+ }
+
time_state->rtc = devm_rtc_device_register(&pdev->dev,
"hid-sensor-time", &hid_time_rtc_ops,
THIS_MODULE);
if (IS_ERR(time_state->rtc)) {
dev_err(&pdev->dev, "rtc device register failed!\n");
- return PTR_ERR(time_state->rtc);
+ ret = PTR_ERR(time_state->rtc);
+ goto err_rtc;
}
+ return 0;
+
+err_rtc:
+ sensor_hub_device_close(hsdev);
+err_open:
+ sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
return ret;
}
@@ -295,6 +308,7 @@ static int hid_time_remove(struct platform_device *pdev)
{
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+ sensor_hub_device_close(hsdev);
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2] Input: wacom - add support for three new Intuos Pro devices
From: Ping Cheng @ 2013-09-10 21:57 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, chris, Ping Cheng
Acked-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
v2: Updated with Chris' comment (missed model specific bits in v1).
---
drivers/input/tablet/wacom_sys.c | 10 +++++--
drivers/input/tablet/wacom_wac.c | 58 ++++++++++++++++++++++++++++++----------
drivers/input/tablet/wacom_wac.h | 3 +++
3 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index 79b69ea..cec3f9a 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -719,7 +719,7 @@ static int wacom_led_control(struct wacom *wacom)
return -ENOMEM;
if (wacom->wacom_wac.features.type >= INTUOS5S &&
- wacom->wacom_wac.features.type <= INTUOS5L) {
+ wacom->wacom_wac.features.type <= INTUOSPL) {
/*
* Touch Ring and crop mark LED luminance may take on
* one of four values:
@@ -981,6 +981,9 @@ static int wacom_initialize_leds(struct wacom *wacom)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
wacom->led.select[0] = 0;
wacom->led.select[1] = 0;
wacom->led.llv = 32;
@@ -1024,6 +1027,9 @@ static void wacom_destroy_leds(struct wacom *wacom)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
sysfs_remove_group(&wacom->intf->dev.kobj,
&intuos5_led_attr_group);
break;
@@ -1298,7 +1304,7 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
* HID descriptor. If this is the touch interface (wMaxPacketSize
* of WACOM_PKGLEN_BBTOUCH3), override the table values.
*/
- if (features->type >= INTUOS5S && features->type <= INTUOS5L) {
+ if (features->type >= INTUOS5S && features->type <= INTUOSPL) {
if (endpoint->wMaxPacketSize == WACOM_PKGLEN_BBTOUCH3) {
features->device_type = BTN_TOOL_FINGER;
features->pktlen = WACOM_PKGLEN_BBTOUCH3;
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 541197b..dc96274 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -619,14 +619,14 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
} else {
input_report_abs(input, ABS_MISC, 0);
}
- } else if (features->type >= INTUOS5S && features->type <= INTUOS5L) {
+ } else if (features->type >= INTUOS5S && features->type <= INTUOSPL) {
int i;
/* Touch ring mode switch has no capacitive sensor */
input_report_key(input, BTN_0, (data[3] & 0x01));
/*
- * ExpressKeys on Intuos5 have a capacitive sensor in
+ * ExpressKeys on Intuos5/Intuos Pro have a capacitive sensor in
* addition to the mechanical switch. Switch data is
* stored in data[4], capacitive data in data[5].
*/
@@ -714,7 +714,9 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
features->type == INTUOS4 ||
features->type == INTUOS4S ||
features->type == INTUOS5 ||
- features->type == INTUOS5S)) {
+ features->type == INTUOS5S ||
+ features->type == INTUOSPM ||
+ features->type == INTUOSPS)) {
return 0;
}
@@ -767,8 +769,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
} else if (wacom->tool[idx] == BTN_TOOL_MOUSE) {
/* I4 mouse */
- if ((features->type >= INTUOS4S && features->type <= INTUOS4L) ||
- (features->type >= INTUOS5S && features->type <= INTUOS5L)) {
+ if (features->type >= INTUOS4S && features->type <= INTUOSPL) {
input_report_key(input, BTN_LEFT, data[6] & 0x01);
input_report_key(input, BTN_MIDDLE, data[6] & 0x02);
input_report_key(input, BTN_RIGHT, data[6] & 0x04);
@@ -795,7 +796,8 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
}
}
} else if ((features->type < INTUOS3S || features->type == INTUOS3L ||
- features->type == INTUOS4L || features->type == INTUOS5L) &&
+ features->type == INTUOS4L || features->type == INTUOS5L ||
+ features->type == INTUOSPL) &&
wacom->tool[idx] == BTN_TOOL_LENS) {
/* Lens cursor packets */
input_report_key(input, BTN_LEFT, data[8] & 0x01);
@@ -1105,6 +1107,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
{
+ struct wacom_features *features = &wacom->features;
struct input_dev *input = wacom->input;
bool touch = data[1] & 0x80;
int slot = input_mt_get_slot_by_key(input, data[0]);
@@ -1120,14 +1123,20 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
if (touch) {
int x = (data[2] << 4) | (data[4] >> 4);
int y = (data[3] << 4) | (data[4] & 0x0f);
- int a = data[5];
+ int width, height;
- // "a" is a scaled-down area which we assume is roughly
- // circular and which can be described as: a=(pi*r^2)/C.
- int x_res = input_abs_get_res(input, ABS_X);
- int y_res = input_abs_get_res(input, ABS_Y);
- int width = 2 * int_sqrt(a * WACOM_CONTACT_AREA_SCALE);
- int height = width * y_res / x_res;
+ if (features->type >= INTUOSPS && features->type <= INTUOSPL) {
+ width = data[5];
+ height = data[6];
+ } else {
+ int a = data[5];
+ // "a" is a scaled-down area which we assume is roughly
+ // circular and which can be described as: a=(pi*r^2)/C.
+ int x_res = input_abs_get_res(input, ABS_X);
+ int y_res = input_abs_get_res(input, ABS_Y);
+ width = 2 * int_sqrt(a * WACOM_CONTACT_AREA_SCALE);
+ height = width * y_res / x_res;
+ }
input_report_abs(input, ABS_MT_POSITION_X, x);
input_report_abs(input, ABS_MT_POSITION_Y, y);
@@ -1335,6 +1344,9 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
case INTUOS5S:
case INTUOS5:
case INTUOS5L:
+ case INTUOSPS:
+ case INTUOSPM:
+ case INTUOSPL:
if (len == WACOM_PKGLEN_BBTOUCH3)
sync = wacom_bpt3_touch(wacom_wac);
else
@@ -1418,7 +1430,7 @@ void wacom_setup_device_quirks(struct wacom_features *features)
/* these device have multiple inputs */
if (features->type >= WIRELESS ||
- (features->type >= INTUOS5S && features->type <= INTUOS5L) ||
+ (features->type >= INTUOS5S && features->type <= INTUOSPL) ||
(features->oVid && features->oPid))
features->quirks |= WACOM_QUIRK_MULTI_INPUT;
@@ -1625,6 +1637,8 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case INTUOS5:
case INTUOS5L:
+ case INTUOSPM:
+ case INTUOSPL:
if (features->device_type == BTN_TOOL_PEN) {
__set_bit(BTN_7, input_dev->keybit);
__set_bit(BTN_8, input_dev->keybit);
@@ -1632,6 +1646,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
/* fall through */
case INTUOS5S:
+ case INTUOSPS:
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
if (features->device_type == BTN_TOOL_PEN) {
@@ -1950,6 +1965,18 @@ static const struct wacom_features wacom_features_0x29 =
static const struct wacom_features wacom_features_0x2A =
{ "Wacom Intuos5 M", WACOM_PKGLEN_INTUOS, 44704, 27940, 2047,
63, INTUOS5, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
+static const struct wacom_features wacom_features_0x314 =
+ { "Wacom Intuos Pro S", WACOM_PKGLEN_INTUOS, 31496, 19685, 2047,
+ 63, INTUOSPS, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
+static const struct wacom_features wacom_features_0x315 =
+ { "Wacom Intuos Pro M", WACOM_PKGLEN_INTUOS, 44704, 27940, 2047,
+ 63, INTUOSPM, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
+static const struct wacom_features wacom_features_0x317 =
+ { "Wacom Intuos Pro L", WACOM_PKGLEN_INTUOS, 65024, 40640, 2047,
+ 63, INTUOSPL, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ .touch_max = 16 };
static const struct wacom_features wacom_features_0xF4 =
{ "Wacom Cintiq 24HD", WACOM_PKGLEN_INTUOS, 104480, 65600, 2047,
63, WACOM_24HD, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
@@ -2241,6 +2268,9 @@ const struct usb_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0x101) },
{ USB_DEVICE_WACOM(0x10D) },
{ USB_DEVICE_WACOM(0x304) },
+ { USB_DEVICE_DETAILED(0x314, USB_CLASS_HID, 0, 0) },
+ { USB_DEVICE_DETAILED(0x315, USB_CLASS_HID, 0, 0) },
+ { USB_DEVICE_DETAILED(0x317, USB_CLASS_HID, 0, 0) },
{ USB_DEVICE_WACOM(0x4001) },
{ USB_DEVICE_WACOM(0x47) },
{ USB_DEVICE_WACOM(0xF4) },
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index dfc9e08..d6dec58 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -76,6 +76,9 @@ enum {
INTUOS5S,
INTUOS5,
INTUOS5L,
+ INTUOSPS,
+ INTUOSPM,
+ INTUOSPL,
WACOM_21UX2,
WACOM_22HD,
DTK,
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Ryan Mallon @ 2013-09-10 23:32 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: carl, linux-input, linux-kernel, Ryan Mallon
In-Reply-To: <1378855973-11595-1-git-send-email-rmallon@gmail.com>
Call input_inject_event rather than input_event in uinput_inject_event. This
mirrors the behaviour of evdev_write. input_inject_event will ignore the
injected event if the uinput input device has been grabbed for exclusive
access by a handler other than uinput.
Signed-off-by: Ryan Mallon <rmallon@gmail.com>
---
drivers/input/misc/uinput.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 6aea346..0962ecb 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -443,7 +443,7 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
if (input_event_from_user(buffer + bytes, &ev))
return -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_inject_event(udev->dev, ev.type, ev.code, ev.value);
bytes += input_event_size();
}
--
1.7.9.7
^ permalink raw reply related
* [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Ryan Mallon @ 2013-09-10 23:32 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: carl, linux-input, linux-kernel, Ryan Mallon
Rework the code in uinput_inject_event so that it matches the code in
evdev_write and allows injecting more than one event, or zero events.
Signed-off-by: Ryan Mallon <rmallon@gmail.com>
---
drivers/input/misc/uinput.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a0a4bba..6aea346 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
const char __user *buffer, size_t count)
{
struct input_event ev;
+ size_t bytes = 0;
- if (count < input_event_size())
+ if (count != 0 && count < input_event_size())
return -EINVAL;
- if (input_event_from_user(buffer, &ev))
- return -EFAULT;
+ while (bytes + input_event_size() <= count) {
+ if (input_event_from_user(buffer + bytes, &ev))
+ return -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_event(udev->dev, ev.type, ev.code, ev.value);
+ bytes += input_event_size();
+ }
- return input_event_size();
+ return bytes;
}
static ssize_t uinput_write(struct file *file, const char __user *buffer,
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Dmitry Torokhov @ 2013-09-11 0:12 UTC (permalink / raw)
To: Ryan Mallon; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <1378855973-11595-2-git-send-email-rmallon@gmail.com>
Hi Ryan,
On Wed, Sep 11, 2013 at 09:32:53AM +1000, Ryan Mallon wrote:
> Call input_inject_event rather than input_event in uinput_inject_event. This
> mirrors the behaviour of evdev_write. input_inject_event will ignore the
> injected event if the uinput input device has been grabbed for exclusive
> access by a handler other than uinput.
uinput is not an input handler but input driver and therefore should
continue using input_event().
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Dmitry Torokhov @ 2013-09-11 0:14 UTC (permalink / raw)
To: Ryan Mallon; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <1378855973-11595-1-git-send-email-rmallon@gmail.com>
Hi Ryan,
On Wed, Sep 11, 2013 at 09:32:52AM +1000, Ryan Mallon wrote:
> Rework the code in uinput_inject_event so that it matches the code in
> evdev_write and allows injecting more than one event, or zero events.
>
> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
> ---
> drivers/input/misc/uinput.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index a0a4bba..6aea346 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
> const char __user *buffer, size_t count)
> {
> struct input_event ev;
> + size_t bytes = 0;
>
> - if (count < input_event_size())
> + if (count != 0 && count < input_event_size())
> return -EINVAL;
>
> - if (input_event_from_user(buffer, &ev))
> - return -EFAULT;
> + while (bytes + input_event_size() <= count) {
> + if (input_event_from_user(buffer + bytes, &ev))
> + return -EFAULT;
Unless the failure is in the very first packet we should tell the user
how many bytes we have transferred successfully instead of returning
-EFAULT.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] uinput: Use input_inject_event in uinput_inject_event
From: Ryan Mallon @ 2013-09-11 0:26 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <20130911001235.GA5840@core.coreip.homeip.net>
On 11/09/13 10:12, Dmitry Torokhov wrote:
> Hi Ryan,
>
> On Wed, Sep 11, 2013 at 09:32:53AM +1000, Ryan Mallon wrote:
>> Call input_inject_event rather than input_event in uinput_inject_event. This
>> mirrors the behaviour of evdev_write. input_inject_event will ignore the
>> injected event if the uinput input device has been grabbed for exclusive
>> access by a handler other than uinput.
>
> uinput is not an input handler but input driver and therefore should
> continue using input_event().
Ah, I wasn't sure about this, which is why I broke it out into a
separate patch. Sorry for the noise.
~Ryan
^ permalink raw reply
* Re: [PATCH 1/2] uinput: Support injecting multiple events in one write() call
From: Ryan Mallon @ 2013-09-11 0:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: rydberg, carl, linux-input, linux-kernel
In-Reply-To: <20130911001428.GB5840@core.coreip.homeip.net>
On 11/09/13 10:14, Dmitry Torokhov wrote:
> Hi Ryan,
>
> On Wed, Sep 11, 2013 at 09:32:52AM +1000, Ryan Mallon wrote:
>> Rework the code in uinput_inject_event so that it matches the code in
>> evdev_write and allows injecting more than one event, or zero events.
>>
>> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
>> ---
>> drivers/input/misc/uinput.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index a0a4bba..6aea346 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *udev,
>> const char __user *buffer, size_t count)
>> {
>> struct input_event ev;
>> + size_t bytes = 0;
>>
>> - if (count < input_event_size())
>> + if (count != 0 && count < input_event_size())
>> return -EINVAL;
>>
>> - if (input_event_from_user(buffer, &ev))
>> - return -EFAULT;
>> + while (bytes + input_event_size() <= count) {
>> + if (input_event_from_user(buffer + bytes, &ev))
>> + return -EFAULT;
> Unless the failure is in the very first packet we should tell the user
> how many bytes we have transferred successfully instead of returning
> -EFAULT.
Good point. evdev is also broken in this regard, I'll send a follow-up
patch for that once you are happy with this one. Updated patch is below:
~Ryan
---
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a0a4bba..92ca2c5 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -434,16 +434,20 @@ static ssize_t uinput_inject_event(struct uinput_device *u
const char __user *buffer, size_t count)
{
struct input_event ev;
+ size_t bytes = 0;
- if (count < input_event_size())
+ if (count != 0 && count < input_event_size())
return -EINVAL;
- if (input_event_from_user(buffer, &ev))
- return -EFAULT;
+ while (bytes + input_event_size() <= count) {
+ if (input_event_from_user(buffer + bytes, &ev))
+ return bytes ?: -EFAULT;
- input_event(udev->dev, ev.type, ev.code, ev.value);
+ input_event(udev->dev, ev.type, ev.code, ev.value);
+ bytes += input_event_size();
+ }
- return input_event_size();
+ return bytes;
}
static ssize_t uinput_write(struct file *file, const char __user *buffer,
^ permalink raw reply related
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah : @ 2013-09-11 16:02 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Zubair Lutfullah, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <522C7EDB.7090306-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Sun, Sep 08, 2013 at 02:42:51PM +0100, Jonathan Cameron wrote:
> On 09/01/13 12:17, Zubair Lutfullah wrote:
Thanks for the review. Fitting this driver in the usual trigger ABI
is a bad workaround indeed.
I am still a bit unclear regarding the implementation.
Questions and replies inline.
I hope I find the time to pursue the approach you suggest.
Thanks
ZubairLK
> > #include <linux/of.h>
> Ouch, how did that slip in here. Dropping this should have been
> in a separate patch as it doesn't really have anything to do with
> the rest of this series.
>
> > -#include <linux/iio/machine.h>
Squashed it in.. I'll do a cleanup patch separately.
> > #include <linux/iio/driver.h>
> > +static void tiadc_step_config(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > unsigned int stepconfig;
> > - int i, steps;
> > + int i, steps, chan;
> Whilst I agree that pulling chan up here is slightly cleaner, technically that
> is a bit of unconnected cleanup and should have been in a precursor patch.
> (though don't bother doing so now!)
Again. Squashed in..
> > +
> > +static irqreturn_t tiadc_irq(int irq, void *private)
> > +{
...
> > + return IRQ_HANDLED;
> > + } else if (status & IRQENB_FIFO1THRES) {
> > + /* Trigger to push FIFO data to iio buffer */
> > + tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
>
> So this is triggering at the threshold. Thus it isn't acting as a conventional
> trigger in the IIO sense at all. It is of no real use to anything outside
> this driver, so at the very least add the two callbacks to prevent this
> driver using any other trigger and the trigger being used by any other device.
> However, I'm really not happy with this solution.
>
> Take a look at running with no trigger at all and calling the buffer
> filling more directly. The issue is that if we have a trigger now it will
> become part of the userspace interface and become a pain to drop later.
>
> It is valid here I think to use the INDIO_BUFFER_HARDWARE mode (which skips
> the checks for a trigger) then just don't call iio_triggered_buffer_post_enable
> or iio_triggered_buffer_predisable. Then make this a threaded interrupt and
> put your tiadc_trigger_h as the thread. Finally just return IRQ_WAKE_THREAD
> in this case. You'll have to pull a few bits out of
> industrialio-triggered-buffer.c to create the kfifo etc, but no more than
> a few lines given the pollfunc won't exist.
If I understand correctly.
buffer setup ops for postenable and predisable have
return iio_triggered_buffer_*(indio_dev)
calls at the end. I return 0 instead.
Insead of iio_triggered_buffer setup. I only use iio_kfifo_alloc
(what size fifo is made? :s )
And do
indio_dev->modes |= INDIO_BUFFER_HARDWARE;
Then I use request_threaded_irq(*). The hardware IRQ line for the fifos
So the HW irq handler runs in interrupt context which wakes the threaded handler
which pushes samples to the iio buffer.
Shared IRQ line. ADC uses request_threaded_irq(). TSC uses request_irq()
ADC returns IRQ_WAKE_THREAD. But the TSC side returns NONE/HANDLED.
Does this change to a threaded irq affect the TSC side of things?
I just received an ack by Dmitry for that patch..
>
> Actually, might be cleaner to move the flush case into the thread as well
> and just have a single check in this top half of the handler. That bit
> is up to you given it is really a question of how long it take and hence
> if it is acceptable in interrupt context.
Sorry. I don't understand this paragraph.. Could you please elaborate?
The flushing fifo before/after enabling/disabling the buffer part?
I explained the reason behind the flushes
>
> > + iio_trigger_poll(indio_dev->trig, iio_get_time_ns());
> > + return IRQ_HANDLED;
> > + } else
> > + return IRQ_NONE;
> > +
> > +}
> > +
> > +static irqreturn_t tiadc_trigger_h(int irq, void *p)
> > +{
...
>
> > + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> > + for (k = 0; k < fifo1count; k = k + i) {
> As mentioned below, why not use 16 bit storage in the kfifo?
32 bit storage is because the FIFO is 32 bit in hardware.
It returns 12 bits data. some reserve bits. And 4 bit channel id.
The channel id is used to separate which channel data which.
Need the full 32 bits..
I've added a comment. Not here. but when the storage_bits = 32 is
set.
...
> > +static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
...
> Why would there be data in the fifo? Seems to me that flushing
> after disabling it and before enabling it should not both be
> necessary? Is this just a case of playing safe?
>
> > + /* Flush FIFO before starting sampling */
> > + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> > + for (i = 0; i < fifo1count; i++)
> > + read = tiadc_readl(adc_dev, REG_FIFO1);
Pre-flush was needed because TSC events would cause some
redundant ADC samples in the FIFO. Fixes have been sent to input.
Shouldn't happen now.
Post-flush is playing safe. There 'might' be a chance that
another sample comes in the time it takes for fifocount to be read
And that number of samples read from fifo. and then adc disabled.
> > +static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > +
> > + tiadc_step_config(indio_dev);
> > + kfree(adc_dev->data);
> blank line here please.
ok
> > + return 0;
> > +}
> > +
> > +static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
> > + .preenable = &tiadc_buffer_preenable,
> > + .postenable = &tiadc_buffer_postenable,
> > + .predisable = &tiadc_buffer_predisable,
> > + .postdisable = &tiadc_buffer_postdisable,
> > +};
> > +
> > +static const struct iio_trigger_ops tiadc_trigger_ops = {
> > + .owner = THIS_MODULE,
> > +};
> > +
>
> If you want a utility function for allocation, convention would
> say have one for freeing as well so that the code is obviously
> balanced and easy to check. Also this registers the buffer as
> well so the name should probably reflect that.
>
Noted.
> > +static int tiadc_iio_allocate_trigger(struct iio_dev *indio_dev)
> > +{
> > + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> > + struct iio_trigger *trig = adc_dev->trig;
> > + int ret;
> > +
> devm_iio_trigger_alloc is now available and will simplify things a little.
>
Trigger related stuff is redundant anyways as moving away from trigger..
> > + trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id);
> > @@ -120,6 +295,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
> > chan->scan_type.realbits = 12;
> > chan->scan_type.storagebits = 32;
> I never noticed this before but putting 12 bit values in 32 bit storage
> does seem rather wasteful. Far as I can see above there is no real
> reason for doing so. If there is, then please add a comment justifying this
> choice!
>
Answered earlier. I added a comment here explaining.
> > @@ -142,11 +318,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
...
> > * Hence we need to flush out this data.
> > */
> Is the comment above still valid? I'd guess it referred to this code
> which has now gone.
> >
Its valid when the comment is read completely. All ADC channels
are sampled even if only one is read. The one needed is returned
to userspace. The rest of the fifo is flushed.
Not the ideal way. But I'm focusing on continuous sampling mode at the moment
> > adc_dev->channels = channels;
>
> Use the devm interfaces and there is no need to keep a copy
> of the irq number about.
>
Noted.
...
> >
> devm_request_irq will simplify error handling a little.
>
Indeed error handling gets confusing. I'll update.
...
> > + iio_trigger_free(adc_dev->trig);
> There is a devm trigger allocation function now. Using that
> would make things a little cleaner.
Noted
>
> > + free_irq(adc_dev->irq, indio_dev);
> Best to use the devm interfaces for irq handling unless
> there is a good reason to not do so (cleaner code ;)
>
> > iio_device_unregister(indio_dev);
>
Noted.
> We would normally expect this function to run in exact
> reverse order of the probe. That is not the case here
> so please reorder things so it is.
> > + iio_buffer_unregister(indio_dev);
I'll look into it.
> > tiadc_channels_remove(indio_dev);
> >
> > step_en = get_adc_step_mask(adc_dev);
> > @@ -298,10 +495,16 @@ static int tiadc_resume(struct device *dev)
> >
> > /* Make sure ADC is powered up */
> This comment doesn't seem to me to be true any more.
> > restore = tiadc_readl(adc_dev, REG_CTRL);
> > - restore &= ~(CNTRLREG_POWERDOWN);
> Some coments here would be good to explain the sequence that
> is going on.
>
> I'm guessing a bit, but looks like you turn off the touch screen
> but leave everything else alone, then set the threshold then
> reenable the touch screen before powering up the screen?
>
To be honest. I don't even remember.
It might have been a patch I squashed in.
does not seem to be related to the continuous sampling I guess..
I should remove this and send it separately.
> > + restore &= ~(CNTRLREG_TSCSSENB);
...
> > tiadc_writel(adc_dev, REG_CTRL, restore);
> > return 0;
> > }
> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> > index db1791b..a372ebf 100644
> > --- a/include/linux/mfd/ti_am335x_tscadc.h
> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
> > @@ -46,17 +46,25 @@
> > /* adc device */
> > struct adc_device *adc;
> > +
> > + /* Context save */
> > + unsigned int irqstat;
> Where are these used?
> > + unsigned int ctrl;
> > };
Oops. Don't think they are used anywhere.
Thanks for pointing it out.
> >
> > static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
> >
Thanks
ZubairLK
^ permalink raw reply
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC
From: Zubair Lutfullah : @ 2013-09-11 16:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Cameron, Zubair Lutfullah,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
On Mon, Sep 09, 2013 at 09:12:30AM -0700, Dmitry Torokhov wrote:
> On Sun, Sep 08, 2013 at 12:29:26PM +0100, Jonathan Cameron wrote:
> > On 09/01/13 12:17, Zubair Lutfullah wrote:
> > > Enable shared IRQ to allow ADC to share IRQ line from
> > > parent MFD core. Only FIFO0 IRQs are for TSC and handled
> > > on the TSC side.
> > >
> > > Step mask would be updated from cached variable only previously.
> > > In rare cases when both TSC and ADC are used, the cached
> > > variable gets mixed up.
> > > The step mask is written with the required mask every time.
> > >
> > > Rachna Patil (TI) laid ground work for shared IRQ.
> > >
> > > Signed-off-by: Zubair Lutfullah <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Whilst I would have prefered an mfd under these drivers and elegant handling
> > of the interrupts, I guess if this works it is at least not terribly invasive.
> >
> > However, this does need an Ack from Dmitry before I can take it (or for
> > Dmitry to take it himself?)
>
> I completely agree with Jonathan, more elegant handling would be nice
> but current one will do for now.
>
> Since most of the work on the driver is going through IIO tree I think
> it would make sense for this patch to go through it as well.
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thank-you.
ZubairLK
^ permalink raw reply
* [PATCH v3 02/10] HID: zeroplus: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
The zeroplus HID driver was not checking the size of allocated values
in fields it used. A HID device could send a malicious output report
that would cause the driver to write beyond the output report allocation
during initialization, causing a heap overflow:
[ 1442.728680] usb 1-1: New USB device found, idVendor=0c12, idProduct=0005
...
[ 1466.243173] BUG kmalloc-192 (Tainted: G W ): Redzone overwritten
CVE-2013-2889
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-zpff.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-zpff.c b/drivers/hid/hid-zpff.c
index 6ec28a3..a29756c 100644
--- a/drivers/hid/hid-zpff.c
+++ b/drivers/hid/hid-zpff.c
@@ -68,21 +68,13 @@ static int zpff_init(struct hid_device *hid)
struct hid_report *report;
struct hid_input *hidinput = list_entry(hid->inputs.next,
struct hid_input, list);
- struct list_head *report_list =
- &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
- int error;
+ int i, error;
- if (list_empty(report_list)) {
- hid_err(hid, "no output report found\n");
- return -ENODEV;
- }
-
- report = list_entry(report_list->next, struct hid_report, list);
-
- if (report->maxfield < 4) {
- hid_err(hid, "not enough fields in report\n");
- return -ENODEV;
+ for (i = 0; i < 4; i++) {
+ report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, i, 1);
+ if (!report)
+ return -ENODEV;
}
zpff = kzalloc(sizeof(struct zpff_device), GFP_KERNEL);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 03/10] HID: sony: validate HID output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
This driver must validate the availability of the HID output report and
its size before it can write LED states via buzz_set_leds(). This stops
a heap overflow that is possible if a device provides a malicious HID
output report:
[ 108.171280] usb 1-1: New USB device found, idVendor=054c, idProduct=0002
...
[ 117.507877] BUG kmalloc-192 (Not tainted): Redzone overwritten
CVE-2013-2890
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-sony.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 30dbb6b..b18320d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -537,6 +537,10 @@ static int buzz_init(struct hid_device *hdev)
drv_data = hid_get_drvdata(hdev);
BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+ /* Validate expected report characteristics. */
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+ return -ENODEV;
+
buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
if (!buzz) {
hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 04/10] HID: steelseries: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
A HID device could send a malicious output report that would cause the
steelseries HID driver to write beyond the output report allocation
during initialization, causing a heap overflow:
[ 167.981534] usb 1-1: New USB device found, idVendor=1038, idProduct=1410
...
[ 182.050547] BUG kmalloc-256 (Tainted: G W ): Redzone overwritten
CVE-2013-2891
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-steelseries.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index d164911..29f328f 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -249,6 +249,11 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
goto err_free;
}
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 16)) {
+ ret = -ENODEV;
+ goto err_free;
+ }
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 05/10] HID: LG: validate HID output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
A HID device could send a malicious output report that would cause the
lg, lg3, and lg4 HID drivers to write beyond the output report allocation
during an event, causing a heap overflow:
[ 325.245240] usb 1-1: New USB device found, idVendor=046d, idProduct=c287
...
[ 414.518960] BUG kmalloc-4096 (Not tainted): Redzone overwritten
Additionally, while lg2 did correctly validate the report details, it was
cleaned up and shortened.
CVE-2013-2893
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- no changes
drivers/hid/hid-lg2ff.c | 19 +++----------------
drivers/hid/hid-lg3ff.c | 29 ++++++-----------------------
drivers/hid/hid-lg4ff.c | 20 +-------------------
drivers/hid/hid-lgff.c | 17 ++---------------
4 files changed, 12 insertions(+), 73 deletions(-)
diff --git a/drivers/hid/hid-lg2ff.c b/drivers/hid/hid-lg2ff.c
index b3cd150..1a42eaa 100644
--- a/drivers/hid/hid-lg2ff.c
+++ b/drivers/hid/hid-lg2ff.c
@@ -64,26 +64,13 @@ int lg2ff_init(struct hid_device *hid)
struct hid_report *report;
struct hid_input *hidinput = list_entry(hid->inputs.next,
struct hid_input, list);
- struct list_head *report_list =
- &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
int error;
- if (list_empty(report_list)) {
- hid_err(hid, "no output report found\n");
+ /* Check that the report looks ok */
+ report = hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7);
+ if (!report)
return -ENODEV;
- }
-
- report = list_entry(report_list->next, struct hid_report, list);
-
- if (report->maxfield < 1) {
- hid_err(hid, "output report is empty\n");
- return -ENODEV;
- }
- if (report->field[0]->report_count < 7) {
- hid_err(hid, "not enough values in the field\n");
- return -ENODEV;
- }
lg2ff = kmalloc(sizeof(struct lg2ff_device), GFP_KERNEL);
if (!lg2ff)
diff --git a/drivers/hid/hid-lg3ff.c b/drivers/hid/hid-lg3ff.c
index e52f181..8c2da18 100644
--- a/drivers/hid/hid-lg3ff.c
+++ b/drivers/hid/hid-lg3ff.c
@@ -66,10 +66,11 @@ static int hid_lg3ff_play(struct input_dev *dev, void *data,
int x, y;
/*
- * Maxusage should always be 63 (maximum fields)
- * likely a better way to ensure this data is clean
+ * Available values in the field should always be 63, but we only use up to
+ * 35. Instead, clear the entire area, however big it is.
*/
- memset(report->field[0]->value, 0, sizeof(__s32)*report->field[0]->maxusage);
+ memset(report->field[0]->value, 0,
+ sizeof(__s32) * report->field[0]->report_count);
switch (effect->type) {
case FF_CONSTANT:
@@ -129,32 +130,14 @@ static const signed short ff3_joystick_ac[] = {
int lg3ff_init(struct hid_device *hid)
{
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
- struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
- struct hid_report *report;
- struct hid_field *field;
const signed short *ff_bits = ff3_joystick_ac;
int error;
int i;
- /* Find the report to use */
- if (list_empty(report_list)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- if (!report) {
- hid_err(hid, "NULL output report\n");
- return -1;
- }
-
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
- return -1;
- }
+ if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35))
+ return -ENODEV;
/* Assume single fixed device G940 */
for (i = 0; ff_bits[i] >= 0; i++)
diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index 0ddae2a..8782fe1 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -484,34 +484,16 @@ static enum led_brightness lg4ff_led_get_brightness(struct led_classdev *led_cde
int lg4ff_init(struct hid_device *hid)
{
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
- struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
- struct hid_report *report;
- struct hid_field *field;
struct lg4ff_device_entry *entry;
struct lg_drv_data *drv_data;
struct usb_device_descriptor *udesc;
int error, i, j;
__u16 bcdDevice, rev_maj, rev_min;
- /* Find the report to use */
- if (list_empty(report_list)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- if (!report) {
- hid_err(hid, "NULL output report\n");
+ if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
return -1;
- }
-
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
- return -1;
- }
/* Check what wheel has been connected */
for (i = 0; i < ARRAY_SIZE(lg4ff_devices); i++) {
diff --git a/drivers/hid/hid-lgff.c b/drivers/hid/hid-lgff.c
index d7ea8c8..e1394af 100644
--- a/drivers/hid/hid-lgff.c
+++ b/drivers/hid/hid-lgff.c
@@ -128,27 +128,14 @@ static void hid_lgff_set_autocenter(struct input_dev *dev, u16 magnitude)
int lgff_init(struct hid_device* hid)
{
struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
- struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
struct input_dev *dev = hidinput->input;
- struct hid_report *report;
- struct hid_field *field;
const signed short *ff_bits = ff_joystick;
int error;
int i;
- /* Find the report to use */
- if (list_empty(report_list)) {
- hid_err(hid, "No output report found\n");
- return -1;
- }
-
/* Check that the report looks ok */
- report = list_entry(report_list->next, struct hid_report, list);
- field = report->field[0];
- if (!field) {
- hid_err(hid, "NULL field\n");
- return -1;
- }
+ if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 7))
+ return -ENODEV;
for (i = 0; i < ARRAY_SIZE(devices); i++) {
if (dev->id.vendor == devices[i].idVendor &&
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 06/10] HID: lenovo-tpkbd: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
From: Kees Cook <keescook@chromium.org>
A HID device could send a malicious output report that would cause the
lenovo-tpkbd HID driver to write just beyond the output report allocation
during initialization, causing a heap overflow:
[ 76.109807] usb 1-1: New USB device found, idVendor=17ef, idProduct=6009
...
[ 80.462540] BUG kmalloc-192 (Tainted: G W ): Redzone overwritten
CVE-2013-2894
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- fix feature report check for report ID 4
drivers/hid/hid-lenovo-tpkbd.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 07837f5..762d988 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -339,7 +339,15 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
struct tpkbd_data_pointer *data_pointer;
size_t name_sz = strlen(dev_name(dev)) + 16;
char *name_mute, *name_micmute;
- int ret;
+ int i, ret;
+
+ /* Validate required reports. */
+ for (i = 0; i < 4; i++) {
+ if (!hid_validate_values(hdev, HID_FEATURE_REPORT, 4, i, 1))
+ return -ENODEV;
+ }
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
+ return -ENODEV;
if (sysfs_create_group(&hdev->dev.kobj,
&tpkbd_attr_group_pointer)) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 07/10] HID: logitech-dj: validate output report details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
From: Kees Cook <keescook@chromium.org>
A HID device could send a malicious output report that would cause the
logitech-dj HID driver to leak kernel memory contents to the device, or
trigger a NULL dereference during initialization:
[ 304.424553] usb 1-1: New USB device found, idVendor=046d, idProduct=c52b
...
[ 304.780467] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
[ 304.781409] IP: [<ffffffff815d50aa>] logi_dj_recv_send_report.isra.11+0x1a/0x90
CVE-2013-2895
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
---
v3:
- check for the whole size of the DJ report, as per the spec
drivers/hid/hid-logitech-dj.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 7800b14..2e53024 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -461,7 +461,7 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
struct hid_report *report;
struct hid_report_enum *output_report_enum;
u8 *data = (u8 *)(&dj_report->device_index);
- int i;
+ unsigned int i;
output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
@@ -471,7 +471,7 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
return -ENODEV;
}
- for (i = 0; i < report->field[0]->report_count; i++)
+ for (i = 0; i < DJREPORT_SHORT_LENGTH - 1; i++)
report->field[0]->value[i] = data[i];
hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
@@ -791,6 +791,12 @@ static int logi_dj_probe(struct hid_device *hdev,
goto hid_parse_fail;
}
+ if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, REPORT_ID_DJ_SHORT,
+ 0, DJREPORT_SHORT_LENGTH - 1)) {
+ retval = -ENODEV;
+ goto hid_parse_fail;
+ }
+
/* Starts the usb device and connects to upper interfaces hiddev and
* hidraw */
retval = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 09/10] HID: multitouch: validate indexes details
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
When working on report indexes, always validate that they are in bounds.
Without this, a HID device could report a malicious feature report that
could trick the driver into a heap overflow:
[ 634.885003] usb 1-1: New USB device found, idVendor=0596, idProduct=0500
...
[ 676.469629] BUG kmalloc-192 (Tainted: G W ): Redzone overwritten
Note that we need to change the indexes from s8 to s16 as they can
be between -1 and 255.
CVE-2013-2897
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- extract from hid-multitouch the generic checks so that every hid drivers will
benefit from them
- change __s8 index declarations into __s16
- use usage_index for the input_mode index instead of a half working code
- check the indexes validities only once
drivers/hid/hid-multitouch.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ac28f08..5e5fe1b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -101,9 +101,9 @@ struct mt_device {
unsigned last_slot_field; /* the last field of a slot */
unsigned mt_report_id; /* the report ID of the multitouch device */
unsigned pen_report_id; /* the report ID of the pen device */
- __s8 inputmode; /* InputMode HID feature, -1 if non-existent */
- __s8 inputmode_index; /* InputMode HID feature index in the report */
- __s8 maxcontact_report_id; /* Maximum Contact Number HID feature,
+ __s16 inputmode; /* InputMode HID feature, -1 if non-existent */
+ __s16 inputmode_index; /* InputMode HID feature index in the report */
+ __s16 maxcontact_report_id; /* Maximum Contact Number HID feature,
-1 if non-existent */
__u8 num_received; /* how many contacts we received */
__u8 num_expected; /* expected last contact index */
@@ -312,20 +312,18 @@ static void mt_feature_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
{
struct mt_device *td = hid_get_drvdata(hdev);
- int i;
switch (usage->hid) {
case HID_DG_INPUTMODE:
- td->inputmode = field->report->id;
- td->inputmode_index = 0; /* has to be updated below */
-
- for (i=0; i < field->maxusage; i++) {
- if (field->usage[i].hid == usage->hid) {
- td->inputmode_index = i;
- break;
- }
+ /* Ignore if value index is out of bounds. */
+ if (usage->usage_index >= field->report_count) {
+ dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
+ break;
}
+ td->inputmode = field->report->id;
+ td->inputmode_index = usage->usage_index;
+
break;
case HID_DG_CONTACTMAX:
td->maxcontact_report_id = field->report->id;
@@ -511,6 +509,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
mt_store_field(usage, td, hi);
return 1;
case HID_DG_CONTACTCOUNT:
+ /* Ignore if indexes are out of bounds. */
+ if (field->index >= field->report->maxfield ||
+ usage->usage_index >= field->report_count)
+ return 1;
td->cc_index = field->index;
td->cc_value_index = usage->usage_index;
return 1;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 10/10] HID: lenovo-tpkbd: fix leak if tpkbd_probe_tp fails
From: Benjamin Tissoires @ 2013-09-11 19:56 UTC (permalink / raw)
To: Benjamin Tissoires, Kees Cook, Henrik Rydberg, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <1378929419-6269-1-git-send-email-benjamin.tissoires@redhat.com>
If tpkbd_probe_tp() bails out, the probe() function return an error,
but hid_hw_stop() is never called.
fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1003998
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
v3:
- new patch
drivers/hid/hid-lenovo-tpkbd.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 762d988..31cf29a 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -414,22 +414,27 @@ static int tpkbd_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "hid_parse failed\n");
- goto err_free;
+ goto err;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hid_hw_start failed\n");
- goto err_free;
+ goto err;
}
uhdev = (struct usbhid_device *) hdev->driver_data;
- if (uhdev->ifnum == 1)
- return tpkbd_probe_tp(hdev);
+ if (uhdev->ifnum == 1) {
+ ret = tpkbd_probe_tp(hdev);
+ if (ret)
+ goto err_hid;
+ }
return 0;
-err_free:
+err_hid:
+ hid_hw_stop(hdev);
+err:
return ret;
}
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox