From 0fc52b6f561980193afbca3add1e4c3ecdb0ecaf Mon Sep 17 00:00:00 2001 From: Evoke Zhang Date: Mon, 4 Sep 2017 16:41:42 +0800 Subject: [PATCH] lcd_extern: driver defect clean up PD#150081: driver defect clean up: #20 #38 #87 #468 #469 #470 #471 #613 #668 Change-Id: Ifa0756e25088599571761b33efaffb6c2c898dfc Signed-off-by: Evoke Zhang --- .../media/vout/lcd/lcd_extern/ext_default.c | 33 ++++-- .../amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c | 24 ++-- .../amlogic/media/vout/lcd/lcd_extern/lcd_extern.c | 130 ++++++++++++--------- 3 files changed, 113 insertions(+), 74 deletions(-) diff --git a/drivers/amlogic/media/vout/lcd/lcd_extern/ext_default.c b/drivers/amlogic/media/vout/lcd/lcd_extern/ext_default.c index 5ef6b507..c8b2ed3 100644 --- a/drivers/amlogic/media/vout/lcd/lcd_extern/ext_default.c +++ b/drivers/amlogic/media/vout/lcd/lcd_extern/ext_default.c @@ -181,7 +181,7 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table, switch (ext_config->type) { case LCD_EXTERN_I2C: - while (i <= max_len) { + while ((i + 2) < max_len) { type = init_table[i]; if (type == LCD_EXTERN_INIT_END) break; @@ -191,6 +191,9 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table, init_table[i], init_table[i+1]); } cmd_size = init_table[i+1]; + if ((i + 2 + cmd_size) > max_len) + break; + if (type == LCD_EXTERN_INIT_NONE) { if (cmd_size < 1) { EXTERR("step %d: invalid cmd_size %d\n", @@ -238,7 +241,7 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table, } break; case LCD_EXTERN_SPI: - while (i <= max_len) { + while ((i + 2) < max_len) { type = init_table[i]; if (type == LCD_EXTERN_INIT_END) break; @@ -248,6 +251,9 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table, init_table[i], init_table[i+1]); } cmd_size = init_table[i+1]; + if ((i + 2 + cmd_size) > max_len) + break; + if (type == LCD_EXTERN_INIT_NONE) { if (cmd_size < 1) { EXTERR("step %d: invalid cmd_size %d\n", @@ -311,7 +317,7 @@ static int lcd_extern_power_cmd_fixed_size(unsigned char *init_table, int flag) cmd_size = ext_config->cmd_size; switch (ext_config->type) { case LCD_EXTERN_I2C: - while (i <= max_len) { + while ((i + cmd_size) <= max_len) { type = init_table[i]; if (type == LCD_EXTERN_INIT_END) break; @@ -346,7 +352,7 @@ static int lcd_extern_power_cmd_fixed_size(unsigned char *init_table, int flag) } break; case LCD_EXTERN_SPI: - while (i <= max_len) { + while ((i + cmd_size) <= max_len) { type = init_table[i]; if (type == LCD_EXTERN_INIT_END) break; @@ -494,10 +500,12 @@ static int aml_default_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id) { - if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { EXTERR("%s: functionality check failed\n", __func__); - else - aml_default_i2c_client = client; + return -ENODEV; + } + + aml_default_i2c_client = client; EXTPR("%s OK\n", __func__); return 0; @@ -546,6 +554,7 @@ int aml_lcd_extern_default_probe(struct aml_lcd_extern_driver_s *ext_drv) } strncpy(i2c_info.type, ext_drv->config.name, I2C_NAME_SIZE); + i2c_info.type[I2C_NAME_SIZE-1] = '\0'; i2c_info.addr = ext_drv->config.i2c_addr; i2c_info.platform_data = &ext_drv->config; i2c_info.flags = 0; @@ -558,11 +567,11 @@ int aml_lcd_extern_default_probe(struct aml_lcd_extern_driver_s *ext_drv) if (!i2c_client) { EXTERR("%s failed to new i2c device\n", ext_drv->config.name); - } else { - if (lcd_debug_print_flag) { - EXTPR("%s new i2c device succeed\n", - ext_drv->config.name); - } + return -1; + } + if (lcd_debug_print_flag) { + EXTPR("%s new i2c device succeed\n", + ext_drv->config.name); } if (!aml_default_i2c_client) { diff --git a/drivers/amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c b/drivers/amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c index b8f6648..94df58a 100644 --- a/drivers/amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c +++ b/drivers/amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c @@ -136,7 +136,7 @@ static int lcd_extern_power_cmd(unsigned char *init_table, int flag) else max_len = LCD_EXTERN_INIT_OFF_MAX; - while (i <= max_len) { + while ((i + cmd_size) <= max_len) { if (init_table[i] == LCD_EXTERN_INIT_END) break; if (lcd_debug_print_flag) { @@ -216,7 +216,10 @@ static int lcd_extern_driver_update(struct aml_lcd_extern_driver_s *ext_drv) return 0; } -/* debug function */ +/* ********************************************************* + * debug function + * ********************************************************* + */ static const char *lcd_extern_debug_usage_str = { "Usage:\n" " echo <> ... <> > write ; T5800Q i2c command write, 7 parameters without address\n" @@ -301,10 +304,12 @@ static int aml_T5800Q_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id) { - if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { EXTERR("%s: functionality check failed\n", __func__); - else - aml_T5800Q_i2c_client = client; + return -ENODEV; + } + + aml_T5800Q_i2c_client = client; EXTPR("%s OK\n", __func__); return 0; @@ -348,6 +353,7 @@ int aml_lcd_extern_i2c_T5800Q_probe(struct aml_lcd_extern_driver_s *ext_drv) } strncpy(i2c_info.type, ext_drv->config.name, I2C_NAME_SIZE); + i2c_info.type[I2C_NAME_SIZE-1] = '\0'; i2c_info.addr = ext_drv->config.i2c_addr; i2c_info.platform_data = &ext_drv->config; i2c_info.flags = 0; @@ -359,12 +365,10 @@ int aml_lcd_extern_i2c_T5800Q_probe(struct aml_lcd_extern_driver_s *ext_drv) i2c_client = i2c_new_device(adapter, &i2c_info); if (!i2c_client) { EXTERR("%s failed to new i2c device\n", ext_drv->config.name); - } else { - if (lcd_debug_print_flag) { - EXTPR("%s new i2c device succeed\n", - ext_drv->config.name); - } + return -1; } + if (lcd_debug_print_flag) + EXTPR("%s new i2c device succeed\n", ext_drv->config.name); if (!aml_T5800Q_i2c_client) { ret = i2c_add_driver(&aml_T5800Q_i2c_driver); diff --git a/drivers/amlogic/media/vout/lcd/lcd_extern/lcd_extern.c b/drivers/amlogic/media/vout/lcd/lcd_extern/lcd_extern.c index 2490664..f768c7cc 100644 --- a/drivers/amlogic/media/vout/lcd/lcd_extern/lcd_extern.c +++ b/drivers/amlogic/media/vout/lcd/lcd_extern/lcd_extern.c @@ -249,7 +249,7 @@ static int lcd_extern_init_table_dynamic_size_load_dts( } i = 0; - while (i < max_len) { + while ((i + 2) < max_len) { /* type & cmd_size detect */ /* step1: type */ ret = of_property_read_u32_index(of_node, propname, i, &val); if (ret) { @@ -277,6 +277,8 @@ static int lcd_extern_init_table_dynamic_size_load_dts( i += 2; continue; } + if ((i + 2 + cmd_size) > max_len) + break; /* step3: data */ for (j = 0; j < cmd_size; j++) { ret = of_property_read_u32_index( @@ -323,7 +325,7 @@ static int lcd_extern_init_table_fixed_size_load_dts( } i = 0; - while (i < max_len) { + while ((i + cmd_size) <= max_len) { /* group detect */ for (j = 0; j < cmd_size; j++) { ret = of_property_read_u32_index( of_node, propname, (i+j), &val); @@ -362,29 +364,32 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, extconf->table_init_loaded = 0; ret = of_property_read_u32(of_node, "index", &val); - if (ret) + if (ret) { EXTERR("get index failed, exit\n"); - else - extconf->index = (unsigned char)val; + return -1; + } + extconf->index = (unsigned char)val; ret = of_property_read_string(of_node, "status", &str); if (ret) { EXTERR("get index %d status failed\n", extconf->index); - } else { - if (lcd_debug_print_flag) - EXTPR("index %d status = %s\n", extconf->index, str); - if (strncmp(str, "okay", 2) == 0) - extconf->status = 1; - else - return -1; + return -1; } + if (lcd_debug_print_flag) + EXTPR("index %d status = %s\n", extconf->index, str); + if (strncmp(str, "okay", 2) == 0) + extconf->status = 1; + else + return -1; ret = of_property_read_string(of_node, "extern_name", &str); if (ret) { str = "none"; EXTERR("get extern_name failed\n"); } - strcpy(extconf->name, str); + strncpy(extconf->name, str, LCD_EXTERN_NAME_LEN_MAX); + /* ensure string ending */ + extconf->name[LCD_EXTERN_NAME_LEN_MAX-1] = '\0'; EXTPR("load config: %s[%d]\n", extconf->name, extconf->index); ret = of_property_read_u32(of_node, "type", &extconf->type); @@ -401,16 +406,16 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, EXTERR("get %s i2c_address failed, exit\n", extconf->name); extconf->i2c_addr = 0xff; - } else { - extconf->i2c_addr = (unsigned char)val; + return -1; } + extconf->i2c_addr = (unsigned char)val; if (lcd_debug_print_flag) { EXTPR("%s i2c_address=0x%02x\n", extconf->name, extconf->i2c_addr); } ret = of_property_read_u32(of_node, "i2c_second_address", &val); if (ret) { - EXTERR("get %s i2c_second_address failed, exit\n", + EXTPR("get %s i2c_second_address failed\n", extconf->name); extconf->i2c_addr2 = 0xff; } else { @@ -425,9 +430,9 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, if (ret) { EXTERR("get %s i2c_bus failed, exit\n", extconf->name); extconf->i2c_bus = AML_I2C_MASTER_A; - } else { - extconf->i2c_bus = lcd_extern_get_i2c_bus_str(str); + return -1; } + extconf->i2c_bus = lcd_extern_get_i2c_bus_str(str); if (lcd_debug_print_flag) { EXTPR("%s i2c_bus=%s[%d]\n", extconf->name, str, extconf->i2c_bus); @@ -435,7 +440,12 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, ret = of_property_read_u32(of_node, "cmd_size", &val); if (ret) { - EXTERR("get %s cmd_size failed\n", extconf->name); + if (extconf->index == 0) { + EXTERR("get %s cmd_size failed\n", + extconf->name); + } else { + EXTPR("%s no cmd_size\n", extconf->name); + } extconf->cmd_size = 0; } else { extconf->cmd_size = (unsigned char)val; @@ -445,7 +455,10 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, extconf->name, extconf->cmd_size); } if (extconf->cmd_size <= 1) { - EXTERR("cmd_size %d is invalid\n", extconf->cmd_size); + if (extconf->index == 0) { + EXTERR("cmd_size %d is invalid\n", + extconf->cmd_size); + } break; } @@ -473,40 +486,34 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, EXTERR("get %s gpio_spi_cs failed, exit\n", extconf->name); extconf->spi_gpio_cs = LCD_EXTERN_GPIO_NUM_MAX; - } else { - extconf->spi_gpio_cs = val; - lcd_extern_gpio_register(val); - if (lcd_debug_print_flag) { - EXTPR("spi_gpio_cs: %d\n", - extconf->spi_gpio_cs); - } + return -1; } + extconf->spi_gpio_cs = val; + lcd_extern_gpio_register(val); + if (lcd_debug_print_flag) + EXTPR("spi_gpio_cs: %d\n", extconf->spi_gpio_cs); ret = of_property_read_u32(of_node, "gpio_spi_clk", &val); if (ret) { EXTERR("get %s gpio_spi_clk failed, exit\n", extconf->name); extconf->spi_gpio_clk = LCD_EXTERN_GPIO_NUM_MAX; - } else { - extconf->spi_gpio_clk = val; - lcd_extern_gpio_register(val); - if (lcd_debug_print_flag) { - EXTPR("spi_gpio_clk: %d\n", - extconf->spi_gpio_clk); - } + return -1; } + extconf->spi_gpio_clk = val; + lcd_extern_gpio_register(val); + if (lcd_debug_print_flag) + EXTPR("spi_gpio_clk: %d\n", extconf->spi_gpio_clk); ret = of_property_read_u32(of_node, "gpio_spi_data", &val); if (ret) { EXTERR("get %s gpio_spi_data failed, exit\n", extconf->name); extconf->spi_gpio_data = LCD_EXTERN_GPIO_NUM_MAX; - } else { - extconf->spi_gpio_data = val; - lcd_extern_gpio_register(val); - if (lcd_debug_print_flag) { - EXTPR("spi_gpio_data: %d\n", - extconf->spi_gpio_data); - } + return -1; } + extconf->spi_gpio_data = val; + lcd_extern_gpio_register(val); + if (lcd_debug_print_flag) + EXTPR("spi_gpio_data: %d\n", extconf->spi_gpio_data); ret = of_property_read_u32(of_node, "spi_clk_freq", &val); if (ret) { EXTERR("get %s spi_clk_freq failed, default to %dHz\n", @@ -533,7 +540,12 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, } ret = of_property_read_u32(of_node, "cmd_size", &val); if (ret) { - EXTERR("get %s cmd_size failed\n", extconf->name); + if (extconf->index == 0) { + EXTERR("get %s cmd_size failed\n", + extconf->name); + } else { + EXTPR("%s no cmd_size\n", extconf->name); + } extconf->cmd_size = 0; } else { extconf->cmd_size = (unsigned char)val; @@ -543,7 +555,10 @@ static int lcd_extern_get_config_dts(struct device_node *of_node, extconf->name, extconf->cmd_size); } if (extconf->cmd_size <= 1) { - EXTERR("cmd_size %d is invalid\n", extconf->cmd_size); + if (extconf->index == 0) { + EXTERR("cmd_size %d is invalid\n", + extconf->cmd_size); + } break; } @@ -619,7 +634,7 @@ static int lcd_extern_init_table_dynamic_size_load_unifykey( } i = 0; - while (i < max_len) { + while ((i + 2) < max_len) { /* type & cmd_size detect */ /* step1: type */ len += 1; ret = lcd_unifykey_len_check(key_len, len); @@ -650,6 +665,8 @@ static int lcd_extern_init_table_dynamic_size_load_unifykey( i += 2; continue; } + if ((i + 2 + cmd_size) > max_len) + break; /* step3: data */ len += cmd_size; @@ -703,7 +720,7 @@ static int lcd_extern_init_table_fixed_size_load_unifykey( } i = 0; - while (i < max_len) { + while ((i + cmd_size) <= max_len) { /* group detect */ len += cmd_size; ret = lcd_unifykey_len_check(key_len, len); if (ret) { @@ -778,9 +795,10 @@ static int lcd_extern_get_config_unifykey(struct lcd_extern_config_s *extconf) /* basic: 33byte */ p = para + LCD_UKEY_HEAD_SIZE; - *(p + LCD_UKEY_EXT_NAME - 1) = '\0'; /* ensure string ending */ str = (const char *)p; - strcpy(extconf->name, str); + strncpy(extconf->name, str, LCD_EXTERN_NAME_LEN_MAX); + /* ensure string ending */ + extconf->name[LCD_EXTERN_NAME_LEN_MAX-1] = '\0'; p += LCD_UKEY_EXT_NAME; extconf->index = *p; p += LCD_UKEY_EXT_INDEX; @@ -1159,6 +1177,10 @@ static int lcd_extern_add_driver(struct lcd_extern_config_s *extconf) return 0; } +/* ********************************************************* + * debug function + * ********************************************************* + */ #define EXT_LEN_MAX 200 static void lcd_extern_init_table_dynamic_size_print( struct lcd_extern_config_s *econf, int flag) @@ -1315,7 +1337,7 @@ static ssize_t lcd_extern_debug_help(struct class *class, static ssize_t lcd_extern_info_dump(struct class *class, struct class_attribute *attr, const char *buf, size_t count) { - unsigned int ret; + unsigned int ret = 0; int i, index; struct aml_lcd_extern_driver_s *ext_drv; @@ -1323,8 +1345,12 @@ static ssize_t lcd_extern_info_dump(struct class *class, switch (buf[0]) { case 'i': ret = sscanf(buf, "index %d", &index); - ext_drv = aml_lcd_extern_get_driver(index); - lcd_extern_config_dump(ext_drv); + if (ret == 1) { + ext_drv = aml_lcd_extern_get_driver(index); + lcd_extern_config_dump(ext_drv); + } else { + EXTERR("invalid parameters\n"); + } break; case 'a': for (i = 0; i < lcd_ext_driver_num; i++) @@ -1388,9 +1414,9 @@ static ssize_t lcd_extern_debug_test_store(struct class *class, static struct class_attribute lcd_extern_class_attrs[] = { __ATTR(info, 0644, lcd_extern_debug_help, lcd_extern_info_dump), - __ATTR(key_valid, 0644, + __ATTR(key_valid, 0444, lcd_extern_debug_key_valid_show, NULL), - __ATTR(config_load, 0644, + __ATTR(config_load, 0444, lcd_extern_debug_config_load_show, NULL), __ATTR(test, 0644, lcd_extern_debug_test_show, lcd_extern_debug_test_store), -- 2.7.4