diff mbox series

[v3,09/11] Input: goodix - Add minimum firmware size check

Message ID 20200307121505.3707-9-hdegoede@redhat.com (mailing list archive)
State Mainlined
Commit 686e8a2489baf6dcbd87de07f37fef07a706dd41
Headers show
Series [v3,01/11] Input: goodix - Refactor IRQ pin GPIO accesses | expand

Commit Message

Hans de Goede March 7, 2020, 12:15 p.m. UTC 
Our goodix_check_cfg_* helpers do things like:

	int i, raw_cfg_len = cfg->size - 2;
	...
	if (check_sum != cfg->data[raw_cfg_len]) {

When cfg->size < 2, this will end up indexing the cfg->data array with
a negative value, which will not end well.

To fix this this commit adds a new GOODIX_CONFIG_MIN_LENGTH define and
adds a minimum size check for firmware-config files using this new define.

For consistency this commit also adds a new GOODIX_CONFIG_GT9X_LENGTH for
the length used for recent gt9xx and gt1xxx chips, instead of using
GOODIX_CONFIG_MAX_LENGTH for this, so that if other length defines get
added in the future it will be clear that the MIN and MAX defines should
contain the min and max values of all the other defines.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- New patch in v2 of this patch series
---
 drivers/input/touchscreen/goodix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Bastien Nocera March 9, 2020, 5:13 p.m. UTC | #1 
On Sat, 2020-03-07 at 13:15 +0100, Hans de Goede wrote:
> Our goodix_check_cfg_* helpers do things like:
> 
> 	int i, raw_cfg_len = cfg->size - 2;
> 	...
> 	if (check_sum != cfg->data[raw_cfg_len]) {
> 
> When cfg->size < 2, this will end up indexing the cfg->data array
> with
> a negative value, which will not end well.
> 
> To fix this this commit adds a new GOODIX_CONFIG_MIN_LENGTH define
> and
> adds a minimum size check for firmware-config files using this new
> define.
> 
> For consistency this commit also adds a new GOODIX_CONFIG_GT9X_LENGTH
> for
> the length used for recent gt9xx and gt1xxx chips, instead of using
> GOODIX_CONFIG_MAX_LENGTH for this, so that if other length defines
> get
> added in the future it will be clear that the MIN and MAX defines
> should
> contain the min and max values of all the other defines.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Bastien Nocera <hadess@hadess.net>
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index eb57c39dc55b..5227223e666b 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -39,9 +39,11 @@ 
 #define GOODIX_MAX_CONTACT_SIZE		9
 #define GOODIX_MAX_CONTACTS		10
 
-#define GOODIX_CONFIG_MAX_LENGTH	240
+#define GOODIX_CONFIG_MIN_LENGTH	186
 #define GOODIX_CONFIG_911_LENGTH	186
 #define GOODIX_CONFIG_967_LENGTH	228
+#define GOODIX_CONFIG_GT9X_LENGTH	240
+#define GOODIX_CONFIG_MAX_LENGTH	240
 
 /* Register defines */
 #define GOODIX_REG_COMMAND		0x8040
@@ -109,7 +111,7 @@  static void goodix_calc_cfg_checksum_16(struct goodix_ts_data *ts);
 
 static const struct goodix_chip_data gt1x_chip_data = {
 	.config_addr		= GOODIX_GT1X_REG_CONFIG_DATA,
-	.config_len		= GOODIX_CONFIG_MAX_LENGTH,
+	.config_len		= GOODIX_CONFIG_GT9X_LENGTH,
 	.check_config		= goodix_check_cfg_16,
 	.calc_config_checksum	= goodix_calc_cfg_checksum_16,
 };
@@ -130,7 +132,7 @@  static const struct goodix_chip_data gt967_chip_data = {
 
 static const struct goodix_chip_data gt9x_chip_data = {
 	.config_addr		= GOODIX_GT9X_REG_CONFIG_DATA,
-	.config_len		= GOODIX_CONFIG_MAX_LENGTH,
+	.config_len		= GOODIX_CONFIG_GT9X_LENGTH,
 	.check_config		= goodix_check_cfg_8,
 	.calc_config_checksum	= goodix_calc_cfg_checksum_8,
 };
@@ -509,7 +511,8 @@  static void goodix_calc_cfg_checksum_16(struct goodix_ts_data *ts)
 static int goodix_check_cfg(struct goodix_ts_data *ts,
 			    const struct firmware *cfg)
 {
-	if (cfg->size > GOODIX_CONFIG_MAX_LENGTH) {
+	if (cfg->size < GOODIX_CONFIG_MIN_LENGTH ||
+	    cfg->size > GOODIX_CONFIG_MAX_LENGTH) {
 		dev_err(&ts->client->dev,
 			"The length of the config fw is not correct");
 		return -EINVAL;