Webrev has been uploaded:
http://cr.opensolaris.org/~bholler/6819156/
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
