| Message ID | 20190829070530.21397-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | opal: add more bounds checking and zero pstates array | expand |
On 2019-08-29 9:05 a.m., Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Currently there is no bounds checking on the maximum number > of pstates, so we can potentially have an out-of-bounds read > on the pstates array. Fix this by checking and force setting > maximum bounds. Also zero the pstates array to ensure no > uninitialized pstates values are printed. Finally add some > missing spaces in some warning messages. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/opal/power_mgmt_info.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c > index ab90b9f2..1175278c 100644 > --- a/src/opal/power_mgmt_info.c > +++ b/src/opal/power_mgmt_info.c > @@ -152,6 +152,7 @@ static int pstate_limits_test(fwts_framework *fw) > return FWTS_ERROR; > } > > + memset(pstates, 0, sizeof(pstates)); > ret = fwts_dt_property_read_u32_arr(fw->fdt, offset, "ibm,pstate-ids", > pstates, &len); > if (ret != FWTS_OK) { > @@ -179,12 +180,12 @@ static int pstate_limits_test(fwts_framework *fw) > > if (proc_gen == proc_gen_p8 && nr_pstates > 128) > fwts_log_warning(fw, > - "More than 128 pstates found,nr_pstates = %d", > + "More than 128 pstates found, nr_pstates = %d", > nr_pstates); > > if (proc_gen == proc_gen_p9 && nr_pstates > 255) > fwts_log_warning(fw, > - "More than 255 pstates found,nr_pstates = %d", > + "More than 255 pstates found, nr_pstates = %d", > nr_pstates); > > if (len != nr_pstates) > @@ -192,6 +193,12 @@ static int pstate_limits_test(fwts_framework *fw) > "Expected %d pstates, found %d pstates", > nr_pstates, len); > > + /* Avoid reads outside of pstates[] */ > + if (nr_pstates > MAX_PSTATES) { > + nr_pstates = MAX_PSTATES; > + fwts_log_warning(fw, "Truncating to %d pstates\n", nr_pstates); > + } > + > for (i = 0; i < nr_pstates; i++) { > if (cmp_pstates(pstate_max, pstates[i]) < 0) { > fwts_log_warning(fw, "Invalid Pstate id %d " > Acked-by: Alex Hung <alex.hung@canonical.com>
On 8/29/19 3:05 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Currently there is no bounds checking on the maximum number > of pstates, so we can potentially have an out-of-bounds read > on the pstates array. Fix this by checking and force setting > maximum bounds. Also zero the pstates array to ensure no > uninitialized pstates values are printed. Finally add some > missing spaces in some warning messages. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/opal/power_mgmt_info.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c > index ab90b9f2..1175278c 100644 > --- a/src/opal/power_mgmt_info.c > +++ b/src/opal/power_mgmt_info.c > @@ -152,6 +152,7 @@ static int pstate_limits_test(fwts_framework *fw) > return FWTS_ERROR; > } > > + memset(pstates, 0, sizeof(pstates)); > ret = fwts_dt_property_read_u32_arr(fw->fdt, offset, "ibm,pstate-ids", > pstates, &len); > if (ret != FWTS_OK) { > @@ -179,12 +180,12 @@ static int pstate_limits_test(fwts_framework *fw) > > if (proc_gen == proc_gen_p8 && nr_pstates > 128) > fwts_log_warning(fw, > - "More than 128 pstates found,nr_pstates = %d", > + "More than 128 pstates found, nr_pstates = %d", > nr_pstates); > > if (proc_gen == proc_gen_p9 && nr_pstates > 255) > fwts_log_warning(fw, > - "More than 255 pstates found,nr_pstates = %d", > + "More than 255 pstates found, nr_pstates = %d", > nr_pstates); > > if (len != nr_pstates) > @@ -192,6 +193,12 @@ static int pstate_limits_test(fwts_framework *fw) > "Expected %d pstates, found %d pstates", > nr_pstates, len); > > + /* Avoid reads outside of pstates[] */ > + if (nr_pstates > MAX_PSTATES) { > + nr_pstates = MAX_PSTATES; > + fwts_log_warning(fw, "Truncating to %d pstates\n", nr_pstates); > + } > + > for (i = 0; i < nr_pstates; i++) { > if (cmp_pstates(pstate_max, pstates[i]) < 0) { > fwts_log_warning(fw, "Invalid Pstate id %d " Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c index ab90b9f2..1175278c 100644 --- a/src/opal/power_mgmt_info.c +++ b/src/opal/power_mgmt_info.c @@ -152,6 +152,7 @@ static int pstate_limits_test(fwts_framework *fw) return FWTS_ERROR; } + memset(pstates, 0, sizeof(pstates)); ret = fwts_dt_property_read_u32_arr(fw->fdt, offset, "ibm,pstate-ids", pstates, &len); if (ret != FWTS_OK) { @@ -179,12 +180,12 @@ static int pstate_limits_test(fwts_framework *fw) if (proc_gen == proc_gen_p8 && nr_pstates > 128) fwts_log_warning(fw, - "More than 128 pstates found,nr_pstates = %d", + "More than 128 pstates found, nr_pstates = %d", nr_pstates); if (proc_gen == proc_gen_p9 && nr_pstates > 255) fwts_log_warning(fw, - "More than 255 pstates found,nr_pstates = %d", + "More than 255 pstates found, nr_pstates = %d", nr_pstates); if (len != nr_pstates) @@ -192,6 +193,12 @@ static int pstate_limits_test(fwts_framework *fw) "Expected %d pstates, found %d pstates", nr_pstates, len); + /* Avoid reads outside of pstates[] */ + if (nr_pstates > MAX_PSTATES) { + nr_pstates = MAX_PSTATES; + fwts_log_warning(fw, "Truncating to %d pstates\n", nr_pstates); + } + for (i = 0; i < nr_pstates; i++) { if (cmp_pstates(pstate_max, pstates[i]) < 0) { fwts_log_warning(fw, "Invalid Pstate id %d "