From patchwork Tue May 28 09:02:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10964285 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1823476 for ; Tue, 28 May 2019 09:04:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 05F66285E2 for ; Tue, 28 May 2019 09:04:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EDEE6285F7; Tue, 28 May 2019 09:04:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4D54D271FD for ; Tue, 28 May 2019 09:04:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0392589E7C; Tue, 28 May 2019 09:03:41 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id ED1B789E9E for ; Tue, 28 May 2019 09:03:36 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id n17so30688493edb.0 for ; Tue, 28 May 2019 02:03:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=qp8gYAWJCiDOhrz1RfuOYXHHnFuqBtMXvmbENgYp7nA=; b=JEmpLQZtlQhBcqVjcCW3QjEJggZpXcI/Xc1Z4L0uyUlcdoOY3NuNrkx4hUAlMA/cU1 mh9TYYT7t4oVLXR2s9a+TOlBMfQZW4qv3tmBgqB9k0YtFLDZHgpSjv/DY6Cvk8f1lRc6 uudlWo58j/EPEl0Y/4t109G9ymt79TpusHDo2vDq8EZf0ymbrMf3IS14kkeFAa8XEeSX PVL1gZKBMZVyOwKSZ2UBS2TtyfPfa1ZS7TX8rIPhmVz6p2dsv+YXeNiCyjHOrmr29+Cq 8g8rBA/S5GcsuUOj158k288570Wbk8L1aT3IOfGPK7lMzpJ0IKW/ryBuqJSX+QURZcNt YuRQ== X-Gm-Message-State: APjAAAWuJPXugHVjcMeAGqSqSjdiItJgHG5qq6XdLbPTNPASscaoCjpR mTXWUAhQJ+5k9rwhLCu5Gmj81w== X-Google-Smtp-Source: APXvYqxWwkOmq1B5WlNL4LZsiFHjKy9tx1xdx2Sqd/9u6FMIp/LEyZi1VfAxiFE3W+jJaainnSnkzA== X-Received: by 2002:a17:906:491b:: with SMTP id b27mr33967503ejq.234.1559034215068; Tue, 28 May 2019 02:03:35 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id x49sm4072656edm.25.2019.05.28.02.03.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 May 2019 02:03:34 -0700 (PDT) From: Daniel Vetter To: LKML Subject: [PATCH 16/33] fbdev: lock_fb_info cannot fail Date: Tue, 28 May 2019 11:02:47 +0200 Message-Id: <20190528090304.9388-17-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190528090304.9388-1-daniel.vetter@ffwll.ch> References: <20190528090304.9388-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=qp8gYAWJCiDOhrz1RfuOYXHHnFuqBtMXvmbENgYp7nA=; b=D/Qm2B4Nwv9VOkhXwW2zqWA9R6pO/8mljnKNDoMbQqwe10J4DIpCovISBE10ZfmIiz DUfZs/QewB0L/nMLc+4gH29xMFxS8TlJqHQOMPNMiOtE5d5W6SGFISi4qVJuzIvKRILw UsMDYJ37BMOxsF4thN3vvqssEwWctti5fq1Os= X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Thompson , Sergey Senozhatsky , Bartlomiej Zolnierkiewicz , Daniel Vetter , Intel Graphics Development , "Gustavo A. R. Silva" , linux-fbdev@vger.kernel.org, DRI Development , =?utf-8?b?TWljaGHFgiBN?= =?utf-8?b?aXJvc8WCYXc=?= , Yisheng Xie , Hans de Goede , Mikulas Patocka , Daniel Vetter , Sam Ravnborg , Peter Rosin Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Ever since commit c47747fde931c02455683bd00ea43eaa62f35b0e Author: Linus Torvalds Date: Wed May 11 14:58:34 2011 -0700 fbmem: make read/write/ioctl use the frame buffer at open time fbdev has gained proper refcounting for the fbinfo attached to any open files, which means that the backing driver (stored in fb_info->fbops) cannot untimely disappear anymore. The only thing that can happen is that the entire device just outright disappears and gets unregistered, but file_fb_info does check for that. Except that it's racy - it only checks once at the start of a file_ops, there's no guarantee that the underlying fbdev won't untimely disappear. Aside: A proper way to fix that race is probably to replicate the srcu trickery we've rolled out in drm. But given that this race has existed since forever it's probably not one we need to fix right away. do_unregister_framebuffer also nowhere clears fb_info->fbops, hence the check in lock_fb_info can't possible catch a disappearing fbdev later on. Long story short: Ever since the above commit the fb_info->fbops checks have essentially become dead code. Remove this all. Aside from the file_ops callbacks, and stuff called from there there's only register/unregister code left. If that goes wrong a driver managed to register/unregister a device instance twice or in the wrong order. That's just a driver bug. v2: - fb_mmap had an open-coded version of the fbinfo->fops check, because it doesn't need the fbinfo->lock. Delete that too. - Use the wrapper function in fb_open/release now, since no difference anymore. Signed-off-by: Daniel Vetter Reviewed-by: Sam Ravnborg Reviewed-by: Maarten Lankhorst Cc: Bartlomiej Zolnierkiewicz Cc: Daniel Vetter Cc: Hans de Goede Cc: Yisheng Xie Cc: Sergey Senozhatsky Cc: "Noralf Trønnes" Cc: Peter Rosin Cc: "Michał Mirosław" Cc: Mikulas Patocka Cc: "Gustavo A. R. Silva" Cc: linux-fbdev@vger.kernel.org --- drivers/video/fbdev/core/fbcmap.c | 6 +-- drivers/video/fbdev/core/fbcon.c | 3 +- drivers/video/fbdev/core/fbmem.c | 73 +++++++------------------------ include/linux/fb.h | 5 ++- 4 files changed, 23 insertions(+), 64 deletions(-) diff --git a/drivers/video/fbdev/core/fbcmap.c b/drivers/video/fbdev/core/fbcmap.c index 2811c4afde01..e5ae33c1a8e8 100644 --- a/drivers/video/fbdev/core/fbcmap.c +++ b/drivers/video/fbdev/core/fbcmap.c @@ -285,11 +285,7 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info) goto out; } umap.start = cmap->start; - if (!lock_fb_info(info)) { - rc = -ENODEV; - goto out; - } - + lock_fb_info(info); rc = fb_set_cmap(&umap, info); unlock_fb_info(info); out: diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 5a9fe0c0dd3c..a7e2d5b88914 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2364,8 +2364,7 @@ static void fbcon_generic_blank(struct vc_data *vc, struct fb_info *info, } - if (!lock_fb_info(info)) - return; + lock_fb_info(info); event.info = info; event.data = ␣ fb_notifier_call_chain(FB_EVENT_CONBLANK, &event); diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index bed7698ad18a..d73762324ca2 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -80,17 +80,6 @@ static void put_fb_info(struct fb_info *fb_info) fb_info->fbops->fb_destroy(fb_info); } -int lock_fb_info(struct fb_info *info) -{ - mutex_lock(&info->lock); - if (!info->fbops) { - mutex_unlock(&info->lock); - return 0; - } - return 1; -} -EXPORT_SYMBOL(lock_fb_info); - /* * Helpers */ @@ -1121,8 +1110,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, switch (cmd) { case FBIOGET_VSCREENINFO: - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); var = info->var; unlock_fb_info(info); @@ -1132,10 +1120,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, if (copy_from_user(&var, argp, sizeof(var))) return -EFAULT; console_lock(); - if (!lock_fb_info(info)) { - console_unlock(); - return -ENODEV; - } + lock_fb_info(info); info->flags |= FBINFO_MISC_USEREVENT; ret = fb_set_var(info, &var); info->flags &= ~FBINFO_MISC_USEREVENT; @@ -1145,8 +1130,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, ret = -EFAULT; break; case FBIOGET_FSCREENINFO: - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); fix = info->fix; if (info->flags & FBINFO_HIDE_SMEM_START) fix.smem_start = 0; @@ -1162,8 +1146,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOGETCMAP: if (copy_from_user(&cmap, argp, sizeof(cmap))) return -EFAULT; - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); cmap_from = info->cmap; unlock_fb_info(info); ret = fb_cmap_to_user(&cmap_from, &cmap); @@ -1172,10 +1155,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, if (copy_from_user(&var, argp, sizeof(var))) return -EFAULT; console_lock(); - if (!lock_fb_info(info)) { - console_unlock(); - return -ENODEV; - } + lock_fb_info(info); ret = fb_pan_display(info, &var); unlock_fb_info(info); console_unlock(); @@ -1192,8 +1172,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, return -EINVAL; con2fb.framebuffer = -1; event.data = &con2fb; - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); event.info = info; fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event); unlock_fb_info(info); @@ -1214,10 +1193,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, } event.data = &con2fb; console_lock(); - if (!lock_fb_info(info)) { - console_unlock(); - return -ENODEV; - } + lock_fb_info(info); event.info = info; ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event); unlock_fb_info(info); @@ -1225,10 +1201,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, break; case FBIOBLANK: console_lock(); - if (!lock_fb_info(info)) { - console_unlock(); - return -ENODEV; - } + lock_fb_info(info); info->flags |= FBINFO_MISC_USEREVENT; ret = fb_blank(info, arg); info->flags &= ~FBINFO_MISC_USEREVENT; @@ -1236,8 +1209,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, console_unlock(); break; default: - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); fb = info->fbops; if (fb->fb_ioctl) ret = fb->fb_ioctl(info, cmd, arg); @@ -1357,8 +1329,7 @@ static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd, { struct fb_fix_screeninfo fix; - if (!lock_fb_info(info)) - return -ENODEV; + lock_fb_info(info); fix = info->fix; if (info->flags & FBINFO_HIDE_SMEM_START) fix.smem_start = 0; @@ -1418,8 +1389,6 @@ fb_mmap(struct file *file, struct vm_area_struct * vma) if (!info) return -ENODEV; fb = info->fbops; - if (!fb) - return -ENODEV; mutex_lock(&info->mm_lock); if (fb->fb_mmap) { int res; @@ -1483,7 +1452,7 @@ __releases(&info->lock) if (IS_ERR(info)) return PTR_ERR(info); - mutex_lock(&info->lock); + lock_fb_info(info); if (!try_module_get(info->fbops->owner)) { res = -ENODEV; goto out; @@ -1499,7 +1468,7 @@ __releases(&info->lock) fb_deferred_io_open(info, inode, file); #endif out: - mutex_unlock(&info->lock); + unlock_fb_info(info); if (res) put_fb_info(info); return res; @@ -1512,11 +1481,11 @@ __releases(&info->lock) { struct fb_info * const info = file->private_data; - mutex_lock(&info->lock); + lock_fb_info(info); if (info->fbops->fb_release) info->fbops->fb_release(info,1); module_put(info->fbops->owner); - mutex_unlock(&info->lock); + unlock_fb_info(info); put_fb_info(info); return 0; } @@ -1734,14 +1703,10 @@ static int do_register_framebuffer(struct fb_info *fb_info) console_lock(); else atomic_inc(&ignore_console_lock_warning); - if (!lock_fb_info(fb_info)) { - ret = -ENODEV; - goto unlock_console; - } - + lock_fb_info(fb_info); ret = fbcon_fb_registered(fb_info); unlock_fb_info(fb_info); -unlock_console: + if (!lockless_register_fb) console_unlock(); else @@ -1759,11 +1724,7 @@ static int unbind_console(struct fb_info *fb_info) return -EINVAL; console_lock(); - if (!lock_fb_info(fb_info)) { - console_unlock(); - return -ENODEV; - } - + lock_fb_info(fb_info); event.info = fb_info; ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event); unlock_fb_info(fb_info); diff --git a/include/linux/fb.h b/include/linux/fb.h index 288175fafaf6..aa8f18163151 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -663,7 +663,10 @@ extern struct class *fb_class; for (i = 0; i < FB_MAX; i++) \ if (!registered_fb[i]) {} else -extern int lock_fb_info(struct fb_info *info); +static inline void lock_fb_info(struct fb_info *info) +{ + mutex_lock(&info->lock); +} static inline void unlock_fb_info(struct fb_info *info) {