| Message ID | 20230307130856.2295182-1-harperchen1110@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fbdev: tgafb: Fix potential divide by zero | expand |
On 3/7/23 14:08, harperchen wrote: > fb_set_var would by called when user invokes ioctl with cmd > FBIOPUT_VSCREENINFO. User-provided data would finally reach > tgafb_check_var. In case var->pixclock is assigned to zero, > divide by zero would occur when checking whether reciprocal > of var->pixclock is too high. > > Similar crashes have happened in other fbdev drivers. There > is no check and modification on var->pixclock along the call > chain to tgafb_check_var. We believe it could also be triggered > in driver tgafb from user site. > > Signed-off-by: harperchen <harperchen1110@gmail.com> Could you provide a real name? Otherwise applied to fbdev git tree. Thanks! Helge > --- > drivers/video/fbdev/tgafb.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c > index 14d37c49633c..b44004880f0d 100644 > --- a/drivers/video/fbdev/tgafb.c > +++ b/drivers/video/fbdev/tgafb.c > @@ -173,6 +173,9 @@ tgafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) > { > struct tga_par *par = (struct tga_par *)info->par; > > + if (!var->pixclock) > + return -EINVAL; > + > if (par->tga_type == TGA_TYPE_8PLANE) { > if (var->bits_per_pixel != 8) > return -EINVAL;
Dear Helge, Thank you for the kind words. My real name is Wei Chen. Please apply this patch to fbdev git tree if it is correct. Best, Wei On Thu, 9 Mar 2023 at 06:05, Helge Deller <deller@gmx.de> wrote: > > On 3/7/23 14:08, harperchen wrote: > > fb_set_var would by called when user invokes ioctl with cmd > > FBIOPUT_VSCREENINFO. User-provided data would finally reach > > tgafb_check_var. In case var->pixclock is assigned to zero, > > divide by zero would occur when checking whether reciprocal > > of var->pixclock is too high. > > > > Similar crashes have happened in other fbdev drivers. There > > is no check and modification on var->pixclock along the call > > chain to tgafb_check_var. We believe it could also be triggered > > in driver tgafb from user site. > > > > Signed-off-by: harperchen <harperchen1110@gmail.com> > > Could you provide a real name? > Otherwise applied to fbdev git tree. > > Thanks! > Helge > > > --- > > drivers/video/fbdev/tgafb.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c > > index 14d37c49633c..b44004880f0d 100644 > > --- a/drivers/video/fbdev/tgafb.c > > +++ b/drivers/video/fbdev/tgafb.c > > @@ -173,6 +173,9 @@ tgafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) > > { > > struct tga_par *par = (struct tga_par *)info->par; > > > > + if (!var->pixclock) > > + return -EINVAL; > > + > > if (par->tga_type == TGA_TYPE_8PLANE) { > > if (var->bits_per_pixel != 8) > > return -EINVAL; >
On Wed, 08 Mar 2023, Helge Deller <deller@gmx.de> wrote: > On 3/7/23 14:08, harperchen wrote: >> fb_set_var would by called when user invokes ioctl with cmd >> FBIOPUT_VSCREENINFO. User-provided data would finally reach >> tgafb_check_var. In case var->pixclock is assigned to zero, >> divide by zero would occur when checking whether reciprocal >> of var->pixclock is too high. >> >> Similar crashes have happened in other fbdev drivers. There >> is no check and modification on var->pixclock along the call >> chain to tgafb_check_var. We believe it could also be triggered >> in driver tgafb from user site. >> >> Signed-off-by: harperchen <harperchen1110@gmail.com> > > Could you provide a real name? > Otherwise applied to fbdev git tree. See commit d4563201f33a ("Documentation: simplify and clarify DCO contribution example language"). BR, Jani. > > Thanks! > Helge > >> --- >> drivers/video/fbdev/tgafb.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c >> index 14d37c49633c..b44004880f0d 100644 >> --- a/drivers/video/fbdev/tgafb.c >> +++ b/drivers/video/fbdev/tgafb.c >> @@ -173,6 +173,9 @@ tgafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) >> { >> struct tga_par *par = (struct tga_par *)info->par; >> >> + if (!var->pixclock) >> + return -EINVAL; >> + >> if (par->tga_type == TGA_TYPE_8PLANE) { >> if (var->bits_per_pixel != 8) >> return -EINVAL; >
On 3/9/23 08:53, Jani Nikula wrote: > On Wed, 08 Mar 2023, Helge Deller <deller@gmx.de> wrote: >> On 3/7/23 14:08, harperchen wrote: >>> fb_set_var would by called when user invokes ioctl with cmd >>> FBIOPUT_VSCREENINFO. User-provided data would finally reach >>> tgafb_check_var. In case var->pixclock is assigned to zero, >>> divide by zero would occur when checking whether reciprocal >>> of var->pixclock is too high. >>> >>> Similar crashes have happened in other fbdev drivers. There >>> is no check and modification on var->pixclock along the call >>> chain to tgafb_check_var. We believe it could also be triggered >>> in driver tgafb from user site. >>> >>> Signed-off-by: harperchen <harperchen1110@gmail.com> >> >> Could you provide a real name? >> Otherwise applied to fbdev git tree. > > See commit d4563201f33a ("Documentation: simplify and clarify DCO > contribution example language"). Nice. Thanks for that link! Btw, I did applied that patch yesterday to my tree with just the nickname, but of course I do prefer real names which is why I asked. Helge
diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c index 14d37c49633c..b44004880f0d 100644 --- a/drivers/video/fbdev/tgafb.c +++ b/drivers/video/fbdev/tgafb.c @@ -173,6 +173,9 @@ tgafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) { struct tga_par *par = (struct tga_par *)info->par; + if (!var->pixclock) + return -EINVAL; + if (par->tga_type == TGA_TYPE_8PLANE) { if (var->bits_per_pixel != 8) return -EINVAL;
fb_set_var would by called when user invokes ioctl with cmd FBIOPUT_VSCREENINFO. User-provided data would finally reach tgafb_check_var. In case var->pixclock is assigned to zero, divide by zero would occur when checking whether reciprocal of var->pixclock is too high. Similar crashes have happened in other fbdev drivers. There is no check and modification on var->pixclock along the call chain to tgafb_check_var. We believe it could also be triggered in driver tgafb from user site. Signed-off-by: harperchen <harperchen1110@gmail.com> --- drivers/video/fbdev/tgafb.c | 3 +++ 1 file changed, 3 insertions(+)