On 03/18/09 19:56, Li, Aubrey wrote: > Am I reading the right patch? >
No. There are two different bugs. :-( This is 6819156 not 6818652. This bug 6819156 is being marked as a release stopper. Regards, Bill > No need to replace AcpiEvaluateObject by AcpiEvaluateObjectTyped? > > Thanks, > -Aubrey > > > Bill.Holler wrote: > > >> 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 >>> >> _______________________________________________ >> tesla-dev mailing list >> tesla-dev at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/tesla-dev >> > >
