Re: [PATCH v2 3/4] fbmem: Fix input parameter checks for user-provided screen resolution changes

From: Daniel Vetter <daniel@ffwll.ch>
To: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org, daniel.vetter@ffwll.ch,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 3/4] fbmem: Fix input parameter checks for user-provided screen resolution changes
Date: Sat, 25 Jun 2022 15:00:35 +0200	[thread overview]
Message-ID: <YrcG80jK2e44LCtb@phenom.ffwll.local> (raw)
In-Reply-To: <YrcGCkHuWHv3I6Fx@phenom.ffwll.local>

On Sat, Jun 25, 2022 at 02:56:42PM +0200, Daniel Vetter wrote:
> On Sat, Jun 25, 2022 at 02:25:01PM +0200, Helge Deller wrote:
> > Enhance the checks in the FBIOPUT_VSCREENINFO ioctl handler to verify
> > the user-provided new screen size for:
> > 
> > a) virtual screen size >= physical screen size, and
> > 
> > b) new screen size is bigger than currently configured console font size.
> > 
> > Return -EINVAL on invalid input.
> > 
> > Signed-off-by: Helge Deller <deller@gmx.de>
> > Cc: stable@vger.kernel.org # v5.4+
> 
> Imo squash this into the previous one please. Doesn't make sense to split
> the patch which adds a function from it's callsite.

Correction. The part to add the fbcon_modechange_possible call should be
squashed into the previos patch.

The check for x/yres_virtaul < x/yres should imo be moved into fb_set_var,
next to the other existing checks that have been added over time.
-Daniel

> -Daniel
> 
> > ---
> >  drivers/video/fbdev/core/fbmem.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index afa2863670f3..50fb66b954d6 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -1106,7 +1106,13 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> >  			return -EFAULT;
> >  		console_lock();
> >  		lock_fb_info(info);
> > -		ret = fb_set_var(info, &var);
> > +		if (var.xres_virtual < var.xres ||
> > +		    var.yres_virtual < var.yres)
> > +			ret = -EINVAL;
> > +		if (!ret)
> > +			ret = fbcon_modechange_possible(info, &var);
> > +		if (!ret)
> > +			ret = fb_set_var(info, &var);
> >  		if (!ret)
> >  			fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
> >  		unlock_fb_info(info);
> > --
> > 2.35.3
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch