Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1562707 > unrolled thread

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

Started byGreg KH <gregkh@linuxfoundation.org>
First post2017-01-19 14:30 +0100
Last post2017-01-19 20:20 +0100
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Greg KH <gregkh@linuxfoundation.org> - 2017-01-19 14:30 +0100
    Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Manuel Schölling <manuel.schoelling@gmx.de> - 2017-01-19 17:20 +0100
      Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Greg KH <gregkh@linuxfoundation.org> - 2017-01-19 17:50 +0100
        Bug#845177: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles Adam Borowski <kilobyte@angband.pl> - 2017-01-20 00:10 +0100
          Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Adam Borowski <kilobyte@angband.pl> - 2017-01-20 14:20 +0100
            Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Greg KH <gregkh@linuxfoundation.org> - 2017-01-20 14:40 +0100
              Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Adam Borowski <kilobyte@angband.pl> - 2017-01-20 15:30 +0100
      Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for  all VGA consoles Adam Borowski <kilobyte@angband.pl> - 2017-01-19 20:20 +0100

#1562707 — Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

FromGreg KH <gregkh@linuxfoundation.org>
Date2017-01-19 14:30 +0100
SubjectRe: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles
Message-ID<t1kWS-23z-29@gated-at.bofh.it>
On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> Add a scrollback buffers for each VGA console. The benefit is that
> the scrollback history is not flushed when switching between consoles
> but is persistent.
> The buffers are allocated on demand when a new console is opened.
> 
> This breaks tools like clear_console that rely on flushing the
> scrollback history by switching back and forth between consoles
> which is why this feature is disabled by default.
> Use the escape sequence \e[3J instead for flushing the buffer.
> 
> Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de>
> Reviewed-by: Andrey Utkin <andrey_utkin@fastmail.com>
> Tested-by: Andrey Utkin <andrey_utkin@fastmail.com>
> Tested-by: Adam Borowski <kilobyte@angband.pl>
> ---
>  drivers/video/console/Kconfig  |  25 +++++++-
>  drivers/video/console/vgacon.c | 142 ++++++++++++++++++++++++++---------------
>  2 files changed, 111 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index c3f1fb9ee820..f500e58f7636 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -43,9 +43,28 @@ config VGACON_SOFT_SCROLLBACK_SIZE
>         range 1 1024
>         default "64"
>         help
> -         Enter the amount of System RAM to allocate for the scrollback
> -	 buffer.  Each 64KB will give you approximately 16 80x25
> -	 screenfuls of scrollback buffer
> +	  Enter the amount of System RAM to allocate for scrollback
> +	  buffers of VGA consoles. Each 64KB will give you approximately
> +	  16 80x25 screenfuls of scrollback buffer.
> +
> +config VGACON_SOFT_SCROLLBACK_PERSISTENT
> +	bool "Persistent Scrollback History for each console"
> +	depends on VGACON_SOFT_SCROLLBACK
> +	default n
> +	help
> +	  Say Y here if the scrollback history should persist when switching
> +	  between consoles. Otherwise, the scrollback history will be flushed
> +	  each time the console is switched.
> +
> +	  This feature might break your tool of choice to flush the scrollback
> +	  buffer, e.g. clear(1) will work fine but Debian's clear_console(1)
> +	  will be broken, which might cause security issues.
> +	  You can use the escape sequence \e[3J instead if this feature is
> +	  activated.

This issue is the one that makes me the most worried.  Why doesn't
clear_console() work anymore?  Why doesn't it use \e[3J ?

thanks,

greg k-h

[toc] | [next] | [standalone]


#1562867

FromManuel Schölling <manuel.schoelling@gmx.de>
Date2017-01-19 17:20 +0100
Message-ID<t1nBo-3Mq-31@gated-at.bofh.it>
In reply to#1562707
On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > Add a scrollback buffers for each VGA console. The benefit is that
> > the scrollback history is not flushed when switching between
> > consoles
> > but is persistent.
> > The buffers are allocated on demand when a new console is opened.
> > 
> > This breaks tools like clear_console that rely on flushing the
> > scrollback history by switching back and forth between consoles
> > which is why this feature is disabled by default.
> > Use the escape sequence \e[3J instead for flushing the buffer.
> > 
> > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de>
> > Reviewed-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > Tested-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > Tested-by: Adam Borowski <kilobyte@angband.pl>
> > ---
> >  drivers/video/console/Kconfig  |  25 +++++++-
> >  drivers/video/console/vgacon.c | 142 ++++++++++++++++++++++++++---
> > ------------
> >  2 files changed, 111 insertions(+), 56 deletions(-)
> > 
> > diff --git a/drivers/video/console/Kconfig
> > b/drivers/video/console/Kconfig
> > index c3f1fb9ee820..f500e58f7636 100644
> > --- a/drivers/video/console/Kconfig
> > +++ b/drivers/video/console/Kconfig
> > @@ -43,9 +43,28 @@ config VGACON_SOFT_SCROLLBACK_SIZE
> >         range 1 1024
> >         default "64"
> >         help
> > -         Enter the amount of System RAM to allocate for the
> > scrollback
> > -	 buffer.  Each 64KB will give you approximately 16 80x25
> > -	 screenfuls of scrollback buffer
> > +	  Enter the amount of System RAM to allocate for
> > scrollback
> > +	  buffers of VGA consoles. Each 64KB will give you
> > approximately
> > +	  16 80x25 screenfuls of scrollback buffer.
> > +
> > +config VGACON_SOFT_SCROLLBACK_PERSISTENT
> > +	bool "Persistent Scrollback History for each console"
> > +	depends on VGACON_SOFT_SCROLLBACK
> > +	default n
> > +	help
> > +	  Say Y here if the scrollback history should persist when
> > switching
> > +	  between consoles. Otherwise, the scrollback history will
> > be flushed
> > +	  each time the console is switched.
> > +
> > +	  This feature might break your tool of choice to flush
> > the scrollback
> > +	  buffer, e.g. clear(1) will work fine but Debian's
> > clear_console(1)
> > +	  will be broken, which might cause security issues.
> > +	  You can use the escape sequence \e[3J instead if this
> > feature is
> > +	  activated.
> 
> This issue is the one that makes me the most worried.  Why doesn't
> clear_console() work anymore?  Why doesn't it use \e[3J ?

Well, clear_console() just switches from one console to another and
back again. It just assumes that the scrollback buffer is flushed when
switching.
My plan is to make a patch for clear_console() as soon as these patches
are in the kernel - it's chicken-and-egg problem.

Bye,

Manuel

[toc] | [prev] | [next] | [standalone]


#1562905

FromGreg KH <gregkh@linuxfoundation.org>
Date2017-01-19 17:50 +0100
Message-ID<t1o4r-3Xc-55@gated-at.bofh.it>
In reply to#1562867
On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > Add a scrollback buffers for each VGA console. The benefit is that
> > > the scrollback history is not flushed when switching between
> > > consoles
> > > but is persistent.
> > > The buffers are allocated on demand when a new console is opened.
> > > 
> > > This breaks tools like clear_console that rely on flushing the
> > > scrollback history by switching back and forth between consoles
> > > which is why this feature is disabled by default.
> > > Use the escape sequence \e[3J instead for flushing the buffer.
> > > 
> > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de>
> > > Reviewed-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > > Tested-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > > Tested-by: Adam Borowski <kilobyte@angband.pl>
> > > ---
> > >  drivers/video/console/Kconfig  |  25 +++++++-
> > >  drivers/video/console/vgacon.c | 142 ++++++++++++++++++++++++++---
> > > ------------
> > >  2 files changed, 111 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/drivers/video/console/Kconfig
> > > b/drivers/video/console/Kconfig
> > > index c3f1fb9ee820..f500e58f7636 100644
> > > --- a/drivers/video/console/Kconfig
> > > +++ b/drivers/video/console/Kconfig
> > > @@ -43,9 +43,28 @@ config VGACON_SOFT_SCROLLBACK_SIZE
> > >         range 1 1024
> > >         default "64"
> > >         help
> > > -         Enter the amount of System RAM to allocate for the
> > > scrollback
> > > -	 buffer.  Each 64KB will give you approximately 16 80x25
> > > -	 screenfuls of scrollback buffer
> > > +	  Enter the amount of System RAM to allocate for
> > > scrollback
> > > +	  buffers of VGA consoles. Each 64KB will give you
> > > approximately
> > > +	  16 80x25 screenfuls of scrollback buffer.
> > > +
> > > +config VGACON_SOFT_SCROLLBACK_PERSISTENT
> > > +	bool "Persistent Scrollback History for each console"
> > > +	depends on VGACON_SOFT_SCROLLBACK
> > > +	default n
> > > +	help
> > > +	  Say Y here if the scrollback history should persist when
> > > switching
> > > +	  between consoles. Otherwise, the scrollback history will
> > > be flushed
> > > +	  each time the console is switched.
> > > +
> > > +	  This feature might break your tool of choice to flush
> > > the scrollback
> > > +	  buffer, e.g. clear(1) will work fine but Debian's
> > > clear_console(1)
> > > +	  will be broken, which might cause security issues.
> > > +	  You can use the escape sequence \e[3J instead if this
> > > feature is
> > > +	  activated.
> > 
> > This issue is the one that makes me the most worried.  Why doesn't
> > clear_console() work anymore?  Why doesn't it use \e[3J ?
> 
> Well, clear_console() just switches from one console to another and
> back again. It just assumes that the scrollback buffer is flushed when
> switching.
> My plan is to make a patch for clear_console() as soon as these patches
> are in the kernel - it's chicken-and-egg problem.

I'd recommend that patch get to clear_console() first, having it use the
new escape sequence, if it isn't supported, shouldn't cause any
problems, right?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]


#1563186 — Bug#845177: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

FromAdam Borowski <kilobyte@angband.pl>
Date2017-01-20 00:10 +0100
SubjectBug#845177: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles
Message-ID<t1u0a-7Tg-23@gated-at.bofh.it>
In reply to#1562905
On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > +	  This feature might break your tool of choice to flush
> > > > the scrollback
> > > > +	  buffer, e.g. clear(1) will work fine but Debian's
> > > > clear_console(1)
> > > > +	  will be broken, which might cause security issues.
> > > > +	  You can use the escape sequence \e[3J instead if this
> > > > feature is
> > > > +	  activated.
> > > 
> > > This issue is the one that makes me the most worried.  Why doesn't
> > > clear_console() work anymore?  Why doesn't it use \e[3J ?
> > 
> > Well, clear_console() just switches from one console to another and
> > back again. It just assumes that the scrollback buffer is flushed when
> > switching.
> > My plan is to make a patch for clear_console() as soon as these patches
> > are in the kernel - it's chicken-and-egg problem.
> 
> I'd recommend that patch get to clear_console() first, having it use the
> new escape sequence, if it isn't supported, shouldn't cause any
> problems, right?

In that case, we need to hurry -- the last day for any non-serious fixes in
Debian is Jan 26, after that it'll be frozen for months, and any subsequent
changes won't get to stable users for around two years.

doko: would you consider, pretty please with a cherry on top, applying the
patch I've sent to this bug?  The privacy/security issue is pretty minor and
applies only to a tiny fraction of users, but I understand why Greg is
reluctant.

Manuel's scrollback changes won't go to 4.9, and won't be enabled by default
for the time being, but using a newer kernel on old userspace is something
really widespread, be it via bpo, containers on an updated host, etc.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

[toc] | [prev] | [next] | [standalone]


#1563560

FromAdam Borowski <kilobyte@angband.pl>
Date2017-01-20 14:20 +0100
Message-ID<t1HgK-7Iz-21@gated-at.bofh.it>
In reply to#1563186
On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > > +	  This feature might break your tool of choice to flush
> > > > > the scrollback
> > > > > +	  buffer, e.g. clear(1) will work fine but Debian's
> > > > > clear_console(1)
> > > > > +	  will be broken, which might cause security issues.
> > > > > +	  You can use the escape sequence \e[3J instead if this
> > > > > feature is
> > > > > +	  activated.
> > 
> > I'd recommend that patch get to clear_console() first, having it use the
> > new escape sequence, if it isn't supported, shouldn't cause any
> > problems, right?
> 
> doko: would you consider, pretty please with a cherry on top, applying the
> patch I've sent to this bug?  The privacy/security issue is pretty minor and
> applies only to a tiny fraction of users, but I understand why Greg is
> reluctant.

# Subject: Bug#845177 closed by Matthias Klose <doko@debian.org>
#
# This is an automatic notification regarding your Bug report
# which was filed against the bash package:
#
# #845177: clear_console: assumes VT switch clears scrollback
#
# It has been closed by Matthias Klose <doko@debian.org>.
[...]
# Changes:
#    * clear_console: Securely erase the current console. Closes: #845177.

-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

[toc] | [prev] | [next] | [standalone]


#1563586

FromGreg KH <gregkh@linuxfoundation.org>
Date2017-01-20 14:40 +0100
Message-ID<t1HA6-7Ps-33@gated-at.bofh.it>
In reply to#1563560
On Fri, Jan 20, 2017 at 02:16:11PM +0100, Adam Borowski wrote:
> On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> > On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > > On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > > > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > > > +	  This feature might break your tool of choice to flush
> > > > > > the scrollback
> > > > > > +	  buffer, e.g. clear(1) will work fine but Debian's
> > > > > > clear_console(1)
> > > > > > +	  will be broken, which might cause security issues.
> > > > > > +	  You can use the escape sequence \e[3J instead if this
> > > > > > feature is
> > > > > > +	  activated.
> > > 
> > > I'd recommend that patch get to clear_console() first, having it use the
> > > new escape sequence, if it isn't supported, shouldn't cause any
> > > problems, right?
> > 
> > doko: would you consider, pretty please with a cherry on top, applying the
> > patch I've sent to this bug?  The privacy/security issue is pretty minor and
> > applies only to a tiny fraction of users, but I understand why Greg is
> > reluctant.
> 
> # Subject: Bug#845177 closed by Matthias Klose <doko@debian.org>
> #
> # This is an automatic notification regarding your Bug report
> # which was filed against the bash package:
> #
> # #845177: clear_console: assumes VT switch clears scrollback
> #
> # It has been closed by Matthias Klose <doko@debian.org>.
> [...]
> # Changes:
> #    * clear_console: Securely erase the current console. Closes: #845177.

This means it was accepted?  Or rejected?

confused,

greg k-h

[toc] | [prev] | [next] | [standalone]


#1563653

FromAdam Borowski <kilobyte@angband.pl>
Date2017-01-20 15:30 +0100
Message-ID<t1Imv-8mw-57@gated-at.bofh.it>
In reply to#1563586
On Fri, Jan 20, 2017 at 02:31:56PM +0100, Greg KH wrote:
> On Fri, Jan 20, 2017 at 02:16:11PM +0100, Adam Borowski wrote:
> > On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> > > On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > > > I'd recommend that patch get to clear_console() first, having it use the
> > > > new escape sequence, if it isn't supported, shouldn't cause any
> > > > problems, right?
> > 
> > # Subject: Bug#845177 closed by Matthias Klose <doko@debian.org>
> > #
> > # This is an automatic notification regarding your Bug report
> > # which was filed against the bash package:
> > #
> > # #845177: clear_console: assumes VT switch clears scrollback
> > #
> > # It has been closed by Matthias Klose <doko@debian.org>.
> > [...]
> > # Changes:
> > #    * clear_console: Securely erase the current console. Closes: #845177.
> 
> This means it was accepted?  Or rejected?

Accepted.

It's in unstable (or will be in the next mirror pulse), we're at the fastest
possible moment to get stuff to the next stable release -- it'll be in
Stretch (about to freeze).  Ubuntu migration is currently open so it'll get
there soon, in time for Zesty (17.04).  Other Debian derivatives likewise
pull at their own pace.

Most distributions unrelated to Debian don't seem to ship clear_console but
I have no real idea: just checked Fedora 25, it doesn't have it in the
default install at least.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

[toc] | [prev] | [next] | [standalone]


#1563021

FromAdam Borowski <kilobyte@angband.pl>
Date2017-01-19 20:20 +0100
Message-ID<t1qpA-5xV-5@gated-at.bofh.it>
In reply to#1562867
On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > Add a scrollback buffers for each VGA console.  The benefit is that
> > > the scrollback history is not flushed when switching between consoles
> > > but is persistent.  The buffers are allocated on demand when a new
> > > console is opened.
> > > 
> > > This breaks tools like clear_console that rely on flushing the
> > > scrollback history by switching back and forth between consoles
> > > which is why this feature is disabled by default.
> > > Use the escape sequence \e[3J instead for flushing the buffer.
> > 
> > This issue is the one that makes me the most worried.  Why doesn't
> > clear_console() work anymore?  Why doesn't it use \e[3J ?
> 
> Well, clear_console() just switches from one console to another and
> back again. It just assumes that the scrollback buffer is flushed when
> switching.
> My plan is to make a patch for clear_console() as soon as these patches
> are in the kernel - it's chicken-and-egg problem.

No need to wait, \e[3J is supported since Linux 2.6.39; the problem I
spotted was that a previous version of your patch would break that.

It is also safe to output that sequence to a terminal unconditionally: I've
tested a number of terminals, they all either support it (most X terminals,
our console) or silently ignore it.

We can't, though, rely on terminfo to do so: it knows about this capability
(which it calls "E3") for TERM=linux only since very recently.  The TERM
variable is also unreliable: it fails to carry over a serial link while
blindly printing \e[3J works.

As for patching clear_console, https://bugs.debian.org/845177 has a minimal
fix; although for all setups supported by Debian that program could be be
better replaced with just "printf '\e[3J\e[2J'" which would make it work on
strictly more terminals than current code does.  Same for distributions
which copied clear_console (it originates from Ubuntu, maintained in Debian
since then).


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web