[PATCH 1/2] Input: synaptics-rmi4 - validate register descriptor structure against its declared size

[PATCH 1/2] Input: synaptics-rmi4 - validate register descriptor structure against its declared size

[Greg Kroah-Hartman]

rmi_read_register_desc() trusts three independent device-supplied
quantities to be correct:
  - struct_size, taken from the presence-register header (up to 65535
    via buf[1]|buf[2]<<8 when buf[0]==0),
  - num_registers, the popcount of the presence bitmap (up to 255),
  - the per-register entries inside struct_buf, each a variable-length
    {reg_size, subpacket-continuation-bytes...} record.

But nothing checks that num_registers entries actually fit in
struct_size bytes, and nothing bounds the subpacket continuation chain,
which can cause two different types of overruns if left unchecked.

Fix this all up by properly checking the values every time and aborting
if anything is out of range.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: 2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/rmi4/rmi_driver.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index ccd9338a44db..9143f11e42a3 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -643,16 +643,24 @@ int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 	reg = find_first_bit(rdesc->presense_map, RMI_REG_DESC_PRESENSE_BITS);
 	for (i = 0; i < rdesc->num_registers; i++) {
 		struct rmi_register_desc_item *item = &rdesc->registers[i];
-		int reg_size = struct_buf[offset];
+		int reg_size;
+
+		if (offset >= rdesc->struct_size)
+			goto malformed;
+		reg_size = struct_buf[offset];
 
 		++offset;
 		if (reg_size == 0) {
+			if (offset + 2 > rdesc->struct_size)
+				goto malformed;
 			reg_size = struct_buf[offset] |
 					(struct_buf[offset + 1] << 8);
 			offset += 2;
 		}
 
 		if (reg_size == 0) {
+			if (offset + 4 > rdesc->struct_size)
+				goto malformed;
 			reg_size = struct_buf[offset] |
 					(struct_buf[offset + 1] << 8) |
 					(struct_buf[offset + 2] << 16) |
@@ -666,6 +674,9 @@ int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 		map_offset = 0;
 
 		do {
+			if (offset >= rdesc->struct_size ||
+			    map_offset >= RMI_REG_DESC_SUBPACKET_BITS)
+				goto malformed;
 			for (b = 0; b < 7; b++) {
 				if (struct_buf[offset] & (0x1 << b))
 					bitmap_set(item->subpacket_map,
@@ -688,6 +699,12 @@ int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 free_struct_buff:
 	kfree(struct_buf);
 	return ret;
+
+malformed:
+	dev_err(&d->dev,
+		"register descriptor structure does not match its declared size\n");
+	ret = -EIO;
+	goto free_struct_buff;
 }
 
 const struct rmi_register_desc_item *rmi_get_register_desc_item(
-- 
2.53.0

[PATCH 2/2] Input: synaptics-rmi4 - use u32 for reg_size to avoid sign extension into item->reg_size

[Greg Kroah-Hartman]

rmi_read_register_desc() builds the 4-byte register size from device
bytes:

	reg_size = struct_buf[offset] |
		   (struct_buf[offset + 1] << 8) |
		   (struct_buf[offset + 2] << 16) |
		   (struct_buf[offset + 3] << 24);

struct_buf is u8 *, so each byte is promoted to int before the shift.  A
device that supplies a top byte with bit 7 set (e.g. 00 00 00 00 00 00
80 in struct_buf to reach the 4-byte path with offset+3 = 0x80) makes
(0x80 << 24) overflow into the int sign bit, and the OR result is
negative.  reg_size is then assigned to item->reg_size, which is
unsigned long, so the negative int sign-extends to a value near
ULONG_MAX.

After this, bad things happen when numbers start wrapping and buffers
are allocatged based on those numbers, and then accessed based on those
buffers assuming to be a sane size (bigger or smaller).

Fix this all up by just properly making reg_size be a u32.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: b43d2c1e9353 ("Input: synaptics-rmi4 - add support for F12")
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/rmi4/rmi_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 9143f11e42a3..801096c7235e 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -643,7 +643,7 @@ int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 	reg = find_first_bit(rdesc->presense_map, RMI_REG_DESC_PRESENSE_BITS);
 	for (i = 0; i < rdesc->num_registers; i++) {
 		struct rmi_register_desc_item *item = &rdesc->registers[i];
-		int reg_size;
+		u32 reg_size;
 
 		if (offset >= rdesc->struct_size)
 			goto malformed;
-- 
2.53.0