| Message ID | 20200715121338.GA18761@e119603-lin.cambridge.arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RESEND] hwmon: scmi: fix potential buffer overflow in scmi_hwmon_probe() | expand |
On Wed, Jul 15, 2020 at 01:13:38PM +0100, Cristian Marussi wrote: > SMATCH detected a potential buffer overflow in the manipulation of > hwmon_attributes array inside the scmi_hwmon_probe function: > > drivers/hwmon/scmi-hwmon.c:226 > scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 > > Fix it by statically declaring the size of the array as the maximum > possible as defined by hwmon_max define. > Makes sense to me, Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> There may be other such instances. I am not sure if Guenter has ignored them intentionally or just no one has fixed them so far. -- Regards, Sudeep
On Wed, Jul 15, 2020 at 01:13:38PM +0100, Cristian Marussi wrote: > SMATCH detected a potential buffer overflow in the manipulation of > hwmon_attributes array inside the scmi_hwmon_probe function: > > drivers/hwmon/scmi-hwmon.c:226 > scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 > > Fix it by statically declaring the size of the array as the maximum > possible as defined by hwmon_max define. > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> Applied. Thanks, Guenter > --- > drivers/hwmon/scmi-hwmon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c > index 281454c5c5b1..758f66fc9afe 100644 > --- a/drivers/hwmon/scmi-hwmon.c > +++ b/drivers/hwmon/scmi-hwmon.c > @@ -149,7 +149,7 @@ static enum hwmon_sensor_types scmi_types[] = { > [ENERGY] = hwmon_energy, > }; > > -static u32 hwmon_attributes[] = { > +static u32 hwmon_attributes[hwmon_max] = { > [hwmon_chip] = HWMON_C_REGISTER_TZ, > [hwmon_temp] = HWMON_T_INPUT | HWMON_T_LABEL, > [hwmon_in] = HWMON_I_INPUT | HWMON_I_LABEL,
On 7/15/20 6:00 AM, Sudeep Holla wrote: > On Wed, Jul 15, 2020 at 01:13:38PM +0100, Cristian Marussi wrote: >> SMATCH detected a potential buffer overflow in the manipulation of >> hwmon_attributes array inside the scmi_hwmon_probe function: >> >> drivers/hwmon/scmi-hwmon.c:226 >> scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 >> >> Fix it by statically declaring the size of the array as the maximum >> possible as defined by hwmon_max define. >> > > Makes sense to me, > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > There may be other such instances. I am not sure if Guenter has ignored > them intentionally or just no one has fixed them so far. I am not perfect. No, I have not intentionally ignored anything, and I don't recall seeing smatch reports (or this patch) before. Guenter
On Wed, Jul 15, 2020 at 07:55:52AM -0700, Guenter Roeck wrote: > On 7/15/20 6:00 AM, Sudeep Holla wrote: > > On Wed, Jul 15, 2020 at 01:13:38PM +0100, Cristian Marussi wrote: > >> SMATCH detected a potential buffer overflow in the manipulation of > >> hwmon_attributes array inside the scmi_hwmon_probe function: > >> > >> drivers/hwmon/scmi-hwmon.c:226 > >> scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 > >> > >> Fix it by statically declaring the size of the array as the maximum > >> possible as defined by hwmon_max define. > >> > > > > Makes sense to me, > > > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > > > There may be other such instances. I am not sure if Guenter has ignored > > them intentionally or just no one has fixed them so far. > > I am not perfect. No, I have not intentionally ignored anything, > and I don't recall seeing smatch reports (or this patch) before. > Sorry, it was not complaint, it does sound so now when I read that again. What I meant is, not everyone likes to fix all the warnings from various tools and I was just asking if this falls into that category as the overflow can't happen if we use the standard hwmon_max enums as indicies.
On Wed, Jul 15, 2020 at 07:55:52AM -0700, Guenter Roeck wrote: > On 7/15/20 6:00 AM, Sudeep Holla wrote: > > On Wed, Jul 15, 2020 at 01:13:38PM +0100, Cristian Marussi wrote: > >> SMATCH detected a potential buffer overflow in the manipulation of > >> hwmon_attributes array inside the scmi_hwmon_probe function: > >> > >> drivers/hwmon/scmi-hwmon.c:226 > >> scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 > >> > >> Fix it by statically declaring the size of the array as the maximum > >> possible as defined by hwmon_max define. > >> > > > > Makes sense to me, > > > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > > > There may be other such instances. I am not sure if Guenter has ignored > > them intentionally or just no one has fixed them so far. > > I am not perfect. No, I have not intentionally ignored anything, > and I don't recall seeing smatch reports (or this patch) before. Sorry, that's my fault I sent this patch the first time to the wrong recipients. Cristian > > Guenter
diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c index 281454c5c5b1..758f66fc9afe 100644 --- a/drivers/hwmon/scmi-hwmon.c +++ b/drivers/hwmon/scmi-hwmon.c @@ -149,7 +149,7 @@ static enum hwmon_sensor_types scmi_types[] = { [ENERGY] = hwmon_energy, }; -static u32 hwmon_attributes[] = { +static u32 hwmon_attributes[hwmon_max] = { [hwmon_chip] = HWMON_C_REGISTER_TZ, [hwmon_temp] = HWMON_T_INPUT | HWMON_T_LABEL, [hwmon_in] = HWMON_I_INPUT | HWMON_I_LABEL,
SMATCH detected a potential buffer overflow in the manipulation of hwmon_attributes array inside the scmi_hwmon_probe function: drivers/hwmon/scmi-hwmon.c:226 scmi_hwmon_probe() error: buffer overflow 'hwmon_attributes' 6 <= 9 Fix it by statically declaring the size of the array as the maximum possible as defined by hwmon_max define. Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> --- drivers/hwmon/scmi-hwmon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)