[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging-4.19] x86/pmstat: Check size of PMSTAT_get_pxstat buffers



commit 81ffb56a23c66f323e062c417dedfd66bb0aa4fd
Author:     Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
AuthorDate: Thu Jun 26 08:48:35 2025 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Jun 26 08:48:35 2025 +0200

    x86/pmstat: Check size of PMSTAT_get_pxstat buffers
    
    Check that the total number of states passed in and hence the size of
    buffers is sufficient to avoid writing more than the caller has
    allocated.
    
    The interface is not explicit about whether getpx.total is expected to
    be set by the caller in this case but since it is always set in
    libxenctrl it seems reasonable to check it and make it explicit.
    
    Fixes: c06a7db0c547 ("X86 and IA64: Update cpufreq statistic logic for 
supporting both x86 and ia64")
    Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    
    # Commit fb16c7411d6e1278155c144fd3310a12f2efbf5e
    # Date 2025-06-18 09:25:09 +0200
    # Author Jan Beulich <jbeulich@xxxxxxxx>
    # Committer Jan Beulich <jbeulich@xxxxxxxx>
    x86/pmstat: correct PMSTAT_get_pxstat buffer size checking
    
    min(pmpt->perf.state_count, op->u.getpx.total) == op->u.getpx.total can
    be expressed differently as pmpt->perf.state_count >= op->u.getpx.total.
    Copying when the two are equal is fine; (partial) copying when the state
    count is larger than the number of array elements that a buffer was
    allocated to hold is what - as per the comment - we mean to avoid. Drop
    the use of min() again, but retain its effect for the subsequent copying
    from pxpt->u.pt.
    
    Fixes: aa70996a6896 ("x86/pmstat: Check size of PMSTAT_get_pxstat buffers")
    Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
    master commit: aa70996a6896dfc9de60f60540098b7d3ac3fb4f
    master date: 2025-06-11 11:05:42 +0200
    master commit: fb16c7411d6e1278155c144fd3310a12f2efbf5e
    master date: 2025-06-18 09:25:09 +0200
---
 xen/drivers/acpi/pmstat.c   |  8 +++++++-
 xen/include/public/sysctl.h | 15 +++++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index f607bc110f..1b18ee378d 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -103,8 +103,14 @@ int do_get_pm_info(struct xen_sysctl_get_pmstat *op)
 
         cpufreq_residency_update(op->cpuid, pxpt->u.cur);
 
+        /*
+         * Avoid partial copying of 2-D array, whereas partial copying of a
+         * simple vector (further down) is deemed okay.
+         */
         ct = pmpt->perf.state_count;
-        if ( copy_to_guest(op->u.getpx.trans_pt, pxpt->u.trans_pt, ct*ct) )
+        if ( ct > op->u.getpx.total )
+            ct = op->u.getpx.total;
+        else if ( copy_to_guest(op->u.getpx.trans_pt, pxpt->u.trans_pt, ct * 
ct) )
         {
             spin_unlock(cpufreq_statistic_lock);
             ret = -EFAULT;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 3a6e7d48f0..a0ddd1dbf5 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -205,11 +205,22 @@ typedef struct pm_px_val pm_px_val_t;
 DEFINE_XEN_GUEST_HANDLE(pm_px_val_t);
 
 struct pm_px_stat {
-    uint8_t total;        /* total Px states */
+    /*
+     * IN: Number of elements in pt, number of rows/columns in trans_pt
+     *     (PMSTAT_get_pxstat)
+     * OUT: total Px states (PMSTAT_get_max_px, PMSTAT_get_pxstat)
+     */
+    uint8_t total;
     uint8_t usable;       /* usable Px states */
     uint8_t last;         /* last Px state */
     uint8_t cur;          /* current Px state */
-    XEN_GUEST_HANDLE_64(uint64) trans_pt;   /* Px transition table */
+    /*
+     * OUT: Px transition table. This should have total * total elements.
+     *      As it is a 2-D array, this will not be copied if input total is
+     *      less than output total. (PMSTAT_get_pxstat)
+     */
+    XEN_GUEST_HANDLE_64(uint64) trans_pt;
+    /* OUT: This should have total elements (PMSTAT_get_pxstat) */
     XEN_GUEST_HANDLE_64(pm_px_val_t) pt;
 };
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.19



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.