Bug 188901
| Summary: | Function cpuidle_add_state_sysfs() may return improper value when the call to kzalloc() fails | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | bianpan (bianpan2010) |
| Component: | Other | Assignee: | drivers_other |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | linux-4.9-rc6 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: | The patch fixes the bug | ||
Created attachment 256433 [details]
The patch fixes the bug
The patch has been merged into the latest version of the Linux kernel. So I will close the bug.
|
Function kzalloc() returns a NULL pointer if there is no enough memory. In the function cpuidle_add_state_sysfs() defined in file drivers/cpuidle/sysfs.c, kzalloc() is called and its return value is checked against NULL (at line 406). If the return value is NULL, the control flow jumps to label "error_state", frees allocated memory and returns the value of variable ret. Because variable ret is re-assigned in the loop body, its value will 0 during the second or after repeats of the loop. If the call to kzalloc() fails then, 0 (indicates success) may be returned to the callers. Maybe it is better to explicitly assign "-ENOMEM" to ret when kzalloc() fails. Codes related to this bug are shown as below. cpuidle_add_state_sysfs @@ drivers/cpuidle/sysfs.c 392 /** 393 * cpuidle_add_state_sysfs - adds cpuidle states sysfs attributes 394 * @device: the target device 395 */ 396 static int cpuidle_add_state_sysfs(struct cpuidle_device *device) 397 { 398 int i, ret = -ENOMEM; 399 struct cpuidle_state_kobj *kobj; 400 struct cpuidle_device_kobj *kdev = device->kobj_dev; 401 struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device); 402 403 /* state statistics */ 404 for (i = 0; i < drv->state_count; i++) { 405 kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL); 406 if (!kobj) // insert "ret = -ENOMEM;" here? 407 goto error_state; 408 kobj->state = &drv->states[i]; 409 kobj->state_usage = &device->states_usage[i]; 410 init_completion(&kobj->kobj_unregister); 411 412 ret = kobject_init_and_add(&kobj->kobj, &ktype_state_cpuidle, 413 &kdev->kobj, "state%d", i); 414 if (ret) { 415 kfree(kobj); 416 goto error_state; 417 } 418 kobject_uevent(&kobj->kobj, KOBJ_ADD); 419 device->kobjs[i] = kobj; 420 } 421 422 return 0; 423 424 error_state: 425 for (i = i - 1; i >= 0; i--) 426 cpuidle_free_state_kobj(device, i); 427 return ret; 428 } Thanks very much!