From: Thomas Zimmermann <tzimmermann@suse.de>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
DRI Development <dri-devel@lists.freedesktop.org>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
linux-fbdev@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
Sam Ravnborg <sam@ravnborg.org>,
Daniel Vetter <daniel.vetter@intel.com>,
Helge Deller <deller@gmx.de>, Daniel Vetter <daniel@ffwll.ch>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Du Cheng <ducheng2@gmail.com>, Claudio Suarez <cssk@net-c.es>
Subject: Re: [PATCH v2 03/19] fbcon: Introduce wrapper for console->fb_info lookup
Date: Thu, 10 Feb 2022 12:18:52 +0100 [thread overview]
Message-ID: <e15718d3-3ff3-8e3e-ca30-a7470e9f9574@suse.de> (raw)
In-Reply-To: <20220208210824.2238981-4-daniel.vetter@ffwll.ch>
[-- Attachment #1.1: Type: text/plain, Size: 14827 bytes --]
Am 08.02.22 um 22:08 schrieb Daniel Vetter:
> Half of it is protected by console_lock, but the other half is a lot
> more awkward: Registration/deregistration of fbdev are serialized, but
> we don't really clear out anything in con2fb_map and so there's
> potential for use-after free mixups.
>
> First step is to encapsulate the lookup.
>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Du Cheng <ducheng2@gmail.com>
> Cc: Claudio Suarez <cssk@net-c.es>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/core/fbcon.c | 76 ++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index e925bb608e25..b75e638cb83d 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -110,6 +110,18 @@ static struct fbcon_display fb_display[MAX_NR_CONSOLES];
> static signed char con2fb_map[MAX_NR_CONSOLES];
> static signed char con2fb_map_boot[MAX_NR_CONSOLES];
>
> +static struct fb_info *fbcon_info_from_console(int console)
> +{
> + WARN_CONSOLE_UNLOCKED();
> +
> + /*
> + * Note that only con2fb_map is protected by the console lock,
> + * registered_fb is protected by a separate mutex. This lookup can
> + * therefore race.
> + */
> + return registered_fb[con2fb_map[console]];
> +}
> +
> static int logo_lines;
> /* logo_shown is an index to vc_cons when >= 0; otherwise follows FBCON_LOGO
> enums. */
> @@ -199,7 +211,7 @@ static void fbcon_rotate(struct fb_info *info, u32 rotate)
> if (!ops || ops->currcon == -1)
> return;
>
> - fb_info = registered_fb[con2fb_map[ops->currcon]];
> + fb_info = fbcon_info_from_console(ops->currcon);
>
> if (info == fb_info) {
> struct fbcon_display *p = &fb_display[ops->currcon];
> @@ -226,7 +238,7 @@ static void fbcon_rotate_all(struct fb_info *info, u32 rotate)
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> vc = vc_cons[i].d;
> if (!vc || vc->vc_mode != KD_TEXT ||
> - registered_fb[con2fb_map[i]] != info)
> + fbcon_info_from_console(i) != info)
> continue;
>
> p = &fb_display[vc->vc_num];
> @@ -356,7 +368,7 @@ static void fb_flashcursor(struct work_struct *work)
> vc = vc_cons[ops->currcon].d;
>
> if (!vc || !con_is_visible(vc) ||
> - registered_fb[con2fb_map[vc->vc_num]] != info ||
> + fbcon_info_from_console(vc->vc_num) != info ||
> vc->vc_deccm != 1) {
> console_unlock();
> return;
> @@ -791,7 +803,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info,
> if (show_logo) {
> struct vc_data *fg_vc = vc_cons[fg_console].d;
> struct fb_info *fg_info =
> - registered_fb[con2fb_map[fg_console]];
> + fbcon_info_from_console(fg_console);
>
> fbcon_prepare_logo(fg_vc, fg_info, fg_vc->vc_cols,
> fg_vc->vc_rows, fg_vc->vc_cols,
> @@ -1014,7 +1026,7 @@ static void fbcon_init(struct vc_data *vc, int init)
> if (con2fb_map[vc->vc_num] == -1)
> con2fb_map[vc->vc_num] = info_idx;
>
> - info = registered_fb[con2fb_map[vc->vc_num]];
> + info = fbcon_info_from_console(vc->vc_num);
>
> if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
> logo_shown = FBCON_LOGO_DONTSHOW;
> @@ -1231,7 +1243,7 @@ static void fbcon_deinit(struct vc_data *vc)
> static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height,
> int width)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
>
> struct fbcon_display *p = &fb_display[vc->vc_num];
> @@ -1269,7 +1281,7 @@ static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height,
> static void fbcon_putcs(struct vc_data *vc, const unsigned short *s,
> int count, int ypos, int xpos)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_display *p = &fb_display[vc->vc_num];
> struct fbcon_ops *ops = info->fbcon_par;
>
> @@ -1289,7 +1301,7 @@ static void fbcon_putc(struct vc_data *vc, int c, int ypos, int xpos)
>
> static void fbcon_clear_margins(struct vc_data *vc, int bottom_only)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
>
> if (!fbcon_is_inactive(vc, info))
> @@ -1298,7 +1310,7 @@ static void fbcon_clear_margins(struct vc_data *vc, int bottom_only)
>
> static void fbcon_cursor(struct vc_data *vc, int mode)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> int c = scr_readw((u16 *) vc->vc_pos);
>
> @@ -1392,7 +1404,7 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
>
> static __inline__ void ywrap_up(struct vc_data *vc, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
>
> @@ -1411,7 +1423,7 @@ static __inline__ void ywrap_up(struct vc_data *vc, int count)
>
> static __inline__ void ywrap_down(struct vc_data *vc, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
>
> @@ -1430,7 +1442,7 @@ static __inline__ void ywrap_down(struct vc_data *vc, int count)
>
> static __inline__ void ypan_up(struct vc_data *vc, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_display *p = &fb_display[vc->vc_num];
> struct fbcon_ops *ops = info->fbcon_par;
>
> @@ -1454,7 +1466,7 @@ static __inline__ void ypan_up(struct vc_data *vc, int count)
>
> static __inline__ void ypan_up_redraw(struct vc_data *vc, int t, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
>
> @@ -1478,7 +1490,7 @@ static __inline__ void ypan_up_redraw(struct vc_data *vc, int t, int count)
>
> static __inline__ void ypan_down(struct vc_data *vc, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_display *p = &fb_display[vc->vc_num];
> struct fbcon_ops *ops = info->fbcon_par;
>
> @@ -1502,7 +1514,7 @@ static __inline__ void ypan_down(struct vc_data *vc, int count)
>
> static __inline__ void ypan_down_redraw(struct vc_data *vc, int t, int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
>
> @@ -1666,7 +1678,7 @@ static void fbcon_redraw(struct vc_data *vc, struct fbcon_display *p,
> static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy, int sx,
> int dy, int dx, int height, int width, u_int y_break)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> u_int b;
>
> @@ -1708,7 +1720,7 @@ static void fbcon_bmove_rec(struct vc_data *vc, struct fbcon_display *p, int sy,
> static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
> int height, int width)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_display *p = &fb_display[vc->vc_num];
>
> if (fbcon_is_inactive(vc, info))
> @@ -1731,7 +1743,7 @@ static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
> static bool fbcon_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
> enum con_scroll dir, unsigned int count)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_display *p = &fb_display[vc->vc_num];
> int scroll_partial = info->flags & FBINFO_PARTIAL_PAN_OK;
>
> @@ -1996,7 +2008,7 @@ static void updatescrollmode(struct fbcon_display *p,
> static int fbcon_resize(struct vc_data *vc, unsigned int width,
> unsigned int height, unsigned int user)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
> struct fb_var_screeninfo var = info->var;
> @@ -2065,7 +2077,7 @@ static int fbcon_switch(struct vc_data *vc)
> struct fb_var_screeninfo var;
> int i, ret, prev_console;
>
> - info = registered_fb[con2fb_map[vc->vc_num]];
> + info = fbcon_info_from_console(vc->vc_num);
> ops = info->fbcon_par;
>
> if (logo_shown >= 0) {
> @@ -2079,7 +2091,7 @@ static int fbcon_switch(struct vc_data *vc)
>
> prev_console = ops->currcon;
> if (prev_console != -1)
> - old_info = registered_fb[con2fb_map[prev_console]];
> + old_info = fbcon_info_from_console(prev_console);
> /*
> * FIXME: If we have multiple fbdev's loaded, we need to
> * update all info->currcon. Perhaps, we can place this
> @@ -2202,7 +2214,7 @@ static void fbcon_generic_blank(struct vc_data *vc, struct fb_info *info,
>
> static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
>
> if (mode_switch) {
> @@ -2244,7 +2256,7 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
>
> static int fbcon_debug_enter(struct vc_data *vc)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
>
> ops->save_graphics = ops->graphics;
> @@ -2257,7 +2269,7 @@ static int fbcon_debug_enter(struct vc_data *vc)
>
> static int fbcon_debug_leave(struct vc_data *vc)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
>
> ops->graphics = ops->save_graphics;
> @@ -2393,7 +2405,7 @@ static void set_vc_hi_font(struct vc_data *vc, bool set)
> static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> const u8 * data, int userfont)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> struct fbcon_ops *ops = info->fbcon_par;
> struct fbcon_display *p = &fb_display[vc->vc_num];
> int resize;
> @@ -2447,7 +2459,7 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
> static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
> unsigned int flags)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> unsigned charcount = font->charcount;
> int w = font->width;
> int h = font->height;
> @@ -2511,7 +2523,7 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
>
> static int fbcon_set_def_font(struct vc_data *vc, struct console_font *font, char *name)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> const struct font_desc *f;
>
> if (!name)
> @@ -2535,7 +2547,7 @@ static struct fb_cmap palette_cmap = {
>
> static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table)
> {
> - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> + struct fb_info *info = fbcon_info_from_console(vc->vc_num);
> int i, j, k, depth;
> u8 val;
>
> @@ -2651,7 +2663,7 @@ static void fbcon_modechanged(struct fb_info *info)
> return;
> vc = vc_cons[ops->currcon].d;
> if (vc->vc_mode != KD_TEXT ||
> - registered_fb[con2fb_map[ops->currcon]] != info)
> + fbcon_info_from_console(ops->currcon) != info)
> return;
>
> p = &fb_display[vc->vc_num];
> @@ -2691,7 +2703,7 @@ static void fbcon_set_all_vcs(struct fb_info *info)
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> vc = vc_cons[i].d;
> if (!vc || vc->vc_mode != KD_TEXT ||
> - registered_fb[con2fb_map[i]] != info)
> + fbcon_info_from_console(i) != info)
> continue;
>
> if (con_is_visible(vc)) {
> @@ -2954,7 +2966,7 @@ void fbcon_fb_blanked(struct fb_info *info, int blank)
>
> vc = vc_cons[ops->currcon].d;
> if (vc->vc_mode != KD_TEXT ||
> - registered_fb[con2fb_map[ops->currcon]] != info)
> + fbcon_info_from_console(ops->currcon) != info)
> return;
>
> if (con_is_visible(vc)) {
> @@ -2974,7 +2986,7 @@ void fbcon_new_modelist(struct fb_info *info)
> const struct fb_videomode *mode;
>
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> - if (registered_fb[con2fb_map[i]] != info)
> + if (fbcon_info_from_console(i) != info)
> continue;
> if (!fb_display[i].mode)
> continue;
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2022-02-10 11:18 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-08 21:08 [PATCH v2 00/19] fbcon patches, take two Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 01/19] fbcon: delete a few unneeded forward decl Daniel Vetter
2022-02-10 11:17 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 02/19] fbcon: Move fbcon_bmove(_rec) functions Daniel Vetter
2022-02-08 23:06 ` Javier Martinez Canillas
2022-02-10 11:17 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 03/19] fbcon: Introduce wrapper for console->fb_info lookup Daniel Vetter
2022-02-10 11:18 ` Thomas Zimmermann [this message]
2022-02-08 21:08 ` [PATCH v2 04/19] fbcon: delete delayed loading code Daniel Vetter
2022-02-10 11:20 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 05/19] fbdev/sysfs: Fix locking Daniel Vetter
2022-02-10 11:22 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 06/19] fbcon: Use delayed work for cursor Daniel Vetter
2022-02-08 23:59 ` Javier Martinez Canillas
2022-02-10 11:37 ` Thomas Zimmermann
2022-02-10 11:43 ` Tetsuo Handa
2022-04-05 20:54 ` Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 07/19] fbcon: Replace FBCON_FLAGS_INIT with a boolean Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 08/19] fb: Delete fb_info->queue Daniel Vetter
2022-02-10 11:38 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 09/19] fbcon: Extract fbcon_open/release helpers Daniel Vetter
2022-02-10 11:46 ` Thomas Zimmermann
2022-04-05 8:45 ` Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 10/19] fbcon: Ditch error handling for con2fb_release_oldinfo Daniel Vetter
2022-02-10 14:14 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 11/19] fbcon: move more common code into fb_open() Daniel Vetter
2022-02-10 14:16 ` Thomas Zimmermann
2022-02-08 21:08 ` [PATCH v2 12/19] fbcon: use lock_fb_info in fbcon_open/release Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 13/19] fbcon: Consistently protect deferred_takeover with console_lock() Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 14/19] fbcon: Move console_lock for register/unlink/unregister Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 15/19] fbcon: Move more code into fbcon_release Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 16/19] fbcon: untangle fbcon_exit Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 17/19] fbcon: Maintain a private array of fb_info Daniel Vetter
2022-02-08 21:08 ` [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered" Daniel Vetter
2022-02-09 0:19 ` Javier Martinez Canillas
2022-04-05 8:36 ` Daniel Vetter
2022-04-05 8:40 ` Daniel Vetter
2022-04-05 9:19 ` Javier Martinez Canillas
2022-04-05 9:24 ` Daniel Vetter
2022-04-05 9:52 ` Javier Martinez Canillas
2022-04-05 10:34 ` Daniel Vetter
2022-04-05 13:24 ` Geert Uytterhoeven
2022-04-05 13:33 ` Greg KH
2022-04-05 16:12 ` Daniel Vetter
2022-04-05 16:44 ` Greg KH
2022-04-05 17:29 ` Daniel Vetter
2022-04-07 17:26 ` Greg KH
2022-04-05 13:25 ` Javier Martinez Canillas
2022-02-08 21:08 ` [PATCH v2 19/19] fbdev: Make registered_fb[] private to fbmem.c Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e15718d3-3ff3-8e3e-ca30-a7470e9f9574@suse.de \
--to=tzimmermann@suse.de \
--cc=cssk@net-c.es \
--cc=daniel.vetter@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=daniel@ffwll.ch \
--cc=deller@gmx.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=ducheng2@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=sam@ravnborg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).