On Wed, 2013-01-16 at 16:22 +0200, Eduardo Valentin wrote: > On 16-01-2013 04:45, Zhang Rui wrote: > > On Wed, 2013-01-02 at 17:29 +0200, Eduardo Valentin wrote: > >> This patch adds an extra check in the data structure while registering > >> a thermal device. The check is to avoid registering zones with a number > >> of trips greater than zero, but with no .get_trip_temp nor .get_trip_type > >> callbacks. Receiving such data structure may end in wrong data access. > >> > >> Signed-off-by: Eduardo Valentin <eduardo.valentin@xxxxxx> > >> --- > >> drivers/thermal/thermal_sys.c | 3 +++ > >> 1 files changed, 3 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > >> index fba27c3..0a1bf6b 100644 > >> --- a/drivers/thermal/thermal_sys.c > >> +++ b/drivers/thermal/thermal_sys.c > >> @@ -1530,6 +1530,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > >> if (!ops || !ops->get_temp) > >> return ERR_PTR(-EINVAL); > >> > >> + if (trips > 0 && !ops->get_trip_type) > >> + return ERR_PTR(-EINVAL); > >> + > > Hmmm, I do not think we need this. > > > > the sysfs I/F trip_point_X_type already does this check. > > Well that's not the point. If you trust only the sysfs handling, fine. > The fix is very punctual. If you pass wrong parameters to > thermal_zone_device_register, it will crash. > > Of course, in either case, with or without his patch, it won't register > the device, so you won't get to the sysfs handling part. At least with > this fix, the crash won't happen and the driver writer will receive an > error code to treat. > > Ok. being specific, what happens is that there are accesses to > ops->get_trip_type, even if trips > 0 and if .get_trip_type is NULL, > right below inside the register function. Instead of checking for > .get_trip_type every time before using it, this patch adds a single > constraint, so that, if you have trips, you must specify their types.. > To me sounds reasonable enough. > agreed. applied to thermal -next. thanks, rui
