2022-02-09 07:34:13

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH v2 12/19] fbcon: use lock_fb_info in fbcon_open/release

Now we get to the real motiviation, because fbmem.c insists that
that's the right lock for these.

Ofc fbcon.c has a lot more places where it probably should call
lock_fb_info(). But looking at fbmem.c at least most of these seem to
be protected by console_lock() too, which is probably what papers over
any issues.

Note that this means we're shuffling around a bit the locking sections
for some of the console takeover and unbind paths, but not all:
- console binding/unbinding from the console layer never with
lock_fb_info
- unbind (as opposed to unlink) never bother with lock_fb_info

Also the real serialization against set_par and set_pan are still
doing by wrapping the entire ioctl code in console_lock(). So this
shuffling shouldn't be worse than what we had from a "can you trigger
races?" pov, but it's at least clearer.

Acked-by: Sam Ravnborg <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Claudio Suarez <[email protected]>
Cc: Tetsuo Handa <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Du Cheng <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: William Kucharski <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Zheyu Ma <[email protected]>
Cc: Zhen Lei <[email protected]>
Cc: Xiyu Yang <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 5 +++++
drivers/video/fbdev/core/fbmem.c | 4 ----
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index f0213a0e3870..cc960bf49991 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -684,8 +684,10 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)

static void fbcon_release(struct fb_info *info)
{
+ lock_fb_info(info);
if (info->fbops->fb_release)
info->fbops->fb_release(info, 0);
+ unlock_fb_info(info);

module_put(info->fbops->owner);
}
@@ -697,11 +699,14 @@ static int fbcon_open(struct fb_info *info)
if (!try_module_get(info->fbops->owner))
return -ENODEV;

+ lock_fb_info(info);
if (info->fbops->fb_open &&
info->fbops->fb_open(info, 0)) {
+ unlock_fb_info(info);
module_put(info->fbops->owner);
return -ENODEV;
}
+ unlock_fb_info(info);

ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
if (!ops) {
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index ad9aac06427a..37656883e7bd 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1674,9 +1674,7 @@ static int do_register_framebuffer(struct fb_info *fb_info)
console_lock();
else
atomic_inc(&ignore_console_lock_warning);
- lock_fb_info(fb_info);
ret = fbcon_fb_registered(fb_info);
- unlock_fb_info(fb_info);

if (!lockless_register_fb)
console_unlock();
@@ -1693,9 +1691,7 @@ static void unbind_console(struct fb_info *fb_info)
return;

console_lock();
- lock_fb_info(fb_info);
fbcon_fb_unbind(fb_info);
- unlock_fb_info(fb_info);
console_unlock();
}

--
2.34.1