From 003f01c780020daa9a06dea1db495b553a868c29 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Mon, 4 Nov 2019 15:58:34 -0800 Subject: [PATCH 1/6] Input: synaptics-rmi4 - fix video buffer size The video buffer used by the queue is a vb2_v4l2_buffer, not a plain vb2_buffer. Using the wrong type causes the allocation of the buffer storage to be too small, causing a out of bounds write when __init_vb2_v4l2_buffer initializes the buffer. Signed-off-by: Lucas Stach Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20191104114454.10500-1-l.stach@pengutronix.de Signed-off-by: Dmitry Torokhov --- drivers/input/rmi4/rmi_f54.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c index 710b025954860..4841354af0d7e 100644 --- a/drivers/input/rmi4/rmi_f54.c +++ b/drivers/input/rmi4/rmi_f54.c @@ -359,7 +359,7 @@ static const struct vb2_ops rmi_f54_queue_ops = { static const struct vb2_queue rmi_f54_queue = { .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, .io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ, - .buf_struct_size = sizeof(struct vb2_buffer), + .buf_struct_size = sizeof(struct vb2_v4l2_buffer), .ops = &rmi_f54_queue_ops, .mem_ops = &vb2_vmalloc_memops, .timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, From f6aabe1ff1d9d7bad0879253011216438bdb2530 Mon Sep 17 00:00:00 2001 From: Andrew Duggan Date: Mon, 4 Nov 2019 16:06:44 -0800 Subject: [PATCH 2/6] Input: synaptics-rmi4 - disable the relative position IRQ in the F12 driver This patch fixes an issue seen on HID touchpads which report finger positions using RMI4 Function 12. The issue manifests itself as spurious button presses as described in: https://www.spinics.net/lists/linux-input/msg58618.html Commit 24d28e4f1271 ("Input: synaptics-rmi4 - convert irq distribution to irq_domain") switched the RMI4 driver to using an irq_domain to handle RMI4 function interrupts. Functions with more then one interrupt now have each interrupt mapped to their own IRQ and IRQ handler. The result of this change is that the F12 IRQ handler was now getting called twice. Once for the absolute data interrupt and once for the relative data interrupt. For HID devices, calling rmi_f12_attention() a second time causes the attn_data data pointer and size to be set incorrectly. When the touchpad button is pressed, F30 will generate an interrupt and attempt to read the F30 data from the invalid attn_data data pointer and report incorrect button events. This patch disables the F12 relative interrupt which prevents rmi_f12_attention() from being called twice. Signed-off-by: Andrew Duggan Reported-by: Simon Wood Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20191025002527.3189-2-aduggan@synaptics.com Signed-off-by: Dmitry Torokhov --- drivers/input/rmi4/rmi_f12.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c index d20a5d6780d1d..734077f2c40b7 100644 --- a/drivers/input/rmi4/rmi_f12.c +++ b/drivers/input/rmi4/rmi_f12.c @@ -55,6 +55,9 @@ struct f12_data { const struct rmi_register_desc_item *data15; u16 data15_offset; + + unsigned long *abs_mask; + unsigned long *rel_mask; }; static int rmi_f12_read_sensor_tuning(struct f12_data *f12) @@ -291,9 +294,18 @@ static int rmi_f12_write_control_regs(struct rmi_function *fn) static int rmi_f12_config(struct rmi_function *fn) { struct rmi_driver *drv = fn->rmi_dev->driver; + struct f12_data *f12 = dev_get_drvdata(&fn->dev); + struct rmi_2d_sensor *sensor; int ret; - drv->set_irq_bits(fn->rmi_dev, fn->irq_mask); + sensor = &f12->sensor; + + if (!sensor->report_abs) + drv->clear_irq_bits(fn->rmi_dev, f12->abs_mask); + else + drv->set_irq_bits(fn->rmi_dev, f12->abs_mask); + + drv->clear_irq_bits(fn->rmi_dev, f12->rel_mask); ret = rmi_f12_write_control_regs(fn); if (ret) @@ -315,9 +327,12 @@ static int rmi_f12_probe(struct rmi_function *fn) struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev); struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev); u16 data_offset = 0; + int mask_size; rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__); + mask_size = BITS_TO_LONGS(drvdata->irq_count) * sizeof(unsigned long); + ret = rmi_read(fn->rmi_dev, query_addr, &buf); if (ret < 0) { dev_err(&fn->dev, "Failed to read general info register: %d\n", @@ -332,10 +347,19 @@ static int rmi_f12_probe(struct rmi_function *fn) return -ENODEV; } - f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data), GFP_KERNEL); + f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data) + mask_size * 2, + GFP_KERNEL); if (!f12) return -ENOMEM; + f12->abs_mask = (unsigned long *)((char *)f12 + + sizeof(struct f12_data)); + f12->rel_mask = (unsigned long *)((char *)f12 + + sizeof(struct f12_data) + mask_size); + + set_bit(fn->irq_pos, f12->abs_mask); + set_bit(fn->irq_pos + 1, f12->rel_mask); + f12->has_dribble = !!(buf & BIT(3)); if (fn->dev.of_node) { From 5d40d95e7e64756cc30606c2ba169271704d47cb Mon Sep 17 00:00:00 2001 From: Andrew Duggan Date: Mon, 4 Nov 2019 16:07:30 -0800 Subject: [PATCH 3/6] Input: synaptics-rmi4 - do not consume more data than we have (F11, F12) Currently, rmi_f11_attention() and rmi_f12_attention() functions update the attn_data data pointer and size based on the size of the expected size of the attention data. However, if the actual valid data in the attn buffer is less then the expected value then the updated data pointer will point to memory beyond the end of the attn buffer. Using the calculated valid_bytes instead will prevent this from happening. Signed-off-by: Andrew Duggan Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20191025002527.3189-3-aduggan@synaptics.com Signed-off-by: Dmitry Torokhov --- drivers/input/rmi4/rmi_f11.c | 4 ++-- drivers/input/rmi4/rmi_f12.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c index f28a7158b2efc..26c239325f95c 100644 --- a/drivers/input/rmi4/rmi_f11.c +++ b/drivers/input/rmi4/rmi_f11.c @@ -1284,8 +1284,8 @@ static irqreturn_t rmi_f11_attention(int irq, void *ctx) valid_bytes = f11->sensor.attn_size; memcpy(f11->sensor.data_pkt, drvdata->attn_data.data, valid_bytes); - drvdata->attn_data.data += f11->sensor.attn_size; - drvdata->attn_data.size -= f11->sensor.attn_size; + drvdata->attn_data.data += valid_bytes; + drvdata->attn_data.size -= valid_bytes; } else { error = rmi_read_block(rmi_dev, data_base_addr, f11->sensor.data_pkt, diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c index 734077f2c40b7..7e97944f76165 100644 --- a/drivers/input/rmi4/rmi_f12.c +++ b/drivers/input/rmi4/rmi_f12.c @@ -212,8 +212,8 @@ static irqreturn_t rmi_f12_attention(int irq, void *ctx) valid_bytes = sensor->attn_size; memcpy(sensor->data_pkt, drvdata->attn_data.data, valid_bytes); - drvdata->attn_data.data += sensor->attn_size; - drvdata->attn_data.size -= sensor->attn_size; + drvdata->attn_data.data += valid_bytes; + drvdata->attn_data.size -= valid_bytes; } else { retval = rmi_read_block(rmi_dev, f12->data_addr, sensor->data_pkt, sensor->pkt_size); From 310ca2a61c410b161ae64fd19a3475500ef9eafb Mon Sep 17 00:00:00 2001 From: Andrew Duggan Date: Mon, 4 Nov 2019 16:09:22 -0800 Subject: [PATCH 4/6] Input: synaptics-rmi4 - remove unused result_bits mask The result_bits mask is no longer used by the driver and should be removed. Signed-off-by: Andrew Duggan Link: https://lore.kernel.org/r/20191025002527.3189-4-aduggan@synaptics.com Signed-off-by: Dmitry Torokhov --- drivers/input/rmi4/rmi_f11.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c index 26c239325f95c..bbf9ae9f3f0cb 100644 --- a/drivers/input/rmi4/rmi_f11.c +++ b/drivers/input/rmi4/rmi_f11.c @@ -510,7 +510,6 @@ struct f11_data { struct rmi_2d_sensor_platform_data sensor_pdata; unsigned long *abs_mask; unsigned long *rel_mask; - unsigned long *result_bits; }; enum f11_finger_state { @@ -1057,7 +1056,7 @@ static int rmi_f11_initialize(struct rmi_function *fn) /* ** init instance data, fill in values and create any sysfs files */ - f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data) + mask_size * 3, + f11 = devm_kzalloc(&fn->dev, sizeof(struct f11_data) + mask_size * 2, GFP_KERNEL); if (!f11) return -ENOMEM; @@ -1076,8 +1075,6 @@ static int rmi_f11_initialize(struct rmi_function *fn) + sizeof(struct f11_data)); f11->rel_mask = (unsigned long *)((char *)f11 + sizeof(struct f11_data) + mask_size); - f11->result_bits = (unsigned long *)((char *)f11 - + sizeof(struct f11_data) + mask_size * 2); set_bit(fn->irq_pos, f11->abs_mask); set_bit(fn->irq_pos + 1, f11->rel_mask); From 549766ac2ac1f6c8bb85906bbcea759541bb19a2 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Tue, 12 Nov 2019 16:47:08 -0800 Subject: [PATCH 5/6] Input: synaptics-rmi4 - clear IRQ enables for F54 The driver for F54 just polls the status and doesn't even have a IRQ handler registered. Make sure to disable all F54 IRQs, so we don't crash the kernel on a nonexistent handler. Signed-off-by: Lucas Stach Link: https://lore.kernel.org/r/20191105114402.6009-1-l.stach@pengutronix.de Cc: stable@vger.kernel.org Signed-off-by: Dmitry Torokhov --- drivers/input/rmi4/rmi_f54.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c index 4841354af0d7e..484ae1f973304 100644 --- a/drivers/input/rmi4/rmi_f54.c +++ b/drivers/input/rmi4/rmi_f54.c @@ -601,7 +601,7 @@ static int rmi_f54_config(struct rmi_function *fn) { struct rmi_driver *drv = fn->rmi_dev->driver; - drv->set_irq_bits(fn->rmi_dev, fn->irq_mask); + drv->clear_irq_bits(fn->rmi_dev, fn->irq_mask); return 0; } From 79aae6acbef16f720a7949f8fc6ac69816c79d62 Mon Sep 17 00:00:00 2001 From: Pan Bian Date: Tue, 12 Nov 2019 17:04:54 -0800 Subject: [PATCH 6/6] Input: cyttsp4_core - fix use after free bug The device md->input is used after it is released. Setting the device data to NULL is unnecessary as the device is never used again. Instead, md->input should be assigned NULL to avoid accessing the freed memory accidently. Besides, checking md->si against NULL is superfluous as it points to a variable address, which cannot be NULL. Signed-off-by: Pan Bian Link: https://lore.kernel.org/r/1572936379-6423-1-git-send-email-bianpan2016@163.com Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/cyttsp4_core.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/input/touchscreen/cyttsp4_core.c b/drivers/input/touchscreen/cyttsp4_core.c index 4b22d49a0f49a..6bcffc930384a 100644 --- a/drivers/input/touchscreen/cyttsp4_core.c +++ b/drivers/input/touchscreen/cyttsp4_core.c @@ -1990,11 +1990,6 @@ static int cyttsp4_mt_probe(struct cyttsp4 *cd) /* get sysinfo */ md->si = &cd->sysinfo; - if (!md->si) { - dev_err(dev, "%s: Fail get sysinfo pointer from core p=%p\n", - __func__, md->si); - goto error_get_sysinfo; - } rc = cyttsp4_setup_input_device(cd); if (rc) @@ -2004,8 +1999,6 @@ static int cyttsp4_mt_probe(struct cyttsp4 *cd) error_init_input: input_free_device(md->input); -error_get_sysinfo: - input_set_drvdata(md->input, NULL); error_alloc_failed: dev_err(dev, "%s failed.\n", __func__); return rc;