[PATCH] fbmem: prevent potential use-after-free issues with console_lock()

[Helge Deller]

On 12/30/22 07:35, Hang Zhang wrote:
> In do_fb_ioctl(), user specified "fb_info" can be freed in the callee
> fbcon_get_con2fb_map_ioctl() -> set_con2fb_map() ->
> con2fb_release_oldinfo(), this free operation is protected by
> console_lock() in fbcon_set_con2fb_map_ioctl(), it also results in
> the change of certain states such as "minfo->dead" in matroxfb_remove(),
> so that it can be checked to avoid use-after-free before the use sites
> (e.g., the check at the beginning of matroxfb_ioctl()). However,
> the problem is that the use site is not protected by the same locks
> as for the free operation, e.g., "default" case in do_fb_ioctl()
> can lead to "matroxfb_ioctl()" but it's not protected by console_lock(),
> which can invalidate the aforementioned state set and check in a
> concurrent setting.
>
> Prevent the potential use-after-free issues by protecting the "default"
> case in do_fb_ioctl() with console_lock(), similarly as for many other
> cases like "case FBIOBLANK" and "case FBIOPAN_DISPLAY".
>
> Signed-off-by: Hang Zhang <zh.nvgt at gmail.com>

applied to fbdev git tree.

Thanks,
Helge

> ---
>   drivers/video/fbdev/core/fbmem.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 1e70d8c67653..8b1a1527d18a 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1182,6 +1182,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>   		console_unlock();
>   		break;
>   	default:
> +		console_lock();
>   		lock_fb_info(info);
>   		fb = info->fbops;
>   		if (fb->fb_ioctl)
> @@ -1189,6 +1190,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>   		else
>   			ret = -ENOTTY;
>   		unlock_fb_info(info);
> +		console_unlock();
>   	}
>   	return ret;
>   }