* the patch "vt: perform safe console erase" introduces a bug
@ 2019-03-30 18:51 Mikulas Patocka
2019-03-31 20:18 ` Mikulas Patocka
0 siblings, 1 reply; 6+ messages in thread
From: Mikulas Patocka @ 2019-03-30 18:51 UTC (permalink / raw)
To: Nicolas Pitre, Matthew Whitehead; +Cc: Greg Kroah-Hartman, stable
Hi
The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe
console erase in the right order") introduces a bug.
In order to reproduce the bug
- use framebuffer console with the AMDGPU driver
- type "links" to start the console www browser
- press 'q' and space to exit links
--- now, the cursor line will be permanently visible in the center of the
screen. It will stay there until something overwrites it.
Before the patch, there was a call to do_update_region, the patch changes
it to update_region - and this seems to cause the bug with the cursor.
The bug goes away if we change update_region back to do_update_region.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order")
---
drivers/tty/vt/vt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-5.0.5/drivers/tty/vt/vt.c
===================================================================
--- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100
+++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100
@@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in
return;
}
scr_memsetw(start, vc->vc_video_erase_char, 2 * count);
- update_region(vc, (unsigned long) start, count);
+ do_update_region(vc, (unsigned long) start, count);
vc->vc_need_wrap = 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: the patch "vt: perform safe console erase" introduces a bug 2019-03-30 18:51 the patch "vt: perform safe console erase" introduces a bug Mikulas Patocka @ 2019-03-31 20:18 ` Mikulas Patocka 2019-03-31 23:43 ` Nicolas Pitre 2019-04-16 13:23 ` Greg Kroah-Hartman 0 siblings, 2 replies; 6+ messages in thread From: Mikulas Patocka @ 2019-03-31 20:18 UTC (permalink / raw) To: Nicolas Pitre, Matthew Whitehead; +Cc: Greg Kroah-Hartman, stable On Sat, 30 Mar 2019, Mikulas Patocka wrote: > Hi > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > console erase in the right order") introduces a bug. > > --- > drivers/tty/vt/vt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-5.0.5/drivers/tty/vt/vt.c > =================================================================== > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100 > @@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in > return; > } > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > - update_region(vc, (unsigned long) start, count); > + do_update_region(vc, (unsigned long) start, count); ^^^^ this is wrong too - it will clear the screen if \e[2J is printed on inactive console. We need to use con_should_update(vc), just like it was before: The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe console erase in the right order") introduces a bug. In order to reproduce the bug - use framebuffer console with the AMDGPU driver - type "links" to start the console www browser - press 'q' and space to exit links --- now, the cursor line will be permanently visible in the center of the screen. It will stay there until something overwrites it. Before the patch, there was a call to do_update_region, the patch changes it to update_region - and this seems to cause the bug with the cursor. The bug goes away if we change update_region back to do_update_region. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order") --- drivers/tty/vt/vt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: linux-5.0.5/drivers/tty/vt/vt.c =================================================================== --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-31 13:06:24.000000000 +0200 @@ -1518,7 +1518,8 @@ static void csi_J(struct vc_data *vc, in return; } scr_memsetw(start, vc->vc_video_erase_char, 2 * count); - update_region(vc, (unsigned long) start, count); + if (con_should_update(vc)) + do_update_region(vc, (unsigned long) start, count); vc->vc_need_wrap = 0; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: the patch "vt: perform safe console erase" introduces a bug 2019-03-31 20:18 ` Mikulas Patocka @ 2019-03-31 23:43 ` Nicolas Pitre 2019-04-16 13:23 ` Greg Kroah-Hartman 1 sibling, 0 replies; 6+ messages in thread From: Nicolas Pitre @ 2019-03-31 23:43 UTC (permalink / raw) To: Mikulas Patocka; +Cc: Matthew Whitehead, Greg Kroah-Hartman, stable On Sun, 31 Mar 2019, Mikulas Patocka wrote: > > > On Sat, 30 Mar 2019, Mikulas Patocka wrote: > > > Hi > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > console erase in the right order") introduces a bug. > > > > --- > > drivers/tty/vt/vt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > =================================================================== > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100 > > @@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in > > return; > > } > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > - update_region(vc, (unsigned long) start, count); > > + do_update_region(vc, (unsigned long) start, count); > ^^^^ this is wrong too - it will clear the screen if \e[2J is printed on > inactive console. We need to use con_should_update(vc), just like it was > before: > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > console erase in the right order") introduces a bug. > > In order to reproduce the bug > - use framebuffer console with the AMDGPU driver > - type "links" to start the console www browser > - press 'q' and space to exit links > > --- now, the cursor line will be permanently visible in the center of the > screen. It will stay there until something overwrites it. > > Before the patch, there was a call to do_update_region, the patch changes > it to update_region - and this seems to cause the bug with the cursor. > > The bug goes away if we change update_region back to do_update_region. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Cc: stable@vger.kernel.org > Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order") The code in update_region(() does call do_update_region() conditionally on con_should_update(vc). But it does a set_cursor(vc) too which is the problem. So for the patch below: Acked-by: Nicolas Pitre <nico@fluxnic.net> However there is another unconditional call to do_update_region() in do_con_trol() (see case EShash) which might require a separate fix. > --- > drivers/tty/vt/vt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-5.0.5/drivers/tty/vt/vt.c > =================================================================== > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-31 13:06:24.000000000 +0200 > @@ -1518,7 +1518,8 @@ static void csi_J(struct vc_data *vc, in > return; > } > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > - update_region(vc, (unsigned long) start, count); > + if (con_should_update(vc)) > + do_update_region(vc, (unsigned long) start, count); > vc->vc_need_wrap = 0; > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: the patch "vt: perform safe console erase" introduces a bug 2019-03-31 20:18 ` Mikulas Patocka 2019-03-31 23:43 ` Nicolas Pitre @ 2019-04-16 13:23 ` Greg Kroah-Hartman 2019-04-16 14:53 ` Nicolas Pitre 1 sibling, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2019-04-16 13:23 UTC (permalink / raw) To: Mikulas Patocka; +Cc: Nicolas Pitre, Matthew Whitehead, stable On Sun, Mar 31, 2019 at 04:18:01PM -0400, Mikulas Patocka wrote: > > > On Sat, 30 Mar 2019, Mikulas Patocka wrote: > > > Hi > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > console erase in the right order") introduces a bug. > > > > --- > > drivers/tty/vt/vt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > =================================================================== > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100 > > @@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in > > return; > > } > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > - update_region(vc, (unsigned long) start, count); > > + do_update_region(vc, (unsigned long) start, count); > ^^^^ this is wrong too - it will clear the screen if \e[2J is printed on > inactive console. We need to use con_should_update(vc), just like it was > before: > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > console erase in the right order") introduces a bug. > > In order to reproduce the bug > - use framebuffer console with the AMDGPU driver > - type "links" to start the console www browser > - press 'q' and space to exit links > > --- now, the cursor line will be permanently visible in the center of the > screen. It will stay there until something overwrites it. > > Before the patch, there was a call to do_update_region, the patch changes > it to update_region - and this seems to cause the bug with the cursor. > > The bug goes away if we change update_region back to do_update_region. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Cc: stable@vger.kernel.org > Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order") > > --- > drivers/tty/vt/vt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-5.0.5/drivers/tty/vt/vt.c > =================================================================== > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-31 13:06:24.000000000 +0200 > @@ -1518,7 +1518,8 @@ static void csi_J(struct vc_data *vc, in > return; > } > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > - update_region(vc, (unsigned long) start, count); > + if (con_should_update(vc)) > + do_update_region(vc, (unsigned long) start, count); > vc->vc_need_wrap = 0; > } > Can you resend this, with the ack, in a format that I can apply it in? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: the patch "vt: perform safe console erase" introduces a bug 2019-04-16 13:23 ` Greg Kroah-Hartman @ 2019-04-16 14:53 ` Nicolas Pitre 2019-04-17 12:38 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Nicolas Pitre @ 2019-04-16 14:53 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Mikulas Patocka, Matthew Whitehead, stable On Tue, 16 Apr 2019, Greg Kroah-Hartman wrote: > On Sun, Mar 31, 2019 at 04:18:01PM -0400, Mikulas Patocka wrote: > > > > > > On Sat, 30 Mar 2019, Mikulas Patocka wrote: > > > > > Hi > > > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > > console erase in the right order") introduces a bug. > > > > > > --- > > > drivers/tty/vt/vt.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > > =================================================================== > > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100 > > > @@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in > > > return; > > > } > > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > > - update_region(vc, (unsigned long) start, count); > > > + do_update_region(vc, (unsigned long) start, count); > > ^^^^ this is wrong too - it will clear the screen if \e[2J is printed on > > inactive console. We need to use con_should_update(vc), just like it was > > before: > > > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > console erase in the right order") introduces a bug. > > > > In order to reproduce the bug > > - use framebuffer console with the AMDGPU driver > > - type "links" to start the console www browser > > - press 'q' and space to exit links > > > > --- now, the cursor line will be permanently visible in the center of the > > screen. It will stay there until something overwrites it. > > > > Before the patch, there was a call to do_update_region, the patch changes > > it to update_region - and this seems to cause the bug with the cursor. > > > > The bug goes away if we change update_region back to do_update_region. > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > Cc: stable@vger.kernel.org > > Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order") > > > > --- > > drivers/tty/vt/vt.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > =================================================================== > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-31 13:06:24.000000000 +0200 > > @@ -1518,7 +1518,8 @@ static void csi_J(struct vc_data *vc, in > > return; > > } > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > - update_region(vc, (unsigned long) start, count); > > + if (con_should_update(vc)) > > + do_update_region(vc, (unsigned long) start, count); > > vc->vc_need_wrap = 0; > > } > > > > Can you resend this, with the ack, in a format that I can apply it in? I did resend that patch to you with a proper changelog on April 4th. Subject is "[PATCH] fix cursor when clearing the screen". Nicolas ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: the patch "vt: perform safe console erase" introduces a bug 2019-04-16 14:53 ` Nicolas Pitre @ 2019-04-17 12:38 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2019-04-17 12:38 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Mikulas Patocka, Matthew Whitehead, stable On Tue, Apr 16, 2019 at 10:53:54AM -0400, Nicolas Pitre wrote: > On Tue, 16 Apr 2019, Greg Kroah-Hartman wrote: > > > On Sun, Mar 31, 2019 at 04:18:01PM -0400, Mikulas Patocka wrote: > > > > > > > > > On Sat, 30 Mar 2019, Mikulas Patocka wrote: > > > > > > > Hi > > > > > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > > > console erase in the right order") introduces a bug. > > > > > > > > --- > > > > drivers/tty/vt/vt.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > > > =================================================================== > > > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-30 19:30:50.000000000 +0100 > > > > @@ -1518,7 +1518,7 @@ static void csi_J(struct vc_data *vc, in > > > > return; > > > > } > > > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > > > - update_region(vc, (unsigned long) start, count); > > > > + do_update_region(vc, (unsigned long) start, count); > > > ^^^^ this is wrong too - it will clear the screen if \e[2J is printed on > > > inactive console. We need to use con_should_update(vc), just like it was > > > before: > > > > > > > > > The patch a6dbe442755999960ca54a9b8ecfd9606be0ea75 ("vt: perform safe > > > console erase in the right order") introduces a bug. > > > > > > In order to reproduce the bug > > > - use framebuffer console with the AMDGPU driver > > > - type "links" to start the console www browser > > > - press 'q' and space to exit links > > > > > > --- now, the cursor line will be permanently visible in the center of the > > > screen. It will stay there until something overwrites it. > > > > > > Before the patch, there was a call to do_update_region, the patch changes > > > it to update_region - and this seems to cause the bug with the cursor. > > > > > > The bug goes away if we change update_region back to do_update_region. > > > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > > Cc: stable@vger.kernel.org > > > Fixes: a6dbe4427559 ("vt: perform safe console erase in the right order") > > > > > > --- > > > drivers/tty/vt/vt.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > Index: linux-5.0.5/drivers/tty/vt/vt.c > > > =================================================================== > > > --- linux-5.0.5.orig/drivers/tty/vt/vt.c 2019-03-30 19:29:26.000000000 +0100 > > > +++ linux-5.0.5/drivers/tty/vt/vt.c 2019-03-31 13:06:24.000000000 +0200 > > > @@ -1518,7 +1518,8 @@ static void csi_J(struct vc_data *vc, in > > > return; > > > } > > > scr_memsetw(start, vc->vc_video_erase_char, 2 * count); > > > - update_region(vc, (unsigned long) start, count); > > > + if (con_should_update(vc)) > > > + do_update_region(vc, (unsigned long) start, count); > > > vc->vc_need_wrap = 0; > > > } > > > > > > > Can you resend this, with the ack, in a format that I can apply it in? > > I did resend that patch to you with a proper changelog on April 4th. > Subject is "[PATCH] fix cursor when clearing the screen". Ah, sorry, I see it now, it's burried in my "to-review" queue. Will go apply it now, thanks! greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-17 12:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-30 18:51 the patch "vt: perform safe console erase" introduces a bug Mikulas Patocka 2019-03-31 20:18 ` Mikulas Patocka 2019-03-31 23:43 ` Nicolas Pitre 2019-04-16 13:23 ` Greg Kroah-Hartman 2019-04-16 14:53 ` Nicolas Pitre 2019-04-17 12:38 ` Greg Kroah-Hartman
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.