2022-02-09 07:13:45

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH v2 06/19] fbcon: Use delayed work for cursor

Allows us to delete a bunch of hand-rolled stuff. Also to simplify the
code we initialize the cursor_work completely when we allocate the
fbcon_ops structure, instead of trying to cope with console
re-initialization.

The motiviation here is that fbcon code stops using the fb_info.queue,
which helps with locking issues around cleanup and all that in a later
patch.

Also note that this allows us to ditch the hand-rolled work cleanup in
fbcon_exit - we already call fbcon_del_cursor_timer, which takes care
of everything. Plus this was racy anyway.

Signed-off-by: Daniel Vetter <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Claudio Suarez <[email protected]>
Cc: Du Cheng <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Tetsuo Handa <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 85 +++++++++++++-------------------
drivers/video/fbdev/core/fbcon.h | 4 +-
2 files changed, 35 insertions(+), 54 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 83f0223f5333..a368ed602e2e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -350,8 +350,8 @@ static int get_color(struct vc_data *vc, struct fb_info *info,

static void fb_flashcursor(struct work_struct *work)
{
- struct fb_info *info = container_of(work, struct fb_info, queue);
- struct fbcon_ops *ops = info->fbcon_par;
+ struct fbcon_ops *ops = container_of(work, struct fbcon_ops, cursor_work.work);
+ struct fb_info *info;
struct vc_data *vc = NULL;
int c;
int mode;
@@ -364,7 +364,10 @@ static void fb_flashcursor(struct work_struct *work)
if (ret == 0)
return;

- if (ops && ops->currcon != -1)
+ /* protected by console_lock */
+ info = ops->info;
+
+ if (ops->currcon != -1)
vc = vc_cons[ops->currcon].d;

if (!vc || !con_is_visible(vc) ||
@@ -380,42 +383,25 @@ static void fb_flashcursor(struct work_struct *work)
ops->cursor(vc, info, mode, get_color(vc, info, c, 1),
get_color(vc, info, c, 0));
console_unlock();
-}

-static void cursor_timer_handler(struct timer_list *t)
-{
- struct fbcon_ops *ops = from_timer(ops, t, cursor_timer);
- struct fb_info *info = ops->info;
-
- queue_work(system_power_efficient_wq, &info->queue);
- mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
+ queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
+ ops->cur_blink_jiffies);
}

-static void fbcon_add_cursor_timer(struct fb_info *info)
+static void fbcon_add_cursor_work(struct fb_info *info)
{
struct fbcon_ops *ops = info->fbcon_par;

- if ((!info->queue.func || info->queue.func == fb_flashcursor) &&
- !(ops->flags & FBCON_FLAGS_CURSOR_TIMER) &&
- !fbcon_cursor_noblink) {
- if (!info->queue.func)
- INIT_WORK(&info->queue, fb_flashcursor);
-
- timer_setup(&ops->cursor_timer, cursor_timer_handler, 0);
- mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
- ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
- }
+ if (!fbcon_cursor_noblink)
+ queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
+ ops->cur_blink_jiffies);
}

-static void fbcon_del_cursor_timer(struct fb_info *info)
+static void fbcon_del_cursor_work(struct fb_info *info)
{
struct fbcon_ops *ops = info->fbcon_par;

- if (info->queue.func == fb_flashcursor &&
- ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
- del_timer_sync(&ops->cursor_timer);
- ops->flags &= ~FBCON_FLAGS_CURSOR_TIMER;
- }
+ cancel_delayed_work_sync(&ops->cursor_work);
}

#ifndef MODULE
@@ -714,6 +700,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
if (!ops)
err = -ENOMEM;
+
+ INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
}

if (!err) {
@@ -751,7 +739,7 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
}

if (!err) {
- fbcon_del_cursor_timer(oldinfo);
+ fbcon_del_cursor_work(oldinfo);
kfree(ops->cursor_state.mask);
kfree(ops->cursor_data);
kfree(ops->cursor_src);
@@ -867,7 +855,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
logo_shown != FBCON_LOGO_DONTSHOW);

if (!found)
- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);
con2fb_map_boot[unit] = newidx;
con2fb_init_display(vc, info, unit, show_logo);
}
@@ -964,6 +952,8 @@ static const char *fbcon_startup(void)
return NULL;
}

+ INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
+
ops->currcon = -1;
ops->graphics = 1;
ops->cur_rotate = -1;
@@ -1006,7 +996,7 @@ static const char *fbcon_startup(void)
info->var.yres,
info->var.bits_per_pixel);

- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);
return display_desc;
}

@@ -1194,7 +1184,7 @@ static void fbcon_deinit(struct vc_data *vc)
goto finished;

if (con_is_visible(vc))
- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);

ops->flags &= ~FBCON_FLAGS_INIT;
finished:
@@ -1320,9 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
return;

if (vc->vc_cursor_type & CUR_SW)
- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);
else
- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);

ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;

@@ -2132,14 +2122,14 @@ static int fbcon_switch(struct vc_data *vc)
}

if (old_info != info)
- fbcon_del_cursor_timer(old_info);
+ fbcon_del_cursor_work(old_info);
}

if (fbcon_is_inactive(vc, info) ||
ops->blank_state != FB_BLANK_UNBLANK)
- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);
else
- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);

set_blitting_type(vc, info);
ops->cursor_reset = 1;
@@ -2247,9 +2237,9 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)

if (mode_switch || fbcon_is_inactive(vc, info) ||
ops->blank_state != FB_BLANK_UNBLANK)
- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);
else
- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);

return 0;
}
@@ -3181,7 +3171,7 @@ static ssize_t show_cursor_blink(struct device *device,
if (!ops)
goto err;

- blink = (ops->flags & FBCON_FLAGS_CURSOR_TIMER) ? 1 : 0;
+ blink = delayed_work_pending(&ops->cursor_work);
err:
console_unlock();
return snprintf(buf, PAGE_SIZE, "%d\n", blink);
@@ -3210,10 +3200,10 @@ static ssize_t store_cursor_blink(struct device *device,

if (blink) {
fbcon_cursor_noblink = 0;
- fbcon_add_cursor_timer(info);
+ fbcon_add_cursor_work(info);
} else {
fbcon_cursor_noblink = 1;
- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);
}

err:
@@ -3314,15 +3304,9 @@ static void fbcon_exit(void)
#endif

for_each_registered_fb(i) {
- int pending = 0;
-
mapped = 0;
info = registered_fb[i];

- if (info->queue.func)
- pending = cancel_work_sync(&info->queue);
- pr_debug("fbcon: %s pending work\n", (pending ? "canceled" : "no"));
-
for (j = first_fb_vc; j <= last_fb_vc; j++) {
if (con2fb_map[j] == i) {
mapped = 1;
@@ -3338,15 +3322,12 @@ static void fbcon_exit(void)
if (info->fbcon_par) {
struct fbcon_ops *ops = info->fbcon_par;

- fbcon_del_cursor_timer(info);
+ fbcon_del_cursor_work(info);
kfree(ops->cursor_src);
kfree(ops->cursor_state.mask);
kfree(info->fbcon_par);
info->fbcon_par = NULL;
}
-
- if (info->queue.func == fb_flashcursor)
- info->queue.func = NULL;
}
}
}
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 969d41ecede5..6708ca0048aa 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -14,11 +14,11 @@
#include <linux/types.h>
#include <linux/vt_buffer.h>
#include <linux/vt_kern.h>
+#include <linux/workqueue.h>

#include <asm/io.h>

#define FBCON_FLAGS_INIT 1
-#define FBCON_FLAGS_CURSOR_TIMER 2

/*
* This is the interface between the low-level console driver and the
@@ -68,7 +68,7 @@ struct fbcon_ops {
int (*update_start)(struct fb_info *info);
int (*rotate_font)(struct fb_info *info, struct vc_data *vc);
struct fb_var_screeninfo var; /* copy of the current fb_var_screeninfo */
- struct timer_list cursor_timer; /* Cursor timer */
+ struct delayed_work cursor_work; /* Cursor timer */
struct fb_cursor cursor_state;
struct fbcon_display *p;
struct fb_info *info;
--
2.34.1



2022-02-09 08:21:13

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v2 06/19] fbcon: Use delayed work for cursor

Hello Daniel,

On 2/8/22 22:08, Daniel Vetter wrote:
> Allows us to delete a bunch of hand-rolled stuff. Also to simplify the
> code we initialize the cursor_work completely when we allocate the
> fbcon_ops structure, instead of trying to cope with console
> re-initialization.
>

Maybe also make it more explicit in the commit message that the delayed
work is replacing a timer that was used before for the cursor ?

> The motiviation here is that fbcon code stops using the fb_info.queue,

motivation

[snip]

> /*
> * This is the interface between the low-level console driver and the
> @@ -68,7 +68,7 @@ struct fbcon_ops {
> int (*update_start)(struct fb_info *info);
> int (*rotate_font)(struct fb_info *info, struct vc_data *vc);
> struct fb_var_screeninfo var; /* copy of the current fb_var_screeninfo */
> - struct timer_list cursor_timer; /* Cursor timer */
> + struct delayed_work cursor_work; /* Cursor timer */

A delayed_work uses a timer underneath but I wonder if the comment also
needs to be updated since technically isn't a timer anymore but deferred
work that gets re-scheduled each time on fb_flashcursor().

The patch looks good to me and makes the logic much simpler than before.

Reviewed-by: Javier Martinez Canillas <[email protected]>

Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat


2022-02-10 12:37:34

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH v2 06/19] fbcon: Use delayed work for cursor

On 2022/02/09 6:08, Daniel Vetter wrote:
> @@ -714,6 +700,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> if (!ops)
> err = -ENOMEM;
> +
> + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> }
>
> if (!err) {

Memory allocation fault injection will hit NULL pointer dereference.

2022-02-10 16:47:11

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH v2 06/19] fbcon: Use delayed work for cursor



Am 08.02.22 um 22:08 schrieb Daniel Vetter:
> Allows us to delete a bunch of hand-rolled stuff. Also to simplify the
> code we initialize the cursor_work completely when we allocate the
> fbcon_ops structure, instead of trying to cope with console
> re-initialization.
>
> The motiviation here is that fbcon code stops using the fb_info.queue,
> which helps with locking issues around cleanup and all that in a later
> patch.
>
> Also note that this allows us to ditch the hand-rolled work cleanup in
> fbcon_exit - we already call fbcon_del_cursor_timer, which takes care
> of everything. Plus this was racy anyway.
>
> Signed-off-by: Daniel Vetter <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Claudio Suarez <[email protected]>
> Cc: Du Cheng <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Tetsuo Handa <[email protected]>
> ---
> drivers/video/fbdev/core/fbcon.c | 85 +++++++++++++-------------------
> drivers/video/fbdev/core/fbcon.h | 4 +-
> 2 files changed, 35 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 83f0223f5333..a368ed602e2e 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -350,8 +350,8 @@ static int get_color(struct vc_data *vc, struct fb_info *info,
>
> static void fb_flashcursor(struct work_struct *work)
> {
> - struct fb_info *info = container_of(work, struct fb_info, queue);
> - struct fbcon_ops *ops = info->fbcon_par;
> + struct fbcon_ops *ops = container_of(work, struct fbcon_ops, cursor_work.work);
> + struct fb_info *info;
> struct vc_data *vc = NULL;
> int c;
> int mode;
> @@ -364,7 +364,10 @@ static void fb_flashcursor(struct work_struct *work)
> if (ret == 0)
> return;
>
> - if (ops && ops->currcon != -1)
> + /* protected by console_lock */
> + info = ops->info;
> +
> + if (ops->currcon != -1)
> vc = vc_cons[ops->currcon].d;
>
> if (!vc || !con_is_visible(vc) ||
> @@ -380,42 +383,25 @@ static void fb_flashcursor(struct work_struct *work)
> ops->cursor(vc, info, mode, get_color(vc, info, c, 1),
> get_color(vc, info, c, 0));
> console_unlock();
> -}
>
> -static void cursor_timer_handler(struct timer_list *t)
> -{
> - struct fbcon_ops *ops = from_timer(ops, t, cursor_timer);
> - struct fb_info *info = ops->info;
> -
> - queue_work(system_power_efficient_wq, &info->queue);
> - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
> + queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
> + ops->cur_blink_jiffies);
> }
>
> -static void fbcon_add_cursor_timer(struct fb_info *info)
> +static void fbcon_add_cursor_work(struct fb_info *info)
> {
> struct fbcon_ops *ops = info->fbcon_par;
>
> - if ((!info->queue.func || info->queue.func == fb_flashcursor) &&
> - !(ops->flags & FBCON_FLAGS_CURSOR_TIMER) &&
> - !fbcon_cursor_noblink) {
> - if (!info->queue.func)
> - INIT_WORK(&info->queue, fb_flashcursor);
> -
> - timer_setup(&ops->cursor_timer, cursor_timer_handler, 0);
> - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
> - ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
> - }
> + if (!fbcon_cursor_noblink)
> + queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
> + ops->cur_blink_jiffies);
> }
>
> -static void fbcon_del_cursor_timer(struct fb_info *info)
> +static void fbcon_del_cursor_work(struct fb_info *info)
> {
> struct fbcon_ops *ops = info->fbcon_par;
>
> - if (info->queue.func == fb_flashcursor &&
> - ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
> - del_timer_sync(&ops->cursor_timer);
> - ops->flags &= ~FBCON_FLAGS_CURSOR_TIMER;
> - }
> + cancel_delayed_work_sync(&ops->cursor_work);
> }
>
> #ifndef MODULE
> @@ -714,6 +700,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> if (!ops)
> err = -ENOMEM;
> +
> + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);

There's similar code in fbcon_startup() when there should be a single
init function for fbcon_ops. Maybe something for later.

Acked-by: Thomas Zimmermann <[email protected]>

> }
>
> if (!err) {
> @@ -751,7 +739,7 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
> }
>
> if (!err) {
> - fbcon_del_cursor_timer(oldinfo);
> + fbcon_del_cursor_work(oldinfo);
> kfree(ops->cursor_state.mask);
> kfree(ops->cursor_data);
> kfree(ops->cursor_src);
> @@ -867,7 +855,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
> logo_shown != FBCON_LOGO_DONTSHOW);
>
> if (!found)
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
> con2fb_map_boot[unit] = newidx;
> con2fb_init_display(vc, info, unit, show_logo);
> }
> @@ -964,6 +952,8 @@ static const char *fbcon_startup(void)
> return NULL;
> }
>
> + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> +
> ops->currcon = -1;
> ops->graphics = 1;
> ops->cur_rotate = -1;
> @@ -1006,7 +996,7 @@ static const char *fbcon_startup(void)
> info->var.yres,
> info->var.bits_per_pixel);
>
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
> return display_desc;
> }
>
> @@ -1194,7 +1184,7 @@ static void fbcon_deinit(struct vc_data *vc)
> goto finished;
>
> if (con_is_visible(vc))
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
>
> ops->flags &= ~FBCON_FLAGS_INIT;
> finished:
> @@ -1320,9 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> return;
>
> if (vc->vc_cursor_type & CUR_SW)
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
> else
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
>
> ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
>
> @@ -2132,14 +2122,14 @@ static int fbcon_switch(struct vc_data *vc)
> }
>
> if (old_info != info)
> - fbcon_del_cursor_timer(old_info);
> + fbcon_del_cursor_work(old_info);
> }
>
> if (fbcon_is_inactive(vc, info) ||
> ops->blank_state != FB_BLANK_UNBLANK)
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
> else
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
>
> set_blitting_type(vc, info);
> ops->cursor_reset = 1;
> @@ -2247,9 +2237,9 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
>
> if (mode_switch || fbcon_is_inactive(vc, info) ||
> ops->blank_state != FB_BLANK_UNBLANK)
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
> else
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
>
> return 0;
> }
> @@ -3181,7 +3171,7 @@ static ssize_t show_cursor_blink(struct device *device,
> if (!ops)
> goto err;
>
> - blink = (ops->flags & FBCON_FLAGS_CURSOR_TIMER) ? 1 : 0;
> + blink = delayed_work_pending(&ops->cursor_work);
> err:
> console_unlock();
> return snprintf(buf, PAGE_SIZE, "%d\n", blink);
> @@ -3210,10 +3200,10 @@ static ssize_t store_cursor_blink(struct device *device,
>
> if (blink) {
> fbcon_cursor_noblink = 0;
> - fbcon_add_cursor_timer(info);
> + fbcon_add_cursor_work(info);
> } else {
> fbcon_cursor_noblink = 1;
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
> }
>
> err:
> @@ -3314,15 +3304,9 @@ static void fbcon_exit(void)
> #endif
>
> for_each_registered_fb(i) {
> - int pending = 0;
> -
> mapped = 0;
> info = registered_fb[i];
>
> - if (info->queue.func)
> - pending = cancel_work_sync(&info->queue);
> - pr_debug("fbcon: %s pending work\n", (pending ? "canceled" : "no"));
> -
> for (j = first_fb_vc; j <= last_fb_vc; j++) {
> if (con2fb_map[j] == i) {
> mapped = 1;
> @@ -3338,15 +3322,12 @@ static void fbcon_exit(void)
> if (info->fbcon_par) {
> struct fbcon_ops *ops = info->fbcon_par;
>
> - fbcon_del_cursor_timer(info);
> + fbcon_del_cursor_work(info);
> kfree(ops->cursor_src);
> kfree(ops->cursor_state.mask);
> kfree(info->fbcon_par);
> info->fbcon_par = NULL;
> }
> -
> - if (info->queue.func == fb_flashcursor)
> - info->queue.func = NULL;
> }
> }
> }
> diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
> index 969d41ecede5..6708ca0048aa 100644
> --- a/drivers/video/fbdev/core/fbcon.h
> +++ b/drivers/video/fbdev/core/fbcon.h
> @@ -14,11 +14,11 @@
> #include <linux/types.h>
> #include <linux/vt_buffer.h>
> #include <linux/vt_kern.h>
> +#include <linux/workqueue.h>
>
> #include <asm/io.h>
>
> #define FBCON_FLAGS_INIT 1
> -#define FBCON_FLAGS_CURSOR_TIMER 2
>
> /*
> * This is the interface between the low-level console driver and the
> @@ -68,7 +68,7 @@ struct fbcon_ops {
> int (*update_start)(struct fb_info *info);
> int (*rotate_font)(struct fb_info *info, struct vc_data *vc);
> struct fb_var_screeninfo var; /* copy of the current fb_var_screeninfo */
> - struct timer_list cursor_timer; /* Cursor timer */
> + struct delayed_work cursor_work; /* Cursor timer */
> struct fb_cursor cursor_state;
> struct fbcon_display *p;
> struct fb_info *info;

--
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


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-04-06 04:02:53

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2 06/19] fbcon: Use delayed work for cursor

On Thu, Feb 10, 2022 at 08:43:36PM +0900, Tetsuo Handa wrote:
> On 2022/02/09 6:08, Daniel Vetter wrote:
> > @@ -714,6 +700,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> > ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > if (!ops)
> > err = -ENOMEM;
> > +
> > + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > }
> >
> > if (!err) {
>
> Memory allocation fault injection will hit NULL pointer dereference.

The error handling here is convoluted and I got this wrong, but a later
patch to extract an fbcon_open() helper fixes it. I'll fix this small
bisect issue for v3 anyway, thanks for taking a look at the patches.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch