* [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-04-26 14:59 ` Grzegorz Halat 0 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-04-26 14:59 UTC (permalink / raw) To: linux-kernel Cc: Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev, Oleksandr Natalenko, Grzegorz Halat After memory allocation failure vc_allocate() doesn't clean up data which has been initialized in visual_init(). In case of fbcon this leads to divide-by-0 in fbcon_init() on next open of the same tty. memory allocation in vc_allocate() may fail here: 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); on next open() fbcon_init() skips vc_font.data initialization: 1088: if (!p->fontdata) { division by zero in fbcon_init() happens here: 1149: new_cols /= vc->vc_font.width; Additional check is needed in fbcon_deinit() to prevent usage of uninitialized vc_screenbuf: 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) 1252: set_vc_hi_font(vc, false); Crash: #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 [exception RIP: fbcon_init+463] RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 ... #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 ... Signed-off-by: Grzegorz Halat <ghalat@redhat.com> --- drivers/tty/vt/vt.c | 11 +++++++++-- drivers/video/fbdev/core/fbcon.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 650c66886c80..ec85d195678f 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1056,6 +1056,13 @@ static void visual_init(struct vc_data *vc, int num, int init) vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; } + +static void visual_deinit(struct vc_data *vc) +{ + vc->vc_sw->con_deinit(vc); + module_put(vc->vc_sw->owner); +} + int vc_allocate(unsigned int currcons) /* return 0 on success */ { struct vt_notifier_param param; @@ -1103,6 +1110,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ return 0; err_free: + visual_deinit(vc); kfree(vc); vc_cons[currcons].d = NULL; return -ENOMEM; @@ -1331,9 +1339,8 @@ struct vc_data *vc_deallocate(unsigned int currcons) param.vc = vc = vc_cons[currcons].d; atomic_notifier_call_chain(&vt_notifier_list, VT_DEALLOCATE, ¶m); vcs_remove_sysfs(currcons); - vc->vc_sw->con_deinit(vc); + visual_deinit(vc); put_pid(vc->vt_pid); - module_put(vc->vc_sw->owner); vc_uniscr_set(vc, NULL); kfree(vc->vc_screenbuf); vc_cons[currcons].d = NULL; diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index cd059a801662..c59b23f6e9ba 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1248,7 +1248,7 @@ static void fbcon_deinit(struct vc_data *vc) if (free_font) vc->vc_font.data = NULL; - if (vc->vc_hi_font_mask) + if (vc->vc_hi_font_mask && vc->vc_screenbuf) set_vc_hi_font(vc, false); if (!con_is_bound(&fb_con)) -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-04-26 14:59 ` Grzegorz Halat 0 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-04-26 14:59 UTC (permalink / raw) To: linux-kernel Cc: Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev, Oleksandr Natalenko, Grzegorz Halat After memory allocation failure vc_allocate() doesn't clean up data which has been initialized in visual_init(). In case of fbcon this leads to divide-by-0 in fbcon_init() on next open of the same tty. memory allocation in vc_allocate() may fail here: 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); on next open() fbcon_init() skips vc_font.data initialization: 1088: if (!p->fontdata) { division by zero in fbcon_init() happens here: 1149: new_cols /= vc->vc_font.width; Additional check is needed in fbcon_deinit() to prevent usage of uninitialized vc_screenbuf: 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) 1252: set_vc_hi_font(vc, false); Crash: #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 [exception RIP: fbcon_init+463] RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 ... #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 ... Signed-off-by: Grzegorz Halat <ghalat@redhat.com> --- drivers/tty/vt/vt.c | 11 +++++++++-- drivers/video/fbdev/core/fbcon.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 650c66886c80..ec85d195678f 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1056,6 +1056,13 @@ static void visual_init(struct vc_data *vc, int num, int init) vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; } + +static void visual_deinit(struct vc_data *vc) +{ + vc->vc_sw->con_deinit(vc); + module_put(vc->vc_sw->owner); +} + int vc_allocate(unsigned int currcons) /* return 0 on success */ { struct vt_notifier_param param; @@ -1103,6 +1110,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ return 0; err_free: + visual_deinit(vc); kfree(vc); vc_cons[currcons].d = NULL; return -ENOMEM; @@ -1331,9 +1339,8 @@ struct vc_data *vc_deallocate(unsigned int currcons) param.vc = vc = vc_cons[currcons].d; atomic_notifier_call_chain(&vt_notifier_list, VT_DEALLOCATE, ¶m); vcs_remove_sysfs(currcons); - vc->vc_sw->con_deinit(vc); + visual_deinit(vc); put_pid(vc->vt_pid); - module_put(vc->vc_sw->owner); vc_uniscr_set(vc, NULL); kfree(vc->vc_screenbuf); vc_cons[currcons].d = NULL; diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index cd059a801662..c59b23f6e9ba 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1248,7 +1248,7 @@ static void fbcon_deinit(struct vc_data *vc) if (free_font) vc->vc_font.data = NULL; - if (vc->vc_hi_font_mask) + if (vc->vc_hi_font_mask && vc->vc_screenbuf) set_vc_hi_font(vc, false); if (!con_is_bound(&fb_con)) -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation 2019-04-26 14:59 ` Grzegorz Halat @ 2019-05-16 14:33 ` Grzegorz Halat -1 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-05-16 14:33 UTC (permalink / raw) To: linux-kernel, linux-fbdev Cc: Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, Jiri Slaby, Oleksandr Natalenko On Fri, 26 Apr 2019 at 16:59, Grzegorz Halat <ghalat@redhat.com> wrote: > > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. Hi, A gentle reminder. Could you please review my patch? I've seen two crashes caused by this bug. -- Grzegorz Halat ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-16 14:33 ` Grzegorz Halat 0 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-05-16 14:33 UTC (permalink / raw) To: linux-kernel, linux-fbdev Cc: Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, Jiri Slaby, Oleksandr Natalenko On Fri, 26 Apr 2019 at 16:59, Grzegorz Halat <ghalat@redhat.com> wrote: > > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. Hi, A gentle reminder. Could you please review my patch? I've seen two crashes caused by this bug. -- Grzegorz Halat ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation 2019-05-16 14:33 ` Grzegorz Halat @ 2019-05-24 8:06 ` Greg Kroah-Hartman -1 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-05-24 8:06 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Thu, May 16, 2019 at 04:33:40PM +0200, Grzegorz Halat wrote: > On Fri, 26 Apr 2019 at 16:59, Grzegorz Halat <ghalat@redhat.com> wrote: > > > > After memory allocation failure vc_allocate() doesn't clean up data > > which has been initialized in visual_init(). In case of fbcon this > > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > Hi, > A gentle reminder. Could you please review my patch? I've seen two > crashes caused by this bug. How? How are you triggering a memory allocation failure in a "normal" system? Anyway, I'll queue this up, but it really does not seem like anything anyone would see "in the wild". thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-24 8:06 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-05-24 8:06 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Thu, May 16, 2019 at 04:33:40PM +0200, Grzegorz Halat wrote: > On Fri, 26 Apr 2019 at 16:59, Grzegorz Halat <ghalat@redhat.com> wrote: > > > > After memory allocation failure vc_allocate() doesn't clean up data > > which has been initialized in visual_init(). In case of fbcon this > > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > Hi, > A gentle reminder. Could you please review my patch? I've seen two > crashes caused by this bug. How? How are you triggering a memory allocation failure in a "normal" system? Anyway, I'll queue this up, but it really does not seem like anything anyone would see "in the wild". thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation 2019-05-24 8:06 ` Greg Kroah-Hartman @ 2019-05-24 13:52 ` Grzegorz Halat -1 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-05-24 13:52 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Fri, 24 May 2019 at 10:06, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > How? How are you triggering a memory allocation failure in a "normal" > system? > Anyway, I'll queue this up, but it really does not seem like anything > anyone would see "in the wild" I've seen this crash twice in ours customer environment under low memory conditions. There is a report in Debian bug tracker: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=804443 and LKML bug report: https://lkml.org/lkml/2017/12/18/591 -- Grzegorz ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-24 13:52 ` Grzegorz Halat 0 siblings, 0 replies; 14+ messages in thread From: Grzegorz Halat @ 2019-05-24 13:52 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Fri, 24 May 2019 at 10:06, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > How? How are you triggering a memory allocation failure in a "normal" > system? > Anyway, I'll queue this up, but it really does not seem like anything > anyone would see "in the wild" I've seen this crash twice in ours customer environment under low memory conditions. There is a report in Debian bug tracker: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug€4443 and LKML bug report: https://lkml.org/lkml/2017/12/18/591 -- Grzegorz ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation 2019-05-24 13:52 ` Grzegorz Halat @ 2019-05-24 15:08 ` Greg Kroah-Hartman -1 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-05-24 15:08 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Fri, May 24, 2019 at 03:52:31PM +0200, Grzegorz Halat wrote: > On Fri, 24 May 2019 at 10:06, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > How? How are you triggering a memory allocation failure in a "normal" > > system? > > Anyway, I'll queue this up, but it really does not seem like anything > > anyone would see "in the wild" > > I've seen this crash twice in ours customer environment under low > memory conditions. > There is a report in Debian bug tracker: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=804443 > and LKML bug report: > https://lkml.org/lkml/2017/12/18/591 Ok, now queued up to go to Linus for 5.2-final. thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-24 15:08 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-05-24 15:08 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, linux-fbdev, Bartlomiej Zolnierkiewicz, Jiri Slaby, Oleksandr Natalenko On Fri, May 24, 2019 at 03:52:31PM +0200, Grzegorz Halat wrote: > On Fri, 24 May 2019 at 10:06, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > How? How are you triggering a memory allocation failure in a "normal" > > system? > > Anyway, I'll queue this up, but it really does not seem like anything > > anyone would see "in the wild" > > I've seen this crash twice in ours customer environment under low > memory conditions. > There is a report in Debian bug tracker: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug€4443 > and LKML bug report: > https://lkml.org/lkml/2017/12/18/591 Ok, now queued up to go to Linus for 5.2-final. thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation 2019-04-26 14:59 ` Grzegorz Halat @ 2019-05-17 11:24 ` Bartlomiej Zolnierkiewicz -1 siblings, 0 replies; 14+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2019-05-17 11:24 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev, Oleksandr Natalenko On 4/26/19 4:59 PM, Grzegorz Halat wrote: > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > memory allocation in vc_allocate() may fail here: > 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); > > on next open() fbcon_init() skips vc_font.data initialization: > 1088: if (!p->fontdata) { > > division by zero in fbcon_init() happens here: > 1149: new_cols /= vc->vc_font.width; > > Additional check is needed in fbcon_deinit() to prevent > usage of uninitialized vc_screenbuf: > > 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) > 1252: set_vc_hi_font(vc, false); > > Crash: > > #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 > [exception RIP: fbcon_init+463] > RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 > ... > #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e > #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c > #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 > ... > > Signed-off-by: Grzegorz Halat <ghalat@redhat.com> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-17 11:24 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 14+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2019-05-17 11:24 UTC (permalink / raw) To: Grzegorz Halat Cc: linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev, Oleksandr Natalenko On 4/26/19 4:59 PM, Grzegorz Halat wrote: > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > memory allocation in vc_allocate() may fail here: > 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); > > on next open() fbcon_init() skips vc_font.data initialization: > 1088: if (!p->fontdata) { > > division by zero in fbcon_init() happens here: > 1149: new_cols /= vc->vc_font.width; > > Additional check is needed in fbcon_deinit() to prevent > usage of uninitialized vc_screenbuf: > > 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) > 1252: set_vc_hi_font(vc, false); > > Crash: > > #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 > [exception RIP: fbcon_init+463] > RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 > ... > #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e > #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c > #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 > ... > > Signed-off-by: Grzegorz Halat <ghalat@redhat.com> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20190426144357.25826-1-ghalat@redhat.com>]
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation [not found] <20190426144357.25826-1-ghalat@redhat.com> @ 2019-05-21 6:27 ` Oleksandr Natalenko 0 siblings, 0 replies; 14+ messages in thread From: Oleksandr Natalenko @ 2019-05-21 6:27 UTC (permalink / raw) To: Grzegorz Halat Cc: Bartlomiej Zolnierkiewicz, linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev Hi. On Fri, Apr 26, 2019 at 04:43:57PM +0200, Grzegorz Halat wrote: > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > memory allocation in vc_allocate() may fail here: > 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); > > on next open() fbcon_init() skips vc_font.data initialization: > 1088: if (!p->fontdata) { > > division by zero in fbcon_init() happens here: > 1149: new_cols /= vc->vc_font.width; > > Additional check is needed in fbcon_deinit() to prevent > usage of uninitialized vc_screenbuf: > > 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) > 1252: set_vc_hi_font(vc, false); > > Crash: > > #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 > [exception RIP: fbcon_init+463] > RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 > ... > #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e > #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c > #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 > ... > > Signed-off-by: Grzegorz Halat <ghalat@redhat.com> > --- > drivers/tty/vt/vt.c | 11 +++++++++-- > drivers/video/fbdev/core/fbcon.c | 2 +- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 650c66886c80..ec85d195678f 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -1056,6 +1056,13 @@ static void visual_init(struct vc_data *vc, int num, int init) > vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; > } > > + > +static void visual_deinit(struct vc_data *vc) > +{ > + vc->vc_sw->con_deinit(vc); > + module_put(vc->vc_sw->owner); > +} > + > int vc_allocate(unsigned int currcons) /* return 0 on success */ > { > struct vt_notifier_param param; > @@ -1103,6 +1110,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ > > return 0; > err_free: > + visual_deinit(vc); > kfree(vc); > vc_cons[currcons].d = NULL; > return -ENOMEM; > @@ -1331,9 +1339,8 @@ struct vc_data *vc_deallocate(unsigned int currcons) > param.vc = vc = vc_cons[currcons].d; > atomic_notifier_call_chain(&vt_notifier_list, VT_DEALLOCATE, ¶m); > vcs_remove_sysfs(currcons); > - vc->vc_sw->con_deinit(vc); > + visual_deinit(vc); > put_pid(vc->vt_pid); > - module_put(vc->vc_sw->owner); > vc_uniscr_set(vc, NULL); > kfree(vc->vc_screenbuf); > vc_cons[currcons].d = NULL; > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index cd059a801662..c59b23f6e9ba 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1248,7 +1248,7 @@ static void fbcon_deinit(struct vc_data *vc) > if (free_font) > vc->vc_font.data = NULL; > > - if (vc->vc_hi_font_mask) > + if (vc->vc_hi_font_mask && vc->vc_screenbuf) > set_vc_hi_font(vc, false); > > if (!con_is_bound(&fb_con)) > -- > 2.20.1 > LGTM. Reviewed-by: Oleksandr Natalenko <oleksandr@redhat.com> -- Best regards, Oleksandr Natalenko (post-factum) Senior Software Maintenance Engineer ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation @ 2019-05-21 6:27 ` Oleksandr Natalenko 0 siblings, 0 replies; 14+ messages in thread From: Oleksandr Natalenko @ 2019-05-21 6:27 UTC (permalink / raw) To: Grzegorz Halat Cc: Bartlomiej Zolnierkiewicz, linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-fbdev Hi. On Fri, Apr 26, 2019 at 04:43:57PM +0200, Grzegorz Halat wrote: > After memory allocation failure vc_allocate() doesn't clean up data > which has been initialized in visual_init(). In case of fbcon this > leads to divide-by-0 in fbcon_init() on next open of the same tty. > > memory allocation in vc_allocate() may fail here: > 1097: vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); > > on next open() fbcon_init() skips vc_font.data initialization: > 1088: if (!p->fontdata) { > > division by zero in fbcon_init() happens here: > 1149: new_cols /= vc->vc_font.width; > > Additional check is needed in fbcon_deinit() to prevent > usage of uninitialized vc_screenbuf: > > 1251: if (vc->vc_hi_font_mask && vc->vc_screenbuf) > 1252: set_vc_hi_font(vc, false); > > Crash: > > #6 [ffffc90001eafa60] divide_error at ffffffff81a00be4 > [exception RIP: fbcon_init+463] > RIP: ffffffff814b860f RSP: ffffc90001eafb18 RFLAGS: 00010246 > ... > #7 [ffffc90001eafb60] visual_init at ffffffff8154c36e > #8 [ffffc90001eafb80] vc_allocate at ffffffff8154f53c > #9 [ffffc90001eafbc8] con_install at ffffffff8154f624 > ... > > Signed-off-by: Grzegorz Halat <ghalat@redhat.com> > --- > drivers/tty/vt/vt.c | 11 +++++++++-- > drivers/video/fbdev/core/fbcon.c | 2 +- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 650c66886c80..ec85d195678f 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -1056,6 +1056,13 @@ static void visual_init(struct vc_data *vc, int num, int init) > vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; > } > > + > +static void visual_deinit(struct vc_data *vc) > +{ > + vc->vc_sw->con_deinit(vc); > + module_put(vc->vc_sw->owner); > +} > + > int vc_allocate(unsigned int currcons) /* return 0 on success */ > { > struct vt_notifier_param param; > @@ -1103,6 +1110,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ > > return 0; > err_free: > + visual_deinit(vc); > kfree(vc); > vc_cons[currcons].d = NULL; > return -ENOMEM; > @@ -1331,9 +1339,8 @@ struct vc_data *vc_deallocate(unsigned int currcons) > param.vc = vc = vc_cons[currcons].d; > atomic_notifier_call_chain(&vt_notifier_list, VT_DEALLOCATE, ¶m); > vcs_remove_sysfs(currcons); > - vc->vc_sw->con_deinit(vc); > + visual_deinit(vc); > put_pid(vc->vt_pid); > - module_put(vc->vc_sw->owner); > vc_uniscr_set(vc, NULL); > kfree(vc->vc_screenbuf); > vc_cons[currcons].d = NULL; > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index cd059a801662..c59b23f6e9ba 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1248,7 +1248,7 @@ static void fbcon_deinit(struct vc_data *vc) > if (free_font) > vc->vc_font.data = NULL; > > - if (vc->vc_hi_font_mask) > + if (vc->vc_hi_font_mask && vc->vc_screenbuf) > set_vc_hi_font(vc, false); > > if (!con_is_bound(&fb_con)) > -- > 2.20.1 > LGTM. Reviewed-by: Oleksandr Natalenko <oleksandr@redhat.com> -- Best regards, Oleksandr Natalenko (post-factum) Senior Software Maintenance Engineer ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-24 15:08 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190426145959epcas3p452b4b80025c58916331820abbb0060ed@epcas3p4.samsung.com>
2019-04-26 14:59 ` [PATCH] vt/fbcon: deinitialize resources in visual_init() after failed memory allocation Grzegorz Halat
2019-04-26 14:59 ` Grzegorz Halat
2019-05-16 14:33 ` Grzegorz Halat
2019-05-16 14:33 ` Grzegorz Halat
2019-05-24 8:06 ` Greg Kroah-Hartman
2019-05-24 8:06 ` Greg Kroah-Hartman
2019-05-24 13:52 ` Grzegorz Halat
2019-05-24 13:52 ` Grzegorz Halat
2019-05-24 15:08 ` Greg Kroah-Hartman
2019-05-24 15:08 ` Greg Kroah-Hartman
2019-05-17 11:24 ` Bartlomiej Zolnierkiewicz
2019-05-17 11:24 ` Bartlomiej Zolnierkiewicz
[not found] <20190426144357.25826-1-ghalat@redhat.com>
2019-05-21 6:27 ` Oleksandr Natalenko
2019-05-21 6:27 ` Oleksandr Natalenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.