* [PATCH 0/3] follow-up to the unicode vt console series
@ 2018-07-18 1:02 Nicolas Pitre
2018-07-18 1:02 ` [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function Nicolas Pitre
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-18 1:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Geert Uytterhoeven, Adam Borowski, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
This is a follow-up series with 3 patches in response to the following
messages respectively:
http://lkml.kernel.org/r/20180628123803.GA22045@kroah.com
http://lkml.kernel.org/r/CAGXu5j+KLvJ-n_QRrfq15E8iO_rqfpp+K7PDAZHZMHcemy9y7g@mail.gmail.com
http://lkml.kernel.org/r/CAMuHMdUmC+uiKTEMotB83A86E1wEYrNF2qPf3kDACaW27D_NbA@mail.gmail.com
Those patches were tested on top of previous patches already in tty-next.
Please include in tty-next before next merge window.
Patch #1 was posted before. Only difference is that it now uses rate
limited printing.
Patch #2 is new, however it has been tested with the validation code
provided by patch #1. This also fixes a security issue.
Patch #3 is new and trivial.
Diffstat:
Documentation/admin-guide/devices.txt | 16 +++++---
drivers/tty/vt/vt.c | 57 +++++++++++++++++++++++++----
2 files changed, 59 insertions(+), 14 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function
2018-07-18 1:02 [PATCH 0/3] follow-up to the unicode vt console series Nicolas Pitre
@ 2018-07-18 1:02 ` Nicolas Pitre
2018-07-18 1:48 ` Adam Borowski
2018-07-18 1:02 ` [PATCH 2/3] vt: coherence validation code for the unicode screen buffer Nicolas Pitre
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-18 1:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Geert Uytterhoeven, Adam Borowski, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
The nr argument is typically small: most often nr == 1. However this
could be abused with a very large explicit scroll in a resized screen.
Make the code scroll lines one at a time in all cases to avoid the VLA.
Anything smarter is most likely not warranted here.
Requested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
drivers/tty/vt/vt.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 2d14bb195d..03e79f7787 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -433,20 +433,22 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
if (uniscr) {
unsigned int s, d, rescue, clear;
- char32_t *save[nr];
s = clear = t;
- d = t + nr;
- rescue = b - nr;
+ d = t + 1;
+ rescue = b - 1;
if (dir == SM_UP) {
swap(s, d);
swap(clear, rescue);
}
- memcpy(save, uniscr->lines + rescue, nr * sizeof(*save));
- memmove(uniscr->lines + d, uniscr->lines + s,
- (b - t - nr) * sizeof(*uniscr->lines));
- memcpy(uniscr->lines + clear, save, nr * sizeof(*save));
- vc_uniscr_clear_lines(vc, clear, nr);
+ while (nr--) {
+ char32_t *tmp;
+ tmp = uniscr->lines[rescue];
+ memmove(uniscr->lines + d, uniscr->lines + s,
+ (b - t - 1) * sizeof(*uniscr->lines));
+ uniscr->lines[clear] = tmp;
+ vc_uniscr_clear_lines(vc, clear, 1);
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] vt: coherence validation code for the unicode screen buffer
2018-07-18 1:02 [PATCH 0/3] follow-up to the unicode vt console series Nicolas Pitre
2018-07-18 1:02 ` [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function Nicolas Pitre
@ 2018-07-18 1:02 ` Nicolas Pitre
2018-07-18 1:02 ` [PATCH 3/3] vt: add /dev/vcsu* to devices.txt Nicolas Pitre
2018-07-18 14:04 ` [PATCH 0/3] follow-up to the unicode vt console series Kees Cook
3 siblings, 0 replies; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-18 1:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Geert Uytterhoeven, Adam Borowski, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
Make sure the unicode screen buffer matches the video screen content.
This is provided for debugging convenience and disabled by default.
Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
drivers/tty/vt/vt.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 03e79f7787..331f175265 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -328,6 +328,8 @@ void schedule_console_callback(void)
#define get_vc_uniscr(vc) vc->vc_uni_screen
#endif
+#define VC_UNI_SCREEN_DEBUG 0
+
typedef uint32_t char32_t;
/*
@@ -571,6 +573,42 @@ void vc_uniscr_copy_line(struct vc_data *vc, void *dest, int viewed,
}
}
+/* this is for validation and debugging only */
+static void vc_uniscr_debug_check(struct vc_data *vc)
+{
+ struct uni_screen *uniscr = get_vc_uniscr(vc);
+ unsigned short *p;
+ int x, y, mask;
+
+ if (!VC_UNI_SCREEN_DEBUG || !uniscr)
+ return;
+
+ WARN_CONSOLE_UNLOCKED();
+
+ /*
+ * Make sure our unicode screen translates into the same glyphs
+ * as the actual screen. This is brutal indeed.
+ */
+ p = (unsigned short *)vc->vc_origin;
+ mask = vc->vc_hi_font_mask | 0xff;
+ for (y = 0; y < vc->vc_rows; y++) {
+ char32_t *line = uniscr->lines[y];
+ for (x = 0; x < vc->vc_cols; x++) {
+ u16 glyph = scr_readw(p++) & mask;
+ char32_t uc = line[x];
+ int tc = conv_uni_to_pc(vc, uc);
+ if (tc == -4)
+ tc = conv_uni_to_pc(vc, 0xfffd);
+ if (tc == -4)
+ tc = conv_uni_to_pc(vc, '?');
+ if (tc != glyph)
+ pr_err_ratelimited(
+ "%s: mismatch at %d,%d: glyph=%#x tc=%#x\n",
+ __func__, x, y, glyph, tc);
+ }
+ }
+}
+
static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
enum con_scroll dir, unsigned int nr)
@@ -2719,6 +2757,7 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co
do_con_trol(tty, vc, orig);
}
con_flush(vc, draw_from, draw_to, &draw_x);
+ vc_uniscr_debug_check(vc);
console_conditional_schedule();
console_unlock();
notify_update(vc);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] vt: add /dev/vcsu* to devices.txt
2018-07-18 1:02 [PATCH 0/3] follow-up to the unicode vt console series Nicolas Pitre
2018-07-18 1:02 ` [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function Nicolas Pitre
2018-07-18 1:02 ` [PATCH 2/3] vt: coherence validation code for the unicode screen buffer Nicolas Pitre
@ 2018-07-18 1:02 ` Nicolas Pitre
2018-07-18 7:25 ` Geert Uytterhoeven
2018-07-18 14:04 ` [PATCH 0/3] follow-up to the unicode vt console series Kees Cook
3 siblings, 1 reply; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-18 1:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Geert Uytterhoeven, Adam Borowski, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
Also mention that the traditional devices provide glyph values whereas
/dev/vcsu* is unicode based.
Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
Documentation/admin-guide/devices.txt | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/devices.txt b/Documentation/admin-guide/devices.txt
index 4ec843123c..1649117e60 100644
--- a/Documentation/admin-guide/devices.txt
+++ b/Documentation/admin-guide/devices.txt
@@ -173,14 +173,18 @@
they are redirected through the parport multiplex layer.
7 char Virtual console capture devices
- 0 = /dev/vcs Current vc text contents
- 1 = /dev/vcs1 tty1 text contents
+ 0 = /dev/vcs Current vc text (glyph) contents
+ 1 = /dev/vcs1 tty1 text (glyph) contents
...
- 63 = /dev/vcs63 tty63 text contents
- 128 = /dev/vcsa Current vc text/attribute contents
- 129 = /dev/vcsa1 tty1 text/attribute contents
+ 63 = /dev/vcs63 tty63 text (glyph) contents
+ 64 = /dev/vcsu Current vc text (unicode) contents
+ 65 = /dev/vcsu1 tty1 text (unicode) contents
...
- 191 = /dev/vcsa63 tty63 text/attribute contents
+ 127 = /dev/vcsu63 tty63 text (unicode) contents
+ 128 = /dev/vcsa Current vc text/attribute (glyph) contents
+ 129 = /dev/vcsa1 tty1 text/attribute (glyph) contents
+ ...
+ 191 = /dev/vcsa63 tty63 text/attribute (glyph) contents
NOTE: These devices permit both read and write access.
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function
2018-07-18 1:02 ` [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function Nicolas Pitre
@ 2018-07-18 1:48 ` Adam Borowski
2018-07-18 2:54 ` Nicolas Pitre
0 siblings, 1 reply; 10+ messages in thread
From: Adam Borowski @ 2018-07-18 1:48 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Greg Kroah-Hartman, Kees Cook, Geert Uytterhoeven, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
On Tue, Jul 17, 2018 at 09:02:40PM -0400, Nicolas Pitre wrote:
> The nr argument is typically small: most often nr == 1. However this
> could be abused with a very large explicit scroll in a resized screen.
> Make the code scroll lines one at a time in all cases to avoid the VLA.
> Anything smarter is most likely not warranted here.
Even though nr can be 32767 at most, your new version is O(nr*nr) for no
reason. Instead of O(n) memory or O(n²) time, a variant of the original
that copies values one at a time would be shorter and faster.
> Requested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> drivers/tty/vt/vt.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 2d14bb195d..03e79f7787 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -433,20 +433,22 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
>
> if (uniscr) {
> unsigned int s, d, rescue, clear;
> - char32_t *save[nr];
>
> s = clear = t;
> - d = t + nr;
> - rescue = b - nr;
> + d = t + 1;
> + rescue = b - 1;
> if (dir == SM_UP) {
> swap(s, d);
> swap(clear, rescue);
> }
> - memcpy(save, uniscr->lines + rescue, nr * sizeof(*save));
> - memmove(uniscr->lines + d, uniscr->lines + s,
> - (b - t - nr) * sizeof(*uniscr->lines));
> - memcpy(uniscr->lines + clear, save, nr * sizeof(*save));
> - vc_uniscr_clear_lines(vc, clear, nr);
> + while (nr--) {
> + char32_t *tmp;
> + tmp = uniscr->lines[rescue];
> + memmove(uniscr->lines + d, uniscr->lines + s,
> + (b - t - 1) * sizeof(*uniscr->lines));
> + uniscr->lines[clear] = tmp;
> + vc_uniscr_clear_lines(vc, clear, 1);
> + }
> }
> }
What the function does is rotating an array (slice [t..b) here), by nr if
SM_DOWN or by -nr ie (b - t - nr) if SM_UP. A nice problem that almost every
"code interview questions" book includes :)
Please say if you don't have time for such games, I've just refreshed what's
a good answer. :þ
Meow.
--
// If you believe in so-called "intellectual property", please immediately
// cease using counterfeit alphabets. Instead, contact the nearest temple
// of Amon, whose priests will provide you with scribal services for all
// your writing needs, for Reasonable And Non-Discriminatory prices.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function
2018-07-18 1:48 ` Adam Borowski
@ 2018-07-18 2:54 ` Nicolas Pitre
2018-07-19 4:05 ` Nicolas Pitre
0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-18 2:54 UTC (permalink / raw)
To: Adam Borowski
Cc: Greg Kroah-Hartman, Kees Cook, Geert Uytterhoeven, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
[-- Attachment #1: Type: text/plain, Size: 4111 bytes --]
On Wed, 18 Jul 2018, Adam Borowski wrote:
> On Tue, Jul 17, 2018 at 09:02:40PM -0400, Nicolas Pitre wrote:
> > The nr argument is typically small: most often nr == 1. However this
> > could be abused with a very large explicit scroll in a resized screen.
> > Make the code scroll lines one at a time in all cases to avoid the VLA.
> > Anything smarter is most likely not warranted here.
>
> Even though nr can be 32767 at most, your new version is O(nr*nr) for no
> reason. Instead of O(n) memory or O(n²) time, a variant of the original
> that copies values one at a time would be shorter and faster.
Well... even though nr _can_ be up to 32766 to be precise, it is most
likely to be just 1 in 99.9% of the cases. So in that case, you'll
execute the loop only once and the code is currently optimal with O(n).
If nr > 1 then the current cost is O(n*nr) where n is the height of the
scroll window i.e. relatively small in practice (typically between 25
and 60). There is no point optimizing for 32767 rows as that is rather
silly. If we had then the best solution would be a linked list rather
than an array.
But still, if nr > 2 that means you need a temporary storage because the
destination memory has to be preserved before the source memory can be
moved there, and that destination memory content cannot be stored in the
vacated source memory until the source content is moved. Copying values
one at a time cannot work because the destination memory, the source
memory, and the area where the previous content from the destination
memory will end up don't overlap most of the time.
That temporary storage was that VLA. We don't want VLAs. So how do we
efficiently allocate and deallocate memory for, say, 25 words? Maybe
that doesn't have to be efficient because that doesn't happen very often
as we said, at which point we can just do it in a loop with a one-line
increment instead, as this patch does.
If you still have a more clever way of doing this then please propose it
in code form (I'm genuinely curious of what you have in mind). But let's
get the baseline working in an obvious "correct" way first.
> > Requested-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> > drivers/tty/vt/vt.c | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 2d14bb195d..03e79f7787 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -433,20 +433,22 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
> >
> > if (uniscr) {
> > unsigned int s, d, rescue, clear;
> > - char32_t *save[nr];
> >
> > s = clear = t;
> > - d = t + nr;
> > - rescue = b - nr;
> > + d = t + 1;
> > + rescue = b - 1;
> > if (dir == SM_UP) {
> > swap(s, d);
> > swap(clear, rescue);
> > }
> > - memcpy(save, uniscr->lines + rescue, nr * sizeof(*save));
> > - memmove(uniscr->lines + d, uniscr->lines + s,
> > - (b - t - nr) * sizeof(*uniscr->lines));
> > - memcpy(uniscr->lines + clear, save, nr * sizeof(*save));
> > - vc_uniscr_clear_lines(vc, clear, nr);
> > + while (nr--) {
> > + char32_t *tmp;
> > + tmp = uniscr->lines[rescue];
> > + memmove(uniscr->lines + d, uniscr->lines + s,
> > + (b - t - 1) * sizeof(*uniscr->lines));
> > + uniscr->lines[clear] = tmp;
> > + vc_uniscr_clear_lines(vc, clear, 1);
> > + }
> > }
> > }
>
> What the function does is rotating an array (slice [t..b) here), by nr if
> SM_DOWN or by -nr ie (b - t - nr) if SM_UP. A nice problem that almost every
> "code interview questions" book includes :)
>
> Please say if you don't have time for such games, I've just refreshed what's
> a good answer. :þ
>
>
> Meow.
> --
> // If you believe in so-called "intellectual property", please immediately
> // cease using counterfeit alphabets. Instead, contact the nearest temple
> // of Amon, whose priests will provide you with scribal services for all
> // your writing needs, for Reasonable And Non-Discriminatory prices.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] vt: add /dev/vcsu* to devices.txt
2018-07-18 1:02 ` [PATCH 3/3] vt: add /dev/vcsu* to devices.txt Nicolas Pitre
@ 2018-07-18 7:25 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2018-07-18 7:25 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Greg KH, Kees Cook, Adam Borowski, Dave Mielke, Samuel Thibault,
Linux Kernel Mailing List, linux-console
On Wed, Jul 18, 2018 at 3:02 AM Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> Also mention that the traditional devices provide glyph values whereas
> /dev/vcsu* is unicode based.
>
> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] follow-up to the unicode vt console series
2018-07-18 1:02 [PATCH 0/3] follow-up to the unicode vt console series Nicolas Pitre
` (2 preceding siblings ...)
2018-07-18 1:02 ` [PATCH 3/3] vt: add /dev/vcsu* to devices.txt Nicolas Pitre
@ 2018-07-18 14:04 ` Kees Cook
3 siblings, 0 replies; 10+ messages in thread
From: Kees Cook @ 2018-07-18 14:04 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Greg Kroah-Hartman, Geert Uytterhoeven, Adam Borowski,
Dave Mielke, Samuel Thibault, LKML, linux-console
Hi Nicolas,
On Tue, Jul 17, 2018 at 6:02 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> This is a follow-up series with 3 patches in response to the following
> messages respectively:
>
> http://lkml.kernel.org/r/20180628123803.GA22045@kroah.com
>
> http://lkml.kernel.org/r/CAGXu5j+KLvJ-n_QRrfq15E8iO_rqfpp+K7PDAZHZMHcemy9y7g@mail.gmail.com
>
> http://lkml.kernel.org/r/CAMuHMdUmC+uiKTEMotB83A86E1wEYrNF2qPf3kDACaW27D_NbA@mail.gmail.com
>
> Those patches were tested on top of previous patches already in tty-next.
> Please include in tty-next before next merge window.
>
> Patch #1 was posted before. Only difference is that it now uses rate
> limited printing.
>
> Patch #2 is new, however it has been tested with the validation code
> provided by patch #1. This also fixes a security issue.
>
> Patch #3 is new and trivial.
>
> Diffstat:
>
> Documentation/admin-guide/devices.txt | 16 +++++---
> drivers/tty/vt/vt.c | 57 +++++++++++++++++++++++++----
> 2 files changed, 59 insertions(+), 14 deletions(-)
Thanks for fixing the VLA! :)
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function
2018-07-18 2:54 ` Nicolas Pitre
@ 2018-07-19 4:05 ` Nicolas Pitre
2018-07-21 7:20 ` [PATCH v2 " Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Pitre @ 2018-07-19 4:05 UTC (permalink / raw)
To: Adam Borowski, Greg Kroah-Hartman
Cc: Kees Cook, Geert Uytterhoeven, Dave Mielke, Samuel Thibault,
linux-kernel, linux-console
On Tue, 17 Jul 2018, Nicolas Pitre wrote:
> But still, if nr > 2 that means you need a temporary storage because the
> destination memory has to be preserved before the source memory can be
> moved there, and that destination memory content cannot be stored in the
> vacated source memory until the source content is moved.
OK I'm an idiot.
After looking in the literature, I found out that there is indeed a
better way to do this. So here's an updated patch:
----- >8
Subject: [PATCH v2 1/3] vt: avoid a VLA in the unicode screen scroll function
The nr argument is typically small: most often nr == 1. However this
could be abused with a very large explicit scroll in a resized screen.
Make the code scroll lines by performing an array rotation operation to
avoid the need for a large temporary space.
Requested-by: Kees Cook <keescook@chromium.org>
Suggested-by: Adam Borowski <kilobyte@angband.pl>
Signed-off-by: Nicolas Pitre <nico@linaro.org>
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 2d14bb195d..d527184579 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -104,6 +104,7 @@
#include <linux/kdb.h>
#include <linux/ctype.h>
#include <linux/bsearch.h>
+#include <linux/gcd.h>
#define MAX_NR_CON_DRIVER 16
@@ -432,20 +433,29 @@ static void vc_uniscr_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
struct uni_screen *uniscr = get_vc_uniscr(vc);
if (uniscr) {
- unsigned int s, d, rescue, clear;
- char32_t *save[nr];
-
- s = clear = t;
- d = t + nr;
- rescue = b - nr;
- if (dir == SM_UP) {
- swap(s, d);
- swap(clear, rescue);
+ unsigned int i, j, k, sz, d, clear;
+
+ sz = b - t;
+ clear = b - nr;
+ d = nr;
+ if (dir == SM_DOWN) {
+ clear = t;
+ d = sz - nr;
+ }
+ for (i = 0; i < gcd(d, sz); i++) {
+ char32_t *tmp = uniscr->lines[t + i];
+ j = i;
+ while (1) {
+ k = j + d;
+ if (k >= sz)
+ k -= sz;
+ if (k == i)
+ break;
+ uniscr->lines[t + j] = uniscr->lines[t + k];
+ j = k;
+ }
+ uniscr->lines[t + j] = tmp;
}
- memcpy(save, uniscr->lines + rescue, nr * sizeof(*save));
- memmove(uniscr->lines + d, uniscr->lines + s,
- (b - t - nr) * sizeof(*uniscr->lines));
- memcpy(uniscr->lines + clear, save, nr * sizeof(*save));
vc_uniscr_clear_lines(vc, clear, nr);
}
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] vt: avoid a VLA in the unicode screen scroll function
2018-07-19 4:05 ` Nicolas Pitre
@ 2018-07-21 7:20 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-21 7:20 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Adam Borowski, Kees Cook, Geert Uytterhoeven, Dave Mielke,
Samuel Thibault, linux-kernel, linux-console
On Thu, Jul 19, 2018 at 12:05:25AM -0400, Nicolas Pitre wrote:
>
> The nr argument is typically small: most often nr == 1. However this
> could be abused with a very large explicit scroll in a resized screen.
> Make the code scroll lines by performing an array rotation operation to
> avoid the need for a large temporary space.
>
> Requested-by: Kees Cook <keescook@chromium.org>
> Suggested-by: Adam Borowski <kilobyte@angband.pl>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
I've now applied this v2 of the patch, thanks for this.
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-21 7:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 1:02 [PATCH 0/3] follow-up to the unicode vt console series Nicolas Pitre
2018-07-18 1:02 ` [PATCH 1/3] vt: avoid a VLA in the unicode screen scroll function Nicolas Pitre
2018-07-18 1:48 ` Adam Borowski
2018-07-18 2:54 ` Nicolas Pitre
2018-07-19 4:05 ` Nicolas Pitre
2018-07-21 7:20 ` [PATCH v2 " Greg Kroah-Hartman
2018-07-18 1:02 ` [PATCH 2/3] vt: coherence validation code for the unicode screen buffer Nicolas Pitre
2018-07-18 1:02 ` [PATCH 3/3] vt: add /dev/vcsu* to devices.txt Nicolas Pitre
2018-07-18 7:25 ` Geert Uytterhoeven
2018-07-18 14:04 ` [PATCH 0/3] follow-up to the unicode vt console series Kees Cook
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).