On Mo, 2020-01-20 at 11:54 -0800, Dmitry Torokhov wrote:

On Tue, Jan 07, 2020 at 06:17:40PM +0100, Lucas Stach wrote:

quoted

This change allows the device firmware to be updated by putting a
firmware
file in /lib/firmware and providing the name of the file via the
update_fw
sysfs property. The driver will then flash the firmware image into
the
controller internal storage and restart the controller to activate
the new
firmware.

The implementation was done by looking at the the messages passed
between
the controller and proprietary vendor update tool. Not every detail
of the
protocol is totally well understood, so the implementation still
has some
"monkey see, monkey do" parts, as far as they have been found to be
required
for the update to succeed.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/input/touchscreen/exc3000.c | 248
+++++++++++++++++++++++++++-
 1 file changed, 246 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/exc3000.c

b/drivers/input/touchscreen/exc3000.c
index ce83914d65ff..f9a9820dc232 100644

--- a/drivers/input/touchscreen/exc3000.c
+++ b/drivers/input/touchscreen/exc3000.c

@@ -3,8 +3,8 @@
  * Driver for I2C connected EETI EXC3000 multiple touch controller
  *
  * Copyright (C) 2017 Ahmet Inan <inan@distec.de>
- *
- * minimal implementation based on egalax_ts.c and egalax_i2c.c
+ * Copyright (C) 2019 Pengutronix <kernel@pengutronix.de>
+ * Copyright (C) 2019 Zodiac Inflight Innovations
  */
 
 #include <linux/bitops.h>

@@ -18,6 +18,8 @@
 #include <linux/of.h>
 #include <linux/timer.h>
 #include <asm/unaligned.h>
+#include <linux/firmware.h>
+#include <linux/delay.h>
 
 #define EXC3000_NUM_SLOTS		10
 #define EXC3000_SLOTS_PER_FRAME		5

@@ -37,6 +39,7 @@ struct exc3000_data {
 	struct mutex vendor_data_lock;
 	struct completion vendor_data_done;
 	char *type, *model, *fw_rev;
+	int update_status;
 };
 
 static void exc3000_report_slots(struct input_dev *input,

@@ -215,6 +218,8 @@ static int exc3000_populate_device_info(struct

exc3000_data *data)
 	if (ret < 0)
 		return -ENODEV;
 
+	if (data->type)
+		devm_kfree(dev, data->type);
 	data->type = devm_kmemdup(dev, &response[1], ret - 1,
GFP_KERNEL);

This makes me uncomfortable, as this changes the release order of the
resources (newly re-allocated memory will be freed first). Please
reassure me that it will work fine WRT device removal (or,
alternatively, we will need devm_realloc() that would preserve devm
release ordering.

The cached information here is only used for the sysfs FW info
attributes. In v2 I split those out into a separate group, which gets
removed and registered again when refreshing this information after the
FW update. This keeps the devm ordering the same and fixes a possible
use-after-free through sysfs reads at the same time.

Regards,
Lucas