Messages in this thread | | | Date | Mon, 2 Jan 2023 16:28:57 +0100 | Subject | Re: [PATCH] fbmem: prevent potential use-after-free issues with console_lock() | From | 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@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; > }
| |