| Message ID | 20240305082040.7445-1-r.smirnov@omp.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fbmon: prevent division by zero in fb_videomode_from_videomode() | expand |
Hi Am 05.03.24 um 09:20 schrieb Roman Smirnov: > The expression htotal * vtotal can have a zero value > on overflow. It is necessary to prevent division by > zero like in fb_var_to_videomode(). > > Found by Linux Verification Center (linuxtesting.org) with Svace. > > Signed-off-by: Roman Smirnov <r.smirnov@omp.ru> > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > --- > drivers/video/fbdev/core/fbmon.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c > index 79e5bfbdd34c..bd98b138da6a 100644 > --- a/drivers/video/fbdev/core/fbmon.c > +++ b/drivers/video/fbdev/core/fbmon.c > @@ -1311,7 +1311,7 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf > int fb_videomode_from_videomode(const struct videomode *vm, > struct fb_videomode *fbmode) > { > - unsigned int htotal, vtotal; > + unsigned int htotal, vtotal, hfreq; > > fbmode->xres = vm->hactive; > fbmode->left_margin = vm->hback_porch; > @@ -1345,7 +1345,8 @@ int fb_videomode_from_videomode(const struct videomode *vm, > vm->vsync_len; > /* prevent division by zero */ > if (htotal && vtotal) { > - fbmode->refresh = vm->pixelclock / (htotal * vtotal); > + hfreq = vm->pixelclock / htotal; > + fbmode->refresh = hfreq / vtotal; I think this can change the end result because of integer rounding on the intermediate result. Maybe use if (htotal && vtotal && (vm->pixelclock / htotal >= vtotal)) for the test. That rules out overflowing multiplication and sets refresh to 0 in such cases. Best regards Thomas > /* a mode must have htotal and vtotal != 0 or it is invalid */ > } else { > fbmode->refresh = 0;
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c index 79e5bfbdd34c..bd98b138da6a 100644 --- a/drivers/video/fbdev/core/fbmon.c +++ b/drivers/video/fbdev/core/fbmon.c @@ -1311,7 +1311,7 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf int fb_videomode_from_videomode(const struct videomode *vm, struct fb_videomode *fbmode) { - unsigned int htotal, vtotal; + unsigned int htotal, vtotal, hfreq; fbmode->xres = vm->hactive; fbmode->left_margin = vm->hback_porch; @@ -1345,7 +1345,8 @@ int fb_videomode_from_videomode(const struct videomode *vm, vm->vsync_len; /* prevent division by zero */ if (htotal && vtotal) { - fbmode->refresh = vm->pixelclock / (htotal * vtotal); + hfreq = vm->pixelclock / htotal; + fbmode->refresh = hfreq / vtotal; /* a mode must have htotal and vtotal != 0 or it is invalid */ } else { fbmode->refresh = 0;