From adc474a3e2d313a47d101560737cb5d7bb5eb5ed Mon Sep 17 00:00:00 2001 From: Cheon-Woei Ng Date: Fri, 2 Mar 2012 17:49:36 -0800 Subject: [PATCH] Critical Klocwork issues for KERNEL domain BZ: 23909 Fix critical Klocwork issues in codes assigned to the KERNEL domain. Only Intel owned codes are fixed. Change-Id: Icfab0c529294580d60f9a0e6fc87b4f2e1be9b17 Signed-off-by: Cheon-Woei Ng Reviewed-on: http://android.intel.com:8080/37474 Reviewed-by: Monroy, German Reviewed-by: Kuppuswamy, Sathyanarayanan Reviewed-by: Yang, Fei Reviewed-by: buildbot Tested-by: buildbot --- arch/x86/kernel/apb_timer.c | 2 +- arch/x86/platform/intel-mid/board-blackbay.c | 8 +++++--- drivers/gpu/drm/drm_crtc.c | 5 +++-- drivers/idle/intel_idle.c | 14 ++++++++++---- drivers/platform/x86/intel_mid_umip.c | 2 +- drivers/platform/x86/intel_scu_ipc.c | 5 +++-- drivers/tty/serial/mfd.c | 2 +- drivers/tty/vt/keyboard.c | 6 ++++++ drivers/video/fbmem.c | 13 ++++++++++++- net/core/skbuff.c | 24 ++++++++++++++++++++---- 10 files changed, 62 insertions(+), 19 deletions(-) diff --git a/arch/x86/kernel/apb_timer.c b/arch/x86/kernel/apb_timer.c index d84f138..a10ff50 100644 --- a/arch/x86/kernel/apb_timer.c +++ b/arch/x86/kernel/apb_timer.c @@ -626,7 +626,7 @@ void __init apbt_time_init(void) } else printk(KERN_ERR "Failed to get timer for cpu %d\n", i); adev->count = 0; - sprintf(adev->name, "apbt%d", i); + snprintf(adev->name, sizeof(adev->name), "apbt%d", i); } #endif diff --git a/arch/x86/platform/intel-mid/board-blackbay.c b/arch/x86/platform/intel-mid/board-blackbay.c index 5e042be..28bdb28 100644 --- a/arch/x86/platform/intel-mid/board-blackbay.c +++ b/arch/x86/platform/intel-mid/board-blackbay.c @@ -256,7 +256,7 @@ static void __init *max7315_platform_data(void *info) char base_pin_name[SFI_NAME_LEN + 1]; char intr_pin_name[SFI_NAME_LEN + 1]; - if (nr == MAX7315_NUM) { + if (nr >= MAX7315_NUM) { pr_err("too many max7315s, we only support %d\n", MAX7315_NUM); return NULL; @@ -266,8 +266,10 @@ static void __init *max7315_platform_data(void *info) */ strcpy(i2c_info->type, "max7315"); if (nr++) { - sprintf(base_pin_name, "max7315_%d_base", nr); - sprintf(intr_pin_name, "max7315_%d_int", nr); + snprintf(base_pin_name, sizeof(base_pin_name), \ + "max7315_%d_base", nr); + snprintf(intr_pin_name, sizeof(intr_pin_name), \ + "max7315_%d_int", nr); } else { strcpy(base_pin_name, "max7315_base"); strcpy(intr_pin_name, "max7315_int"); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index de8b991..1707198 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2118,7 +2118,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, int flags, INIT_LIST_HEAD(&property->enum_blob_list); if (name) - strncpy(property->name, name, DRM_PROP_NAME_LEN); + strncpy(property->name, name, DRM_PROP_NAME_LEN-1); list_add_tail(&property->head, &dev->mode_config.property_list); return property; @@ -2139,7 +2139,8 @@ int drm_property_add_enum(struct drm_property *property, int index, if (!list_empty(&property->enum_blob_list)) { list_for_each_entry(prop_enum, &property->enum_blob_list, head) { if (prop_enum->value == value) { - strncpy(prop_enum->name, name, DRM_PROP_NAME_LEN); + strncpy(prop_enum->name, name, \ + DRM_PROP_NAME_LEN-1); prop_enum->name[DRM_PROP_NAME_LEN-1] = '\0'; return 0; } diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 8d24934..3d6da2a 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -890,10 +890,16 @@ static int intel_idle_cpuidle_devices_init(void) cpuidle_state_table[cstate].target_residency = get_target_residency(cstate); - dev->states[dev->state_count] = /* structure copy */ - cpuidle_state_table[cstate]; - - dev->state_count += 1; + if (dev->state_count >= \ + (sizeof(dev->states)/sizeof(dev->states[0]))) { + BUG(); + } else { + /* structure copy */ + dev->states[dev->state_count] = \ + cpuidle_state_table[cstate]; + + dev->state_count += 1; + } } dev->cpu = i; diff --git a/drivers/platform/x86/intel_mid_umip.c b/drivers/platform/x86/intel_mid_umip.c index cfa924f..2be9afd 100644 --- a/drivers/platform/x86/intel_mid_umip.c +++ b/drivers/platform/x86/intel_mid_umip.c @@ -102,7 +102,7 @@ ssize_t usb_host_enable_timeout_store(struct device *dev, string_length = strlen(buffer); if (string_length < MAX_USB_TIMEOUT_LEN) { - snprintf(timeout, string_length, "%s", buffer); + snprintf(timeout, sizeof(timeout), "%s", buffer); } else { pr_err("Invalid value written." "Check the Availabe values that can be used\n"); diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c index a45ae22..1e69b24 100644 --- a/drivers/platform/x86/intel_scu_ipc.c +++ b/drivers/platform/x86/intel_scu_ipc.c @@ -1490,7 +1490,7 @@ static void cur_err(const char *err_str) { if (err_str) { memset(err_buf, 0, sizeof(err_buf)); - strncpy(err_buf, err_str, strlen(err_str)); + strncpy(err_buf, err_str, sizeof(err_buf)-1); } } @@ -1960,7 +1960,8 @@ static ssize_t msic_debug_read(struct file *filp, char __user *buffer, msic_debug_reg_addr, ret); return -EFAULT; } - len = sprintf(buf, "msic[%3.3x]=0x%x\n", msic_debug_reg_addr, data); + len = snprintf(buf, sizeof(buf), "msic[%3.3x]=0x%x\n", \ + msic_debug_reg_addr, data); if (copy_to_user(buffer, buf, len)) return -EFAULT; *off = len; diff --git a/drivers/tty/serial/mfd.c b/drivers/tty/serial/mfd.c index d8ad4d6..98bf7e4 100644 --- a/drivers/tty/serial/mfd.c +++ b/drivers/tty/serial/mfd.c @@ -1754,7 +1754,7 @@ static void hsu_global_init(void) uport->port.membase = hsu->reg + HSU_PORT_REG_OFFSET + offset * HSU_PORT_REG_LENGTH; - sprintf(uport->name, "hsu_port%d", i); + snprintf(uport->name, sizeof(uport->name), "hsu_port%d", i); uport->port.fifosize = 64; uport->port.ops = &serial_hsu_pops; uport->port.line = i; diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index 7a8eb2b..a8cc190 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -410,6 +410,12 @@ void compute_shiftstate(void) if (val == KVAL(K_CAPSSHIFT)) val = KVAL(K_SHIFT); + /* fix a KW error */ + if (val >= (sizeof(shift_down) \ + / sizeof(shift_down[0]))) { + BUG(); + continue; + } shift_down[val]++; shift_state |= (1 << val); } diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 1b0a6ea..5a507d3 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -742,7 +742,12 @@ static struct fb_info *file_fb_info(struct file *file) { struct inode *inode = file->f_path.dentry->d_inode; int fbidx = iminor(inode); - struct fb_info *info = registered_fb[fbidx]; + struct fb_info *info = (struct fb_info *)NULL; + + if (fbidx >= (sizeof(registered_fb) / sizeof(registered_fb[0]))) + return (struct fb_info *)NULL; + + info = registered_fb[fbidx]; if (info != file->private_data) info = NULL; @@ -1636,6 +1641,11 @@ static int do_register_framebuffer(struct fb_info *fb_info) fb_var_to_videomode(&mode, &fb_info->var); fb_add_videomode(&mode, &fb_info->modelist); + + /* KW issue 115161 */ + if (i >= (sizeof(registered_fb) / sizeof(registered_fb[0]))) { + return -EINVAL; + } else { registered_fb[i] = fb_info; event.info = fb_info; @@ -1643,6 +1653,7 @@ static int do_register_framebuffer(struct fb_info *fb_info) return -ENODEV; fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event); unlock_fb_info(fb_info); + } return 0; } diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 1ae43b5..8b9d003 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1272,7 +1272,13 @@ pull_pages: put_page(skb_shinfo(skb)->frags[i].page); eat -= skb_shinfo(skb)->frags[i].size; } else { - skb_shinfo(skb)->frags[k] = skb_shinfo(skb)->frags[i]; + /* KW issue 116144 */ + if ((k < sizeof(skb_shinfo(skb)->frags) \ + / sizeof(skb_shinfo(skb)->frags[0])) && + (i < sizeof(skb_shinfo(skb)->frags) \ + / sizeof(skb_shinfo(skb)->frags[0]))) + skb_shinfo(skb)->frags[k] = \ + skb_shinfo(skb)->frags[i]; if (eat) { skb_shinfo(skb)->frags[k].page_offset += eat; skb_shinfo(skb)->frags[k].size -= eat; @@ -2045,7 +2051,10 @@ static inline void skb_split_no_header(struct sk_buff *skb, skb->data_len = len - pos; for (i = 0; i < nfrags; i++) { - int size = skb_shinfo(skb)->frags[i].size; + int size = 0; + if (i < sizeof(skb_shinfo(skb)->frags) \ + / sizeof(skb_shinfo(skb)->frags[0])) + skb_shinfo(skb)->frags[i].size; if (pos + size > len) { skb_shinfo(skb1)->frags[k] = skb_shinfo(skb)->frags[i]; @@ -2208,7 +2217,12 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen) /* Reposition in the original skb */ to = 0; - while (from < skb_shinfo(skb)->nr_frags) + /* KW issue 116146 */ + while (from < skb_shinfo(skb)->nr_frags && + (from < sizeof(skb_shinfo(skb)->frags) \ + / sizeof(skb_shinfo(skb)->frags[0])) && + (to < sizeof(skb_shinfo(skb)->frags) + / sizeof(skb_shinfo(skb)->frags[0]))) skb_shinfo(skb)->frags[to++] = skb_shinfo(skb)->frags[from++]; skb_shinfo(skb)->nr_frags = to; @@ -2689,7 +2703,9 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb) skbinfo->nr_frags = 0; frag = pinfo->frags + nr_frags; - frag2 = skbinfo->frags + i; + /* KW issue 116147 */ + if (i < sizeof(skbinfo->frags) / sizeof(skbinfo->frags[0])) + frag2 = skbinfo->frags + i; do { *--frag = *--frag2; } while (--i); -- 2.7.4