Bill Holler wrote:
> Webrev has been uploaded:
>
> http://cr.opensolaris.org/~bholler/6819156/
Looks good.
>
> Regards,
> Bill
>
>
> On 03/18/09 19:28, Bill Holler wrote:
>> Hi tesla-dev,
>>
>> Please review this suggested fix for 6819156.
>> If we store less cstates than in the _CST object, then re-allocate the
>> ss_state buffer to the proper size to match the count. When the
>> buffer is later freed in cpu_acpi_free_cstate_data() it is of the
>> proper size to match the count.
>>
>> Regards,
>> Bill
>>
>> -bash-3.2$ more 6819156.patch
>> --- old/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:36 2009
>> +++ new/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:35 2009
>> @@ -688,6 +688,7 @@
>> ACPI_OBJECT *obj;
>> ACPI_INTEGER cnt;
>> cpu_acpi_cstate_t *cstate, *p;
>> + size_t orig_size;
>> int i, count;
>>
>> CPU_ACPI_OBJ_IS_NOT_CACHED(handle, CPU_ACPI_CST_CACHED);
>> @@ -721,8 +722,8 @@
>> }
>>
>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt;
>> - CPU_ACPI_CSTATES(handle) =
>> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt),
>> - KM_SLEEP);
>> + size = CPU_ACPI_CSTATES_SIZE(cnt);
>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(size, KM_SLEEP);
>> CPU_ACPI_BM_INFO(handle) = 0;
>> cstate = (cpu_acpi_cstate_t *)CPU_ACPI_CSTATES(handle);
>> p = cstate;
>> @@ -775,6 +776,7 @@
>> if (count < 2) {
>> cmn_err(CE_NOTE, "!cpu_acpi: _CST invalid count %d < 2",
>> count);
>> + kmem_free(CPU_ACPI_CSTATES(handle), size);
>> AcpiOsFree(abuf.Pointer);
>> return (-1);
>> }
>> @@ -782,12 +784,21 @@
>> if (cstate[0].cs_type != CPU_ACPI_C1) {
>> cmn_err(CE_NOTE, "!cpu_acpi: _CST first element type
>> not C1: "
>> "%d", (int)cstate->cs_type);
>> + kmem_free(CPU_ACPI_CSTATES(handle), size);
>> AcpiOsFree(abuf.Pointer);
>> return (-1);
>> }
>>
>> - if (count != cnt)
>> + if (count != cnt) {
>> + void *orig = CPU_ACPI_CSTATES(handle);
>> +
>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)count;
>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(
>> + CPU_ACPI_CSTATES_SIZE(count), KM_SLEEP);
>> + (void) memcpy(CPU_ACPI_CSTATES(handle), orig,
>> + CPU_ACPI_CSTATES_SIZE(count));
>> + kmem_free(orig, size);
>> + }
>>
>> AcpiOsFree(abuf.Pointer);
>> CPU_ACPI_OBJ_IS_CACHED(handle, CPU_ACPI_CST_CACHED);
>>
>>
>> On 03/18/09 19:04, Bill.Holler at Sun.COM wrote:
>>> Sun Confidential: Internal only
>>>
>>> *Synopsis*: cpu_acpi_free_cstate_data can free the wrong size
>>>
>>> CrPrint: http://bt2ws.central.sun.com/CrPrint?id=6819156
>>> Monaco: http://monaco.sfbay.sun.com/detail.jsf?cr=6819156
>>>
>>> CR 6819156 changed on Mar 19 2009 by bill.holler at sun.com
>>>
>>> === Field ============ === New Value ============= === Old Value
>>> =============
>>>
>>> Description New
>>> Note
>>> Evaluation New
>>> Note Interest
>>> List Michael.Corcoran at Sun.COM
>>> Introduced in Build
>>> snv_110 Introduced in
>>> Release solaris_nevada
>>> Status 6-Fix Understood
>>> 1-Dispatched ======================
>>> =========================== ===========================
>>>
>>> *Change Request ID*: 6819156
>>>
>>> *Synopsis*: cpu_acpi_free_cstate_data can free the wrong size
>>>
>>> Product: solaris
>>> Category: kernel
>>> Subcategory: sched
>>> Type: Defect
>>> Subtype: Functionality
>>> Status: 6-Fix Understood
>>> Substatus: Priority: 2-High
>>> Introduced In Release: solaris_nevada
>>> Introduced In Build: snv_110
>>> Responsible Manager: darrin.johnson at sun.com
>>> Responsible Engineer: bill.holler at sun.com
>>> Initial Evaluator: kernel-sched-ie at sun.com
>>> Keywords:
>>> === *Description*
>>> ============================================================
>>> cpu_acpi_free_cstate_data() frees memory kmem_zalloc'ed in
>>> cpu_acpi_cache_cst.
>>> The size to free is calculated with:
>>> kmem_free(CPU_ACPI_CSTATES(handle),
>>> CPU_ACPI_CSTATES_SIZE(CPU_ACPI_CSTATES_COUNT(handle)));
>>>
>>> Unfortunately the cstate count can be less than what was allocated for.
>>> cpu_acpi_cache_cst() sets cstate count to be that provided by _CST
>>> object:
>>> cnt = obj->Package.Elements[0].Integer.Value;
>>> ...
>>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt;
>>> CPU_ACPI_CSTATES(handle) =
>>> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt),
>>> KM_SLEEP);
>>> The function then eliminates duplicate cstates. Each discarded cstate
>>> causes the cstate count to be one less than what was allocated for.
>>>
>>>
>>> One possilble fix when cstate entries have been discarded in
>>> cpu_acpi_cache_cst() is to: reallocate a smaller buffer of just the
>>> right size
>>> to hold the csates as indicated in the cstate count.
>>>
>>> *** (#1 of 2): 2009-03-18 21:08:19 GMT+00:00 bill.holler at sun.com
>>>
>>> Here is an untested fix. re-allocate the ss_state buffer to the
>>> corrected size:
>>>
>>> -bash-3.2$ more 6819156.patch
>>> --- old/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:36
>>> 2009
>>> +++ new/usr/src/uts/i86pc/os/cpupm/cpu_acpi.c Wed Mar 18 18:57:35
>>> 2009
>>> @@ -688,6 +688,7 @@
>>> ACPI_OBJECT *obj;
>>> ACPI_INTEGER cnt;
>>> cpu_acpi_cstate_t *cstate, *p;
>>> + size_t orig_size;
>>> int i, count;
>>>
>>> CPU_ACPI_OBJ_IS_NOT_CACHED(handle, CPU_ACPI_CST_CACHED);
>>> @@ -721,8 +722,8 @@
>>> }
>>>
>>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)cnt;
>>> - CPU_ACPI_CSTATES(handle) =
>>> kmem_zalloc(CPU_ACPI_CSTATES_SIZE(cnt),
>>> - KM_SLEEP);
>>> + size = CPU_ACPI_CSTATES_SIZE(cnt);
>>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(size, KM_SLEEP);
>>> CPU_ACPI_BM_INFO(handle) = 0;
>>> cstate = (cpu_acpi_cstate_t *)CPU_ACPI_CSTATES(handle);
>>> p = cstate;
>>> @@ -775,6 +776,7 @@
>>> if (count < 2) {
>>> cmn_err(CE_NOTE, "!cpu_acpi: _CST invalid count %d <
>>> 2",
>>> count);
>>> + kmem_free(CPU_ACPI_CSTATES(handle), size);
>>> AcpiOsFree(abuf.Pointer);
>>> return (-1);
>>> }
>>> @@ -782,12 +784,21 @@
>>> if (cstate[0].cs_type != CPU_ACPI_C1) {
>>> cmn_err(CE_NOTE, "!cpu_acpi: _CST first element type
>>> not C1: "
>>> "%d", (int)cstate->cs_type);
>>> + kmem_free(CPU_ACPI_CSTATES(handle), size);
>>> AcpiOsFree(abuf.Pointer);
>>> return (-1);
>>> }
>>>
>>> - if (count != cnt)
>>> + if (count != cnt) {
>>> + void *orig = CPU_ACPI_CSTATES(handle);
>>> +
>>> CPU_ACPI_CSTATES_COUNT(handle) = (uint32_t)count;
>>> + CPU_ACPI_CSTATES(handle) = kmem_zalloc(
>>> + CPU_ACPI_CSTATES_SIZE(count), KM_SLEEP);
>>> + (void) memcpy(CPU_ACPI_CSTATES(handle), orig,
>>> + CPU_ACPI_CSTATES_SIZE(count));
>>> + kmem_free(orig, size);
>>> + }
>>>
>>> AcpiOsFree(abuf.Pointer);
>>> CPU_ACPI_OBJ_IS_CACHED(handle, CPU_ACPI_CST_CACHED);
>>>
>>> *** (#2 of 2): 2009-03-19 02:00:44 GMT+00:00 bill.holler at sun.com
>>>
>>>
>>> === *Public Comments*
>>> ========================================================
>>>
>>> === *Comments*
>>> ===============================================================
>>>
>>> === *Evaluation*
>>> =============================================================
>>> See public Decription.
>>>
>>> *** (#1 of 1): 2009-03-19 02:00:44 GMT+00:00 bill.holler at sun.com
>>>
>>>
>>> === *Suggested Fix*
>>> ==========================================================
>>>
>>> === *Workaround*
>>> =============================================================
>>>
>>> === *Justification*
>>> ==========================================================
>>> Priority changed from [] to [2-High]
>>> Machine can panic.
>>> bill.holler at sun.com 2009-03-18 21:08:19 GMT
>>>
>>> *** (#1 of 1): 2009-03-18 21:08:19 GMT+00:00 bill.holler at sun.com
>>>
>>>
>>> === *Additional Details*
>>> =====================================================
>>> Targeted Release: Commit To Fix In Build:
>>> Fixed In Build: Integrated In Build: Verified In
>>> Build: See Also: 6819124
>>> Duplicate of: Hooks:
>>> Hook1: Hook2: Hook3: Hook4:
>>> Hook5: Hook6: Interest List: Michael.Corcoran at Sun.COM,
>>> tesla-dev at sun.com
>>> Program Management: Root Cause: Is a Security Vulnerability?: No
>>> Fix Affects Documentation: No
>>> Fix Affects Localization: No
>>> Reported by:
>>> === *History*
>>> ================================================================
>>> Date Submitted: 2009-03-18 21:08:18 GMT+00:00
>>> Submitted By: bill.holler at sun.com
>>>
>>> Status Changed Date Updated Updated By
>>> 6-Fix Understood 2009-03-19 02:00:44 GMT+00:00
>>> bill.holler at sun.com
>>>
>>>
>>> === *Solution*
>>> ===============================================================
>>>
>>>
>>> === *Service Request*
>>> ========================================================
>>> ID: 1-527843205
>>> Customer:
>>> Account Name: sun
>>> Customer Contact: Customer Contact Role:
>>> F-Functional Test
>>> Customer Contact Type: I-Internal (SMI) Customer
>>> Impact: Critical
>>> Functionality: Secondary
>>> Severity: 2
>>> Synopsis: Product Name: solaris
>>> Product Release: solaris_nevada
>>> Product Build: snv_110
>>> Operating System: snv_110
>>> Hardware: intel
>>> Reference Number: Sun Contact: bill.holler at sun.com
>>> Status: Open
>>> Source: BugTraq2
>>> Reproducible: Submitted By: bill.holler at sun.com
>>> Submitted Date: 2009-03-18 21:08:19 GMT+00:00
>>> Description:
>>>
>>> === *Activity*
>>> ===============================================================
>>>
>>>
>>> === *Multiple Release (MR) Cluster* - 0
>>> ======================================
>>>
>>>
>>> === *Escalations*
>>> ============================================================
>>>
>>>
>> _______________________________________________
>> tesla-dev mailing list
>> tesla-dev at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>
> _______________________________________________
> tesla-dev mailing list
> tesla-dev at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
