Re: [PATCH stable 4.14 v3 2/3] fbcon: Prevent that screen size is smaller than font size

["chenjun (AM)" <chenjun102@huawei.com>]

在 2022/8/5 14:09, Cengiz Can 写道:
> On Thu, 2022-08-04 at 12:27 +0000, Chen Jun wrote:
>> From: Helge Deller <deller@gmx.de>
>>
>> commit e64242caef18b4a5840b0e7a9bff37abd4f4f933 upstream
>>
>> We need to prevent that users configure a screen size which is smaller than the
>> currently selected font size. Otherwise rendering chars on the screen will
>> access memory outside the graphics memory region.
>>
>> This patch adds a new function fbcon_modechange_possible() which
>> implements this check and which later may be extended with other checks
>> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
>> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
>> for a too small screen size.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> [Chen Jun: adjust context]
>> Signed-off-by: Chen Jun <chenjun102@huawei.com>
>> ---
>>   drivers/video/fbdev/core/fbcon.c | 28 ++++++++++++++++++++++++++++
>>   drivers/video/fbdev/core/fbmem.c | 10 +++++++---
>>   include/linux/fbcon.h            |  4 ++++
>>   3 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>> index a97e94b1c84f..b84264e98929 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -2706,6 +2706,34 @@ static void fbcon_set_all_vcs(struct fb_info *info)
>>   		fbcon_modechanged(info);
>>   }
>>   
>> +/* let fbcon check if it supports a new screen resolution */
>> +int fbcon_modechange_possible(struct fb_info *info, struct fb_var_screeninfo *var)
>> +{
>> +	struct fbcon_ops *ops = info->fbcon_par;
>> +	struct vc_data *vc;
>> +	unsigned int i;
>> +
>> +	WARN_CONSOLE_UNLOCKED();
>> +
>> +	if (!ops)
>> +		return 0;
>> +
>> +	/* prevent setting a screen size which is smaller than font size */
>> +	for (i = first_fb_vc; i <= last_fb_vc; i++) {
>> +		vc = vc_cons[i].d;
>> +		if (!vc || vc->vc_mode != KD_TEXT ||
>> +			   registered_fb[con2fb_map[i]] != info)
>> +			continue;
>> +
>> +		if (vc->vc_font.width  > FBCON_SWAP(var->rotate, var->xres, var->yres) ||
>> +		    vc->vc_font.height > FBCON_SWAP(var->rotate, var->yres, var->xres))
>> +			return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fbcon_modechange_possible);
>> +
>>   static int fbcon_mode_deleted(struct fb_info *info,
>>   			      struct fb_videomode *mode)
>>   {
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index 9087d467cc46..264e8ca5efa7 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1134,9 +1134,13 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>   			console_unlock();
>>   			return -ENODEV;
>>   		}
>> -		info->flags |= FBINFO_MISC_USEREVENT;
>> -		ret = fb_set_var(info, &var);
>> -		info->flags &= ~FBINFO_MISC_USEREVENT;
>> +		ret = fbcon_modechange_possible(info, &var);
>> +		if (!ret) {
>> +			info->flags |= FBINFO_MISC_USEREVENT;
>> +			ret = fb_set_var(info, &var);
>> +			info->flags &= ~FBINFO_MISC_USEREVENT;
>> +		}
>> +		lock_fb_info(info);
>>   		unlock_fb_info(info);
> 
> Why do we lock and unlock here consecutively?
> 
> Can it be a leftover?
> 
> Because in upstream commit, lock encapsulates `fb_set_var`,
> `fbcon_modechange_possible` and `fbcon_update_vcs` calls, which makes
> sense.
> 
> Here, it doesn't.
> 

Thanks, lock_fb_info(info) is wrong here.

>>   		console_unlock();
>>   		if (!ret && copy_to_user(argp, &var, sizeof(var)))
>> diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
>> index f68a7db14165..39939d55c834 100644
>> --- a/include/linux/fbcon.h
>> +++ b/include/linux/fbcon.h
>> @@ -4,9 +4,13 @@
>>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE
>>   void __init fb_console_init(void);
>>   void __exit fb_console_exit(void);
>> +int  fbcon_modechange_possible(struct fb_info *info,
>> +			       struct fb_var_screeninfo *var);
>>   #else
>>   static inline void fb_console_init(void) {}
>>   static inline void fb_console_exit(void) {}
>> +static inline int  fbcon_modechange_possible(struct fb_info *info,
>> +				struct fb_var_screeninfo *var) { return 0; }
>>   #endif
>>   
>>   #endif /* _LINUX_FBCON_H */
> 
> 


-- 
Regards
Chen Jun