| Message ID | 20190819205720.24457-3-mike.leach@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: etm4x: Fixes and updates for sysfs API | expand |
On Mon, Aug 19, 2019 at 09:57:14PM +0100, Mike Leach wrote: > A number of issues are fixed relating to sysfs input validation:- > > 1) bb_ctrl_store() - incorrect compare of bit select field to absolute > value. Reworked per ETMv4 specification. > 2) seq_event_store() - incorrect mask value - register has two > event values. > 3) cyc_threshold_store() - must mask with max before checking min > otherwise wrapped values can set illegal value below min. > 4) res_ctrl_store() - update to mask off all res0 bits. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > --- > .../coresight/coresight-etm4x-sysfs.c | 21 ++++++++++++------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index b6984be0c515..fa1d6a938f6c 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -652,10 +652,13 @@ static ssize_t cyc_threshold_store(struct device *dev, > > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > + > + /* mask off max threshold before checking min value */ > + val &= ETM_CYC_THRESHOLD_MASK; > if (val < drvdata->ccitmin) > return -EINVAL; > > - config->ccctlr = val & ETM_CYC_THRESHOLD_MASK; > + config->ccctlr = val; > return size; > } > static DEVICE_ATTR_RW(cyc_threshold); > @@ -686,14 +689,16 @@ static ssize_t bb_ctrl_store(struct device *dev, > return -EINVAL; > if (!drvdata->nr_addr_cmp) > return -EINVAL; > + > /* > - * Bit[7:0] selects which address range comparator is used for > - * branch broadcast control. > + * Bit[8] controls include(1) / exclude(0), bits[0-7] select > + * individual range comparators. If include then at least 1 > + * range must be selected. s/"must be selected"/"must be selected" > */ > - if (BMVAL(val, 0, 7) > drvdata->nr_addr_cmp) > + if ((val & BIT(8)) && (BMVAL(val, 0, 7) == 0)) > return -EINVAL; > > - config->bb_ctrl = val; > + config->bb_ctrl = val & GENMASK(8, 0); > return size; > } > static DEVICE_ATTR_RW(bb_ctrl); > @@ -1324,8 +1329,8 @@ static ssize_t seq_event_store(struct device *dev, > > spin_lock(&drvdata->spinlock); > idx = config->seq_idx; > - /* RST, bits[7:0] */ > - config->seq_ctrl[idx] = val & 0xFF; > + /* Seq control has two masks B[15:5] F[7:0] */ > + config->seq_ctrl[idx] = val & 0xFFFF; > spin_unlock(&drvdata->spinlock); > return size; > } > @@ -1580,7 +1585,7 @@ static ssize_t res_ctrl_store(struct device *dev, > if (idx % 2 != 0) > /* PAIRINV, bit[21] */ > val &= ~BIT(21); > - config->res_ctrl[idx] = val; > + config->res_ctrl[idx] = val & GENMASK(21, 0); > spin_unlock(&drvdata->spinlock); > return size; > } With the above: Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > -- > 2.17.1 >
On Mon, Aug 19, 2019 at 09:57:14PM +0100, Mike Leach wrote: > A number of issues are fixed relating to sysfs input validation:- > > 1) bb_ctrl_store() - incorrect compare of bit select field to absolute > value. Reworked per ETMv4 specification. > 2) seq_event_store() - incorrect mask value - register has two > event values. > 3) cyc_threshold_store() - must mask with max before checking min > otherwise wrapped values can set illegal value below min. > 4) res_ctrl_store() - update to mask off all res0 bits. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > --- > .../coresight/coresight-etm4x-sysfs.c | 21 ++++++++++++------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index b6984be0c515..fa1d6a938f6c 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -652,10 +652,13 @@ static ssize_t cyc_threshold_store(struct device *dev, > > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > + > + /* mask off max threshold before checking min value */ > + val &= ETM_CYC_THRESHOLD_MASK; > if (val < drvdata->ccitmin) > return -EINVAL; > > - config->ccctlr = val & ETM_CYC_THRESHOLD_MASK; > + config->ccctlr = val; > return size; > } > static DEVICE_ATTR_RW(cyc_threshold); > @@ -686,14 +689,16 @@ static ssize_t bb_ctrl_store(struct device *dev, > return -EINVAL; > if (!drvdata->nr_addr_cmp) > return -EINVAL; > + > /* > - * Bit[7:0] selects which address range comparator is used for > - * branch broadcast control. > + * Bit[8] controls include(1) / exclude(0), bits[0-7] select > + * individual range comparators. If include then at least 1 > + * range must be selected. > */ > - if (BMVAL(val, 0, 7) > drvdata->nr_addr_cmp) > + if ((val & BIT(8)) && (BMVAL(val, 0, 7) == 0)) > return -EINVAL; > > - config->bb_ctrl = val; > + config->bb_ctrl = val & GENMASK(8, 0); > return size; > } > static DEVICE_ATTR_RW(bb_ctrl); > @@ -1324,8 +1329,8 @@ static ssize_t seq_event_store(struct device *dev, > > spin_lock(&drvdata->spinlock); > idx = config->seq_idx; > - /* RST, bits[7:0] */ > - config->seq_ctrl[idx] = val & 0xFF; > + /* Seq control has two masks B[15:5] F[7:0] */ s/B[15:5]/B[15:8] With this change and Mathieu mentioned redundant space: Reviewed-by: Leo Yan <leo.yan@linaro.org> > + config->seq_ctrl[idx] = val & 0xFFFF; > spin_unlock(&drvdata->spinlock); > return size; > } > @@ -1580,7 +1585,7 @@ static ssize_t res_ctrl_store(struct device *dev, > if (idx % 2 != 0) > /* PAIRINV, bit[21] */ > val &= ~BIT(21); > - config->res_ctrl[idx] = val; > + config->res_ctrl[idx] = val & GENMASK(21, 0); > spin_unlock(&drvdata->spinlock); > return size; > } > -- > 2.17.1 > > _______________________________________________ > CoreSight mailing list > CoreSight@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/coresight
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index b6984be0c515..fa1d6a938f6c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -652,10 +652,13 @@ static ssize_t cyc_threshold_store(struct device *dev, if (kstrtoul(buf, 16, &val)) return -EINVAL; + + /* mask off max threshold before checking min value */ + val &= ETM_CYC_THRESHOLD_MASK; if (val < drvdata->ccitmin) return -EINVAL; - config->ccctlr = val & ETM_CYC_THRESHOLD_MASK; + config->ccctlr = val; return size; } static DEVICE_ATTR_RW(cyc_threshold); @@ -686,14 +689,16 @@ static ssize_t bb_ctrl_store(struct device *dev, return -EINVAL; if (!drvdata->nr_addr_cmp) return -EINVAL; + /* - * Bit[7:0] selects which address range comparator is used for - * branch broadcast control. + * Bit[8] controls include(1) / exclude(0), bits[0-7] select + * individual range comparators. If include then at least 1 + * range must be selected. */ - if (BMVAL(val, 0, 7) > drvdata->nr_addr_cmp) + if ((val & BIT(8)) && (BMVAL(val, 0, 7) == 0)) return -EINVAL; - config->bb_ctrl = val; + config->bb_ctrl = val & GENMASK(8, 0); return size; } static DEVICE_ATTR_RW(bb_ctrl); @@ -1324,8 +1329,8 @@ static ssize_t seq_event_store(struct device *dev, spin_lock(&drvdata->spinlock); idx = config->seq_idx; - /* RST, bits[7:0] */ - config->seq_ctrl[idx] = val & 0xFF; + /* Seq control has two masks B[15:5] F[7:0] */ + config->seq_ctrl[idx] = val & 0xFFFF; spin_unlock(&drvdata->spinlock); return size; } @@ -1580,7 +1585,7 @@ static ssize_t res_ctrl_store(struct device *dev, if (idx % 2 != 0) /* PAIRINV, bit[21] */ val &= ~BIT(21); - config->res_ctrl[idx] = val; + config->res_ctrl[idx] = val & GENMASK(21, 0); spin_unlock(&drvdata->spinlock); return size; }
A number of issues are fixed relating to sysfs input validation:- 1) bb_ctrl_store() - incorrect compare of bit select field to absolute value. Reworked per ETMv4 specification. 2) seq_event_store() - incorrect mask value - register has two event values. 3) cyc_threshold_store() - must mask with max before checking min otherwise wrapped values can set illegal value below min. 4) res_ctrl_store() - update to mask off all res0 bits. Signed-off-by: Mike Leach <mike.leach@linaro.org> --- .../coresight/coresight-etm4x-sysfs.c | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-)