| Message ID | 20150821085302.GE25369@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote: > The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE. > Besides that was in the original code, the problem still might happen when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns desired length. I suppose you would change this to check len on each iteration or change to scnprintf() if I get it correct. > diff --git a/drivers/video/fbdev/core/fbsysfs.c > b/drivers/video/fbdev/core/fbsysfs.c > index 60c3f0a..827098d 100644 > --- a/drivers/video/fbdev/core/fbsysfs.c > +++ b/drivers/video/fbdev/core/fbsysfs.c > @@ -485,7 +485,7 @@ static ssize_t show_bl_curve(struct device > *device, > > mutex_lock(&fb_info->bl_curve_mutex); > for (i = 0; i < FB_BACKLIGHT_LEVELS; i += 8) > - len += snprintf(&buf[len], PAGE_SIZE, "%8ph\n", > + len += snprintf(&buf[len], PAGE_SIZE - len, > "%8ph\n", > fb_info->bl_curve + i); > mutex_unlock(&fb_info->bl_curve_mutex); >
On Fri, Aug 21, 2015 at 12:22:03PM +0300, Andy Shevchenko wrote: > On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote: > > The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE. > > > > Besides that was in the original code, the problem still might happen > when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns > desired length. I suppose you would change this to check len on each > iteration or change to scnprintf() if I get it correct. > Yeah, you're right. If you pass a negative size to snprintf() it won't overflow, but it will print an error. I will send a v2 and change it to scnprintf(). I don't think we will have 171 levels so it's not worth adding the check? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-08-21 at 12:53 +0300, Dan Carpenter wrote: > On Fri, Aug 21, 2015 at 12:22:03PM +0300, Andy Shevchenko wrote: > > On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote: > > > The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE. > > > > > > > Besides that was in the original code, the problem still might > > happen > > when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns > > desired length. I suppose you would change this to check len on > > each > > iteration or change to scnprintf() if I get it correct. > > > > Yeah, you're right. > > If you pass a negative size to snprintf() it won't overflow, but it > will > print an error. I will send a v2 and change it to scnprintf(). I > don't > think we will have 171 levels so it's not worth adding the check? I think there is no need to check for that. Especially it might be changed in the future (if, for example, they decide to have 16 levels per line instead of 8). > > regards, > dan carpenter >
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index 60c3f0a..827098d 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -485,7 +485,7 @@ static ssize_t show_bl_curve(struct device *device, mutex_lock(&fb_info->bl_curve_mutex); for (i = 0; i < FB_BACKLIGHT_LEVELS; i += 8) - len += snprintf(&buf[len], PAGE_SIZE, "%8ph\n", + len += snprintf(&buf[len], PAGE_SIZE - len, "%8ph\n", fb_info->bl_curve + i); mutex_unlock(&fb_info->bl_curve_mutex);
The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html