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*
> ============================================================
>
>
