linux-kernel - [PATCH] fbmem: prevent potential use-after-free issues with console

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221230063528.41037-1-zh.nvgt@gmail.com>
Date:   Thu, 29 Dec 2022 22:35:28 -0800
From:   Hang Zhang <zh.nvgt@...il.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     Daniel Vetter <daniel@...ll.ch>, Helge Deller <deller@....de>,
        Javier Martinez Canillas <javierm@...hat.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Sam Ravnborg <sam@...nborg.org>,
        Alex Deucher <alexander.deucher@....com>,
        linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Hang Zhang <zh.nvgt@...il.com>
Subject: [PATCH] fbmem: prevent potential use-after-free issues with console_lock()

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@...il.com>
---
 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;
 }
-- 
2.39.0