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:
> > 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@xxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxx>
> > - *
> > - * minimal implementation based on egalax_ts.c and egalax_i2c.c
> > + * Copyright (C) 2019 Pengutronix <kernel@xxxxxxxxxxxxxx>
> > + * 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
