linux-kernel - Re: [PATCHv1] Input: atmel_mxt_ts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180323182559.GB255819@dtor-ws>
Date:   Fri, 23 Mar 2018 11:25:59 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Sebastian Reichel <sebastian.reichel@...labora.co.uk>
Cc:     Sebastian Reichel <sre@...nel.org>, Nick Dyer <nick@...anahar.org>,
        linux-input@...r.kernel.org, Henrik Rydberg <rydberg@...math.org>,
        linux-kernel@...r.kernel.org, kernel@...labora.com,
        Nandor Han <nandor.han@...com>
Subject: Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update

Hi Sebastian, Nandor,

On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:
> From: Nandor Han <nandor.han@...com>
> 
> The automatic update mechanism will trigger an update if the
> info block CRCs are different between maxtouch configuration
> file (maxtouch.cfg) and chip.
> 
> The driver compared the CRCs without retrieving the chip CRC,
> resulting always in a failure and firmware flashing action
> triggered. The patch will fix this issue by retrieving the
> chip info block CRC before the check.
> 
> Suggested-by: Todd Weyenberg <Todd.Weyenberg@....ge.com>
> Signed-off-by: Nandor Han <nandor.han@...com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 7659bc48f1db..ab936e6b0286 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1728,6 +1728,25 @@ static int mxt_get_object_table(struct mxt_data *data)
>  	return error;
>  }
>  
> +static int mxt_get_info_block_crc(struct mxt_data *data)
> +{
> +	size_t table_size;
> +	int error;
> +	u8 info_block_crc[MXT_INFO_CHECKSUM_SIZE];
> +
> +	table_size = data->info.object_num * sizeof(struct mxt_object);
> +
> +	/* Read the info block CRC */
> +	error = __mxt_read_reg(data->client, MXT_OBJECT_START + table_size,
> +					sizeof(info_block_crc), info_block_crc);
> +	if (!error) {
> +		data->info_crc = info_block_crc[0] | (info_block_crc[1] << 8) |
> +				(info_block_crc[2] << 16);
> +	}
> +
> +	return error;
> +}
> +
>  static int mxt_read_t9_resolution(struct mxt_data *data)
>  {
>  	struct i2c_client *client = data->client;
> @@ -2077,6 +2096,14 @@ static int mxt_initialize(struct mxt_data *data)
>  		return error;
>  	}
>  
> +	/* Get info block CRC */
> +	error = mxt_get_info_block_crc(data);
> +	if (error) {
> +		dev_err(&client->dev, "Error %d reading info block CRC\n",
> +			error);
> +		return error;

You are leaking object table memory here.

By the way, why do we do this here, and not when we actually have config
file and is about to apply it?

Thanks.

-- 
Dmitry