* 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 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).