On Fri, 5 Apr 2013 14:35:18 -0700 Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Apr 05, 2013 at 02:26:35PM -0700, Joe Perches wrote: > > On Fri, 2013-04-05 at 14:02 -0700, Jacob Pan wrote: > > > +static ssize_t store_event_control(struct device *dev, > > > + struct device_attribute *attr, > > > + const char *buf, > > > + size_t size) > > > +{ > > > + struct rapl_domain *rd = dev_get_drvdata(dev); > > > + unsigned int efd, new_threshold; > > > + struct file *efile = NULL; > > > + int ret = 0; > > > + int prim; > > > + struct rapl_event *ep; > > > + u64 val; > > > + char cmd[MAX_PRIM_NAME]; > > > + > > > + if (sscanf(buf, "%u %s %u", &efd, cmd, > > > &new_threshold) != 3) > > > + return -EINVAL; > > > > This sscanf looks fragile. > > > > buf = "1 some_really_long_name_longer_than_MAX_PRIM_NAME 2" > > > > stack overrun. > > > > Where does buf come from? > > It comes from the sysfs core, which limits it to a PAGE_SIZE. But > yes, it does look fragile, and flat out wrong, but I'm not going into > that just yet, as that whole api should just be deleted for now. > > greg k-h > I am just curious to know besides fixing string length check, is the problem related to the fact that I have more than one element for the event_control sysfs node? i.e. <eventfd> <event_name> <threshold> If so, the reason being I don't know if there is a standard way to a transaction via sysfs where I can commit multiple inputs atomically. -- Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html