Bug 188901 - Function cpuidle_add_state_sysfs() may return improper value when the call to kzalloc() fails
Summary: Function cpuidle_add_state_sysfs() may return improper value when the call to...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:07 UTC by bianpan
Modified: 2017-05-12 00:08 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (1.32 KB, patch)
2017-05-12 00:08 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 11:07:37 UTC
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!
Comment 1 bianpan 2017-05-12 00:08:04 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.