[PATCH 01/16] vt: make vc_data pointers const in selection.h

[PATCH 01/16] vt: make vc_data pointers const in selection.h

[Jiri Slaby]

There are many functions declared in selection.h which only read from
struct vc_data passed as a parameter. Make all those uses const to hint
the compiler a bit.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/consolemap.c      |  2 +-
 drivers/tty/vt/vt.c              | 15 ++++++++-------
 drivers/video/console/sticon.c   |  2 +-
 drivers/video/fbdev/core/fbcon.c |  2 +-
 include/linux/console.h          |  2 +-
 include/linux/consolemap.h       |  3 ++-
 include/linux/selection.h        | 14 ++++++++------
 7 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 5947b54d92be..5d5a5fd2dce7 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -268,7 +268,7 @@ unsigned short *set_translate(int m, struct vc_data *vc)
  *    was active.
  * Still, it is now possible to a certain extent to cut and paste non-ASCII.
  */
-u16 inverse_translate(struct vc_data *conp, int glyph, int use_unicode)
+u16 inverse_translate(const struct vc_data *conp, int glyph, int use_unicode)
 {
 	struct uni_pagedir *p;
 	int m;
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index ccb533fd00a2..8f283221330e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -283,7 +283,8 @@ static inline bool con_should_update(const struct vc_data *vc)
 	return con_is_visible(vc) && !console_blanked;
 }
 
-static inline unsigned short *screenpos(struct vc_data *vc, int offset, int viewed)
+static inline unsigned short *screenpos(const struct vc_data *vc, int offset,
+		int viewed)
 {
 	unsigned short *p;
 	
@@ -543,7 +544,7 @@ int vc_uniscr_check(struct vc_data *vc)
  * This must be preceded by a successful call to vc_uniscr_check() once
  * the console lock has been taken.
  */
-void vc_uniscr_copy_line(struct vc_data *vc, void *dest, int viewed,
+void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, int viewed,
 			 unsigned int row, unsigned int col, unsigned int nr)
 {
 	struct uni_screen *uniscr = get_vc_uniscr(vc);
@@ -4740,7 +4741,7 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
  */
 
 /* used by selection */
-u16 screen_glyph(struct vc_data *vc, int offset)
+u16 screen_glyph(const struct vc_data *vc, int offset)
 {
 	u16 w = scr_readw(screenpos(vc, offset, 1));
 	u16 c = w & 0xff;
@@ -4751,7 +4752,7 @@ u16 screen_glyph(struct vc_data *vc, int offset)
 }
 EXPORT_SYMBOL_GPL(screen_glyph);
 
-u32 screen_glyph_unicode(struct vc_data *vc, int n)
+u32 screen_glyph_unicode(const struct vc_data *vc, int n)
 {
 	struct uni_screen *uniscr = get_vc_uniscr(vc);
 
@@ -4762,13 +4763,13 @@ u32 screen_glyph_unicode(struct vc_data *vc, int n)
 EXPORT_SYMBOL_GPL(screen_glyph_unicode);
 
 /* used by vcs - note the word offset */
-unsigned short *screen_pos(struct vc_data *vc, int w_offset, int viewed)
+unsigned short *screen_pos(const struct vc_data *vc, int w_offset, int viewed)
 {
 	return screenpos(vc, 2 * w_offset, viewed);
 }
 EXPORT_SYMBOL_GPL(screen_pos);
 
-void getconsxy(struct vc_data *vc, unsigned char *p)
+void getconsxy(const struct vc_data *vc, unsigned char *p)
 {
 	/* clamp values if they don't fit */
 	p[0] = min(vc->state.x, 0xFFu);
@@ -4782,7 +4783,7 @@ void putconsxy(struct vc_data *vc, unsigned char *p)
 	set_cursor(vc);
 }
 
-u16 vcs_scr_readw(struct vc_data *vc, const u16 *org)
+u16 vcs_scr_readw(const struct vc_data *vc, const u16 *org)
 {
 	if ((unsigned long)org == vc->vc_pos && softcursor_original != -1)
 		return softcursor_original;
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 21a5c280c8c9..e21147e8a14e 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -236,7 +236,7 @@ static int sticon_blank(struct vc_data *c, int blank, int mode_switch)
     return 1;
 }
 
-static u16 *sticon_screen_pos(struct vc_data *conp, int offset)
+static u16 *sticon_screen_pos(const struct vc_data *conp, int offset)
 {
     int line;
     unsigned long p;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 8a31fc2b2258..6068845d98f2 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2765,7 +2765,7 @@ static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table)
 	fb_set_cmap(&palette_cmap, info);
 }
 
-static u16 *fbcon_screen_pos(struct vc_data *vc, int offset)
+static u16 *fbcon_screen_pos(const struct vc_data *vc, int offset)
 {
 	unsigned long p;
 	int line;
diff --git a/include/linux/console.h b/include/linux/console.h
index 0670d3491e0e..4b1e26c4cb42 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -74,7 +74,7 @@ struct consw {
 			enum vc_intensity intensity,
 			bool blink, bool underline, bool reverse, bool italic);
 	void	(*con_invert_region)(struct vc_data *vc, u16 *p, int count);
-	u16    *(*con_screen_pos)(struct vc_data *vc, int offset);
+	u16    *(*con_screen_pos)(const struct vc_data *vc, int offset);
 	unsigned long (*con_getxy)(struct vc_data *vc, unsigned long position,
 			int *px, int *py);
 	/*
diff --git a/include/linux/consolemap.h b/include/linux/consolemap.h
index 254246673390..bcfce748c9d8 100644
--- a/include/linux/consolemap.h
+++ b/include/linux/consolemap.h
@@ -17,7 +17,8 @@
 #ifdef CONFIG_CONSOLE_TRANSLATIONS
 struct vc_data;
 
-extern u16 inverse_translate(struct vc_data *conp, int glyph, int use_unicode);
+extern u16 inverse_translate(const struct vc_data *conp, int glyph,
+		int use_unicode);
 extern unsigned short *set_translate(int m, struct vc_data *vc);
 extern int conv_uni_to_pc(struct vc_data *conp, long ucs);
 extern u32 conv_8bit_to_uni(unsigned char c);
diff --git a/include/linux/selection.h b/include/linux/selection.h
index 5b890ef5b59f..34404a019ebf 100644
--- a/include/linux/selection.h
+++ b/include/linux/selection.h
@@ -33,21 +33,23 @@ extern unsigned char default_red[];
 extern unsigned char default_grn[];
 extern unsigned char default_blu[];
 
-extern unsigned short *screen_pos(struct vc_data *vc, int w_offset, int viewed);
-extern u16 screen_glyph(struct vc_data *vc, int offset);
-extern u32 screen_glyph_unicode(struct vc_data *vc, int offset);
+extern unsigned short *screen_pos(const struct vc_data *vc, int w_offset,
+		int viewed);
+extern u16 screen_glyph(const struct vc_data *vc, int offset);
+extern u32 screen_glyph_unicode(const struct vc_data *vc, int offset);
 extern void complement_pos(struct vc_data *vc, int offset);
 extern void invert_screen(struct vc_data *vc, int offset, int count, int shift);
 
-extern void getconsxy(struct vc_data *vc, unsigned char *p);
+extern void getconsxy(const struct vc_data *vc, unsigned char *p);
 extern void putconsxy(struct vc_data *vc, unsigned char *p);
 
-extern u16 vcs_scr_readw(struct vc_data *vc, const u16 *org);
+extern u16 vcs_scr_readw(const struct vc_data *vc, const u16 *org);
 extern void vcs_scr_writew(struct vc_data *vc, u16 val, u16 *org);
 extern void vcs_scr_updated(struct vc_data *vc);
 
 extern int vc_uniscr_check(struct vc_data *vc);
-extern void vc_uniscr_copy_line(struct vc_data *vc, void *dest, int viewed,
+extern void vc_uniscr_copy_line(const struct vc_data *vc, void *dest,
+				int viewed,
 				unsigned int row, unsigned int col,
 				unsigned int nr);
 
-- 
2.28.0

[PATCH 02/16] vt: declare xy for get/putconsxy properly

[Jiri Slaby]

That is:
1) call the parameter 'xy' to denote what it really is, not generic 'p'
2) tell the compiler and users that we expect an array:
   * with at least 2 chars (static 2)
   * which we don't modify in putconsxy (const)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vt.c       | 10 +++++-----
 include/linux/selection.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8f283221330e..a0da7771c327 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4769,17 +4769,17 @@ unsigned short *screen_pos(const struct vc_data *vc, int w_offset, int viewed)
 }
 EXPORT_SYMBOL_GPL(screen_pos);
 
-void getconsxy(const struct vc_data *vc, unsigned char *p)
+void getconsxy(const struct vc_data *vc, unsigned char xy[static 2])
 {
 	/* clamp values if they don't fit */
-	p[0] = min(vc->state.x, 0xFFu);
-	p[1] = min(vc->state.y, 0xFFu);
+	xy[0] = min(vc->state.x, 0xFFu);
+	xy[1] = min(vc->state.y, 0xFFu);
 }
 
-void putconsxy(struct vc_data *vc, unsigned char *p)
+void putconsxy(struct vc_data *vc, unsigned char xy[static const 2])
 {
 	hide_cursor(vc);
-	gotoxy(vc, p[0], p[1]);
+	gotoxy(vc, xy[0], xy[1]);
 	set_cursor(vc);
 }
 
diff --git a/include/linux/selection.h b/include/linux/selection.h
index 34404a019ebf..15e36e7ef869 100644
--- a/include/linux/selection.h
+++ b/include/linux/selection.h
@@ -40,8 +40,8 @@ extern u32 screen_glyph_unicode(const struct vc_data *vc, int offset);
 extern void complement_pos(struct vc_data *vc, int offset);
 extern void invert_screen(struct vc_data *vc, int offset, int count, int shift);
 
-extern void getconsxy(const struct vc_data *vc, unsigned char *p);
-extern void putconsxy(struct vc_data *vc, unsigned char *p);
+extern void getconsxy(const struct vc_data *vc, unsigned char xy[static 2]);
+extern void putconsxy(struct vc_data *vc, unsigned char xy[static const 2]);
 
 extern u16 vcs_scr_readw(const struct vc_data *vc, const u16 *org);
 extern void vcs_scr_writew(struct vc_data *vc, u16 val, u16 *org);
-- 
2.28.0

[PATCH 03/16] vc: propagate "viewed as bool" from screenpos up

[Jiri Slaby]

viewed is used as a flag, i.e. bool. So treat is as such in most of the
places. vcs_vc is handled in the next patch.

Note: the last parameter of invert_screen was misnamed in the
declaration since 1.1.92.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/accessibility/speakup/main.c |  4 ++--
 drivers/tty/vt/selection.c           |  2 +-
 drivers/tty/vt/vt.c                  | 18 ++++++++++--------
 include/linux/selection.h            |  6 +++---
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/accessibility/speakup/main.c b/drivers/accessibility/speakup/main.c
index ddfd12afe3b9..be79b2135fac 100644
--- a/drivers/accessibility/speakup/main.c
+++ b/drivers/accessibility/speakup/main.c
@@ -257,7 +257,7 @@ static struct notifier_block vt_notifier_block = {
 
 static unsigned char get_attributes(struct vc_data *vc, u16 *pos)
 {
-	pos = screen_pos(vc, pos - (u16 *)vc->vc_origin, 1);
+	pos = screen_pos(vc, pos - (u16 *)vc->vc_origin, true);
 	return (scr_readw(pos) & ~vc->vc_hi_font_mask) >> 8;
 }
 
@@ -465,7 +465,7 @@ static u16 get_char(struct vc_data *vc, u16 *pos, u_char *attribs)
 		u16 w;
 		u16 c;
 
-		pos = screen_pos(vc, pos - (u16 *)vc->vc_origin, 1);
+		pos = screen_pos(vc, pos - (u16 *)vc->vc_origin, true);
 		w = scr_readw(pos);
 		c = w & 0xff;
 
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 8e74654c1b27..f245a5acf7e9 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -54,7 +54,7 @@ static struct vc_selection {
 /* set reverse video on characters s-e of console with selection. */
 static inline void highlight(const int s, const int e)
 {
-	invert_screen(vc_sel.cons, s, e-s+2, 1);
+	invert_screen(vc_sel.cons, s, e-s+2, true);
 }
 
 /* use complementary color to show the pointer */
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index a0da7771c327..0f7064d41e92 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -284,7 +284,7 @@ static inline bool con_should_update(const struct vc_data *vc)
 }
 
 static inline unsigned short *screenpos(const struct vc_data *vc, int offset,
-		int viewed)
+		bool viewed)
 {
 	unsigned short *p;
 	
@@ -544,7 +544,7 @@ int vc_uniscr_check(struct vc_data *vc)
  * This must be preceded by a successful call to vc_uniscr_check() once
  * the console lock has been taken.
  */
-void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, int viewed,
+void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 			 unsigned int row, unsigned int col, unsigned int nr)
 {
 	struct uni_screen *uniscr = get_vc_uniscr(vc);
@@ -753,7 +753,7 @@ static void update_attr(struct vc_data *vc)
 }
 
 /* Note: inverting the screen twice should revert to the original state */
-void invert_screen(struct vc_data *vc, int offset, int count, int viewed)
+void invert_screen(struct vc_data *vc, int offset, int count, bool viewed)
 {
 	unsigned short *p;
 
@@ -812,7 +812,7 @@ void complement_pos(struct vc_data *vc, int offset)
 
 	if (old_offset != -1 && old_offset >= 0 &&
 	    old_offset < vc->vc_screenbuf_size) {
-		scr_writew(old, screenpos(vc, old_offset, 1));
+		scr_writew(old, screenpos(vc, old_offset, true));
 		if (con_should_update(vc))
 			vc->vc_sw->con_putc(vc, old, oldy, oldx);
 		notify_update(vc);
@@ -824,7 +824,7 @@ void complement_pos(struct vc_data *vc, int offset)
 	    offset < vc->vc_screenbuf_size) {
 		unsigned short new;
 		unsigned short *p;
-		p = screenpos(vc, offset, 1);
+		p = screenpos(vc, offset, true);
 		old = scr_readw(p);
 		new = old ^ vc->vc_complement_mask;
 		scr_writew(new, p);
@@ -1885,7 +1885,9 @@ static void set_mode(struct vc_data *vc, int on_off)
 			case 5:			/* Inverted screen on/off */
 				if (vc->vc_decscnm != on_off) {
 					vc->vc_decscnm = on_off;
-					invert_screen(vc, 0, vc->vc_screenbuf_size, 0);
+					invert_screen(vc, 0,
+							vc->vc_screenbuf_size,
+							false);
 					update_attr(vc);
 				}
 				break;
@@ -4743,7 +4745,7 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
 /* used by selection */
 u16 screen_glyph(const struct vc_data *vc, int offset)
 {
-	u16 w = scr_readw(screenpos(vc, offset, 1));
+	u16 w = scr_readw(screenpos(vc, offset, true));
 	u16 c = w & 0xff;
 
 	if (w & vc->vc_hi_font_mask)
@@ -4763,7 +4765,7 @@ u32 screen_glyph_unicode(const struct vc_data *vc, int n)
 EXPORT_SYMBOL_GPL(screen_glyph_unicode);
 
 /* used by vcs - note the word offset */
-unsigned short *screen_pos(const struct vc_data *vc, int w_offset, int viewed)
+unsigned short *screen_pos(const struct vc_data *vc, int w_offset, bool viewed)
 {
 	return screenpos(vc, 2 * w_offset, viewed);
 }
diff --git a/include/linux/selection.h b/include/linux/selection.h
index 15e36e7ef869..170ef28ff26b 100644
--- a/include/linux/selection.h
+++ b/include/linux/selection.h
@@ -34,11 +34,11 @@ extern unsigned char default_grn[];
 extern unsigned char default_blu[];
 
 extern unsigned short *screen_pos(const struct vc_data *vc, int w_offset,
-		int viewed);
+		bool viewed);
 extern u16 screen_glyph(const struct vc_data *vc, int offset);
 extern u32 screen_glyph_unicode(const struct vc_data *vc, int offset);
 extern void complement_pos(struct vc_data *vc, int offset);
-extern void invert_screen(struct vc_data *vc, int offset, int count, int shift);
+extern void invert_screen(struct vc_data *vc, int offset, int count, bool viewed);
 
 extern void getconsxy(const struct vc_data *vc, unsigned char xy[static 2]);
 extern void putconsxy(struct vc_data *vc, unsigned char xy[static const 2]);
@@ -49,7 +49,7 @@ extern void vcs_scr_updated(struct vc_data *vc);
 
 extern int vc_uniscr_check(struct vc_data *vc);
 extern void vc_uniscr_copy_line(const struct vc_data *vc, void *dest,
-				int viewed,
+				bool viewed,
 				unsigned int row, unsigned int col,
 				unsigned int nr);
 
-- 
2.28.0

[PATCH 04/16] vc_screen: document and cleanup vcs_vc

[Jiri Slaby]

Document parameters of vcs_vc and make viewed a bool.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 778f83ea2249..adc3c786b621 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -177,12 +177,14 @@ vcs_poll_data_get(struct file *file)
 	return poll;
 }
 
-/*
- * Returns VC for inode.
+/**
+ * vcs_vc -- return VC for @inode
+ * @inode: inode for which to return a VC
+ * @viewed: returns whether this console is currently foreground (viewed)
+ *
  * Must be called with console_lock.
  */
-static struct vc_data*
-vcs_vc(struct inode *inode, int *viewed)
+static struct vc_data *vcs_vc(struct inode *inode, bool *viewed)
 {
 	unsigned int currcons = console(inode);
 
@@ -191,11 +193,11 @@ vcs_vc(struct inode *inode, int *viewed)
 	if (currcons == 0) {
 		currcons = fg_console;
 		if (viewed)
-			*viewed = 1;
+			*viewed = true;
 	} else {
 		currcons--;
 		if (viewed)
-			*viewed = 0;
+			*viewed = false;
 	}
 	return vc_cons[currcons].d;
 }
@@ -247,10 +249,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct vc_data *vc;
 	struct vcs_poll_data *poll;
 	long pos, read;
-	int attr, uni_mode, row, col, maxcol, viewed;
+	int attr, uni_mode, row, col, maxcol;
 	unsigned short *org = NULL;
 	ssize_t ret;
 	char *con_buf;
+	bool viewed;
 
 	con_buf = (char *) __get_free_page(GFP_KERNEL);
 	if (!con_buf)
@@ -451,10 +454,11 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	long pos;
 	long attr, size, written;
 	char *con_buf0;
-	int col, maxcol, viewed;
+	int col, maxcol;
 	u16 *org0 = NULL, *org = NULL;
 	size_t ret;
 	char *con_buf;
+	bool viewed;
 
 	if (use_unicode(inode))
 		return -EOPNOTSUPP;
-- 
2.28.0

[PATCH 05/16] vc_screen: rewrite vcs_size to accept vc, not inode

[Jiri Slaby]

It is weird to fetch the information from the inode over and over. Read
and write already have the needed information, so rewrite vcs_size to
accept a vc, attr and unicode and adapt vcs_lseek to that.

Also make sure all sites check the return value of vcs_size for errors.

And document it using kernel-doc.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 46 ++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index adc3c786b621..8aa0d50bcac7 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -202,39 +202,47 @@ static struct vc_data *vcs_vc(struct inode *inode, bool *viewed)
 	return vc_cons[currcons].d;
 }
 
-/*
- * Returns size for VC carried by inode.
+/**
+ * vcs_size -- return size for a VC in @vc
+ * @vc: which VC
+ * @attr: does it use attributes?
+ * @unicode: is it unicode?
+ *
  * Must be called with console_lock.
  */
-static int
-vcs_size(struct inode *inode)
+static int vcs_size(const struct vc_data *vc, bool attr, bool unicode)
 {
 	int size;
-	struct vc_data *vc;
 
 	WARN_CONSOLE_UNLOCKED();
 
-	vc = vcs_vc(inode, NULL);
-	if (!vc)
-		return -ENXIO;
-
 	size = vc->vc_rows * vc->vc_cols;
 
-	if (use_attributes(inode)) {
-		if (use_unicode(inode))
+	if (attr) {
+		if (unicode)
 			return -EOPNOTSUPP;
-		size = 2*size + HEADER_SIZE;
-	} else if (use_unicode(inode))
+
+		size = 2 * size + HEADER_SIZE;
+	} else if (unicode)
 		size *= 4;
+
 	return size;
 }
 
 static loff_t vcs_lseek(struct file *file, loff_t offset, int orig)
 {
+	struct inode *inode = file_inode(file);
+	struct vc_data *vc;
 	int size;
 
 	console_lock();
-	size = vcs_size(file_inode(file));
+	vc = vcs_vc(inode, NULL);
+	if (!vc) {
+		console_unlock();
+		return -ENXIO;
+	}
+
+	size = vcs_size(vc, use_attributes(inode), use_unicode(inode));
 	console_unlock();
 	if (size < 0)
 		return size;
@@ -295,7 +303,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		 * as copy_to_user at the end of this loop
 		 * could sleep.
 		 */
-		size = vcs_size(inode);
+		size = vcs_size(vc, attr, uni_mode);
 		if (size < 0) {
 			if (read)
 				break;
@@ -480,7 +488,11 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (!vc)
 		goto unlock_out;
 
-	size = vcs_size(inode);
+	size = vcs_size(vc, attr, false);
+	if (size < 0) {
+		ret = size;
+		goto unlock_out;
+	}
 	ret = -EINVAL;
 	if (pos < 0 || pos > size)
 		goto unlock_out;
@@ -519,7 +531,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		 * the user buffer, so recheck.
 		 * Return data written up to now on failure.
 		 */
-		size = vcs_size(inode);
+		size = vcs_size(vc, attr, false);
 		if (size < 0) {
 			if (written)
 				break;
-- 
2.28.0

[PATCH 06/16] vc_screen: sanitize types in vcs_write

[Jiri Slaby]

* ret can carry error codes, so make it signed, i.e. ssize_t
* pos is derived from the passed ppos, so make it long enough, i.e.
  loff_t
* attr is a boolean, so...
* size is limited by vcs_size() which returns an int
* written, p, orig_count and this_round are always ">= 0" and "< size",
  so uint is enough
* col and max_col are derived from vc->vc_cols (uint) and p, so make
  them uint too
* place con_buf0 and con_buf declaration to a single line

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 8aa0d50bcac7..2571d9067b08 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -459,14 +459,13 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	struct inode *inode = file_inode(file);
 	struct vc_data *vc;
-	long pos;
-	long attr, size, written;
-	char *con_buf0;
-	int col, maxcol;
+	char *con_buf0, *con_buf;
 	u16 *org0 = NULL, *org = NULL;
-	size_t ret;
-	char *con_buf;
-	bool viewed;
+	unsigned int written, col, maxcol;
+	int size;
+	ssize_t ret;
+	loff_t pos;
+	bool viewed, attr;
 
 	if (use_unicode(inode))
 		return -EOPNOTSUPP;
@@ -500,9 +499,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		count = size - pos;
 	written = 0;
 	while (count) {
-		long this_round = count;
-		size_t orig_count;
-		long p;
+		unsigned int orig_count, p, this_round = count;
 
 		if (this_round > CON_BUF_SIZE)
 			this_round = CON_BUF_SIZE;
-- 
2.28.0

[PATCH 07/16] vc_screen: extract vcs_write_buf_noattr

[Jiri Slaby]

vcs_write is too long to be readable. Extract buffer handling w/o
attributes from there to a separate function.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 55 +++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 2571d9067b08..62e6d240f0dd 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -454,6 +454,33 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	return ret;
 }
 
+static u16 *vcs_write_buf_noattr(struct vc_data *vc, const char *con_buf,
+		unsigned int pos, unsigned int count, bool viewed, u16 **org0)
+{
+	u16 *org;
+	unsigned int col, maxcol = vc->vc_cols;
+
+	*org0 = org = screen_pos(vc, pos, viewed);
+	col = pos % maxcol;
+	pos += maxcol - col;
+
+	while (count > 0) {
+		unsigned char c = *con_buf++;
+
+		count--;
+		vcs_scr_writew(vc,
+			       (vcs_scr_readw(vc, org) & 0xff00) | c, org);
+		org++;
+		if (++col == maxcol) {
+			org = screen_pos(vc, pos, viewed);
+			col = 0;
+			pos += maxcol;
+		}
+	}
+
+	return org;
+}
+
 static ssize_t
 vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
@@ -544,29 +571,15 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		 * under the lock using the local kernel buffer.
 		 */
 
-		con_buf0 = con_buf;
-		orig_count = this_round;
-		maxcol = vc->vc_cols;
-		p = pos;
 		if (!attr) {
-			org0 = org = screen_pos(vc, p, viewed);
-			col = p % maxcol;
-			p += maxcol - col;
-
-			while (this_round > 0) {
-				unsigned char c = *con_buf0++;
-
-				this_round--;
-				vcs_scr_writew(vc,
-					       (vcs_scr_readw(vc, org) & 0xff00) | c, org);
-				org++;
-				if (++col == maxcol) {
-					org = screen_pos(vc, p, viewed);
-					col = 0;
-					p += maxcol;
-				}
-			}
+			org = vcs_write_buf_noattr(vc, con_buf, pos, this_round,
+					viewed, &org0);
 		} else {
+			con_buf0 = con_buf;
+			orig_count = this_round;
+			maxcol = vc->vc_cols;
+			p = pos;
+
 			if (p < HEADER_SIZE) {
 				char header[HEADER_SIZE];
 
-- 
2.28.0

[PATCH 08/16] vc_screen: extract vcs_write_buf

[Jiri Slaby]

This is the counterpart of the previous patch: here, we extract buffer
writing with attributes from vcs_write.

Now, there is no need for org to be initialized to NULL. The org0
check before update_region() confuses compilers, so check org instead.
It provides the same semantics. And it also eliminates the need for
initialization of org0.

We switch the branches of the attr 'if' too, as the inversion brings only
confusion now.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 168 ++++++++++++++++++++-----------------
 1 file changed, 92 insertions(+), 76 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 62e6d240f0dd..10a26fd5f1b7 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -481,14 +481,93 @@ static u16 *vcs_write_buf_noattr(struct vc_data *vc, const char *con_buf,
 	return org;
 }
 
+static u16 *vcs_write_buf(struct vc_data *vc, const char *con_buf,
+		unsigned int pos, unsigned int count, bool viewed, u16 **org0)
+{
+	u16 *org;
+	unsigned int col, maxcol = vc->vc_cols;
+	unsigned char c;
+
+	/* header */
+	if (pos < HEADER_SIZE) {
+		char header[HEADER_SIZE];
+
+		getconsxy(vc, header + 2);
+		while (pos < HEADER_SIZE && count > 0) {
+			count--;
+			header[pos++] = *con_buf++;
+		}
+		if (!viewed)
+			putconsxy(vc, header + 2);
+	}
+
+	if (!count)
+		return NULL;
+
+	pos -= HEADER_SIZE;
+	col = (pos/2) % maxcol;
+
+	*org0 = org = screen_pos(vc, pos/2, viewed);
+
+	/* odd pos -- the first single character */
+	if (pos & 1) {
+		count--;
+		c = *con_buf++;
+#ifdef __BIG_ENDIAN
+		vcs_scr_writew(vc, c |
+		     (vcs_scr_readw(vc, org) & 0xff00), org);
+#else
+		vcs_scr_writew(vc, (c << 8) |
+		     (vcs_scr_readw(vc, org) & 0xff), org);
+#endif
+		org++;
+		pos++;
+		if (++col == maxcol) {
+			org = screen_pos(vc, pos/2, viewed);
+			col = 0;
+		}
+	}
+
+	pos /= 2;
+	pos += maxcol - col;
+
+	/* even pos -- handle attr+character pairs */
+	while (count > 1) {
+		unsigned short w;
+
+		w = get_unaligned(((unsigned short *)con_buf));
+		vcs_scr_writew(vc, w, org++);
+		con_buf += 2;
+		count -= 2;
+		if (++col == maxcol) {
+			org = screen_pos(vc, pos, viewed);
+			col = 0;
+			pos += maxcol;
+		}
+	}
+
+	if (!count)
+		return org;
+
+	/* odd pos -- the remaining character */
+	c = *con_buf++;
+#ifdef __BIG_ENDIAN
+	vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff) | (c << 8), org);
+#else
+	vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff00) | c, org);
+#endif
+
+	return org;
+}
+
 static ssize_t
 vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	struct inode *inode = file_inode(file);
 	struct vc_data *vc;
-	char *con_buf0, *con_buf;
-	u16 *org0 = NULL, *org = NULL;
-	unsigned int written, col, maxcol;
+	char *con_buf;
+	u16 *org0, *org;
+	unsigned int written;
 	int size;
 	ssize_t ret;
 	loff_t pos;
@@ -526,7 +605,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		count = size - pos;
 	written = 0;
 	while (count) {
-		unsigned int orig_count, p, this_round = count;
+		unsigned int this_round = count;
 
 		if (this_round > CON_BUF_SIZE)
 			this_round = CON_BUF_SIZE;
@@ -571,81 +650,18 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		 * under the lock using the local kernel buffer.
 		 */
 
-		if (!attr) {
+		if (attr)
+			org = vcs_write_buf(vc, con_buf, pos, this_round,
+					viewed, &org0);
+		else
 			org = vcs_write_buf_noattr(vc, con_buf, pos, this_round,
 					viewed, &org0);
-		} else {
-			con_buf0 = con_buf;
-			orig_count = this_round;
-			maxcol = vc->vc_cols;
-			p = pos;
-
-			if (p < HEADER_SIZE) {
-				char header[HEADER_SIZE];
-
-				getconsxy(vc, header + 2);
-				while (p < HEADER_SIZE && this_round > 0) {
-					this_round--;
-					header[p++] = *con_buf0++;
-				}
-				if (!viewed)
-					putconsxy(vc, header + 2);
-			}
-			p -= HEADER_SIZE;
-			col = (p/2) % maxcol;
-			if (this_round > 0) {
-				org0 = org = screen_pos(vc, p/2, viewed);
-				if ((p & 1) && this_round > 0) {
-					char c;
-
-					this_round--;
-					c = *con_buf0++;
-#ifdef __BIG_ENDIAN
-					vcs_scr_writew(vc, c |
-					     (vcs_scr_readw(vc, org) & 0xff00), org);
-#else
-					vcs_scr_writew(vc, (c << 8) |
-					     (vcs_scr_readw(vc, org) & 0xff), org);
-#endif
-					org++;
-					p++;
-					if (++col == maxcol) {
-						org = screen_pos(vc, p/2, viewed);
-						col = 0;
-					}
-				}
-				p /= 2;
-				p += maxcol - col;
-			}
-			while (this_round > 1) {
-				unsigned short w;
 
-				w = get_unaligned(((unsigned short *)con_buf0));
-				vcs_scr_writew(vc, w, org++);
-				con_buf0 += 2;
-				this_round -= 2;
-				if (++col == maxcol) {
-					org = screen_pos(vc, p, viewed);
-					col = 0;
-					p += maxcol;
-				}
-			}
-			if (this_round > 0) {
-				unsigned char c;
-
-				c = *con_buf0++;
-#ifdef __BIG_ENDIAN
-				vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff) | (c << 8), org);
-#else
-				vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff00) | c, org);
-#endif
-			}
-		}
-		count -= orig_count;
-		written += orig_count;
-		buf += orig_count;
-		pos += orig_count;
-		if (org0)
+		count -= this_round;
+		written += this_round;
+		buf += this_round;
+		pos += this_round;
+		if (org)
 			update_region(vc, (unsigned long)(org0), org - org0);
 	}
 	*ppos += written;
-- 
2.28.0

[PATCH 09/16] vc_screen: eliminate ifdefs from vcs_write_buf

[Jiri Slaby]

Introduce a new inline function called vc_compile_le16 and do the shifts
and ORs there. Depending on LE x BE.

I tried cpu_to_le16, but it ends up with worse assembly on BE for
whatever reason -- the compiler seems to be unable to optimize the swap.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 10a26fd5f1b7..36b967825f68 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -481,6 +481,19 @@ static u16 *vcs_write_buf_noattr(struct vc_data *vc, const char *con_buf,
 	return org;
 }
 
+/*
+ * Compilers (gcc 10) are unable to optimize the swap in cpu_to_le16. So do it
+ * the poor man way.
+ */
+static inline u16 vc_compile_le16(u8 hi, u8 lo)
+{
+#ifdef __BIG_ENDIAN
+	return (lo << 8u) | hi;
+#else
+	return (hi << 8u) | lo;
+#endif
+}
+
 static u16 *vcs_write_buf(struct vc_data *vc, const char *con_buf,
 		unsigned int pos, unsigned int count, bool viewed, u16 **org0)
 {
@@ -513,13 +526,8 @@ static u16 *vcs_write_buf(struct vc_data *vc, const char *con_buf,
 	if (pos & 1) {
 		count--;
 		c = *con_buf++;
-#ifdef __BIG_ENDIAN
-		vcs_scr_writew(vc, c |
-		     (vcs_scr_readw(vc, org) & 0xff00), org);
-#else
-		vcs_scr_writew(vc, (c << 8) |
-		     (vcs_scr_readw(vc, org) & 0xff), org);
-#endif
+		vcs_scr_writew(vc, vc_compile_le16(c, vcs_scr_readw(vc, org)),
+				org);
 		org++;
 		pos++;
 		if (++col == maxcol) {
@@ -551,11 +559,8 @@ static u16 *vcs_write_buf(struct vc_data *vc, const char *con_buf,
 
 	/* odd pos -- the remaining character */
 	c = *con_buf++;
-#ifdef __BIG_ENDIAN
-	vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff) | (c << 8), org);
-#else
-	vcs_scr_writew(vc, (vcs_scr_readw(vc, org) & 0xff00) | c, org);
-#endif
+	vcs_scr_writew(vc, vc_compile_le16(vcs_scr_readw(vc, org) >> 8, c),
+				org);
 
 	return org;
 }
-- 
2.28.0

[PATCH 10/16] vc_screen: sanitize types in vcs_read

[Jiri Slaby]

* pos is derived from the passed ppos, so make it long enough, i.e.
  loff_t
* attr and uni_mode are booleans, so...
* size is limited by vcs_size() which returns an int
* read, p, orig_count and this_round are always ">= 0" and "< size",
  so uint is enough
* row, col, and max_col are derived from vc->vc_cols (uint) and p, so
  make them uint too
* tmp_count is derived from this_round, so make it an uint too.
* use u16 * for org (instead of unsigned short *). No need to initialize
  org too.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 36b967825f68..c62c590ed816 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -256,12 +256,12 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct inode *inode = file_inode(file);
 	struct vc_data *vc;
 	struct vcs_poll_data *poll;
-	long pos, read;
-	int attr, uni_mode, row, col, maxcol;
-	unsigned short *org = NULL;
+	u16 *org;
+	unsigned int read, row, col, maxcol;
 	ssize_t ret;
 	char *con_buf;
-	bool viewed;
+	loff_t pos;
+	bool viewed, attr, uni_mode;
 
 	con_buf = (char *) __get_free_page(GFP_KERNEL);
 	if (!con_buf)
@@ -295,9 +295,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	ret = 0;
 	while (count) {
 		char *con_buf0, *con_buf_start;
-		long this_round, size;
-		ssize_t orig_count;
-		long p = pos;
+		unsigned int this_round, orig_count, p = pos;
+		int size;
 
 		/* Check whether we are above size each round,
 		 * as copy_to_user at the end of this loop
@@ -362,7 +361,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			}
 		} else {
 			if (p < HEADER_SIZE) {
-				size_t tmp_count;
+				unsigned int tmp_count;
 
 				/* clamp header values if they don't fit */
 				con_buf0[0] = min(vc->vc_rows, 0xFFu);
-- 
2.28.0

[PATCH 11/16] vs_screen: kill tmp_count from vcs_read

[Jiri Slaby]

Both tmp_count computations and the single use can be eliminated using
min(). Do so.

Side note: we need HEADER_SIZE to be unsigned for min() not to complain.
Fix that too as all its other uses do not mind.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index c62c590ed816..8ebb6724a804 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -53,7 +53,7 @@
 #undef attr
 #undef org
 #undef addr
-#define HEADER_SIZE	4
+#define HEADER_SIZE	4u
 
 #define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
 
@@ -361,8 +361,6 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			}
 		} else {
 			if (p < HEADER_SIZE) {
-				unsigned int tmp_count;
-
 				/* clamp header values if they don't fit */
 				con_buf0[0] = min(vc->vc_rows, 0xFFu);
 				con_buf0[1] = min(vc->vc_cols, 0xFFu);
@@ -375,12 +373,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 					orig_count = this_round - p;
 				}
 
-				tmp_count = HEADER_SIZE;
-				if (tmp_count > this_round)
-					tmp_count = this_round;
-
 				/* Advance state pointers and move on. */
-				this_round -= tmp_count;
+				this_round -= min(HEADER_SIZE, this_round);
 				p = HEADER_SIZE;
 				con_buf0 = con_buf + HEADER_SIZE;
 				/* If this_round >= 0, then p is even... */
-- 
2.28.0

[PATCH 12/16] vc_screen: extract vcs_read_buf_uni

[Jiri Slaby]

The same as making write more readable, extract unicode handling from
vcs_read. The other two cases (w/ and w/o attributes) will follow.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 49 +++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 8ebb6724a804..91d2e0148a67 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -249,6 +249,33 @@ static loff_t vcs_lseek(struct file *file, loff_t offset, int orig)
 	return fixed_size_llseek(file, offset, orig, size);
 }
 
+static int vcs_read_buf_uni(struct vc_data *vc, char *con_buf,
+		unsigned int pos, unsigned int count, bool viewed)
+{
+	unsigned int nr, row, col, maxcol = vc->vc_cols;
+	int ret;
+
+	ret = vc_uniscr_check(vc);
+	if (ret)
+		return ret;
+
+	pos /= 4;
+	row = pos / maxcol;
+	col = pos % maxcol;
+	nr = maxcol - col;
+	do {
+		if (nr > count / 4)
+			nr = count / 4;
+		vc_uniscr_copy_line(vc, con_buf, viewed, row, col, nr);
+		con_buf += nr * 4;
+		count -= nr * 4;
+		row++;
+		col = 0;
+		nr = maxcol;
+	} while (count);
+
+	return 0;
+}
 
 static ssize_t
 vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
@@ -257,7 +284,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct vc_data *vc;
 	struct vcs_poll_data *poll;
 	u16 *org;
-	unsigned int read, row, col, maxcol;
+	unsigned int read, col, maxcol;
 	ssize_t ret;
 	char *con_buf;
 	loff_t pos;
@@ -327,26 +354,10 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		orig_count = this_round;
 		maxcol = vc->vc_cols;
 		if (uni_mode) {
-			unsigned int nr;
-
-			ret = vc_uniscr_check(vc);
+			ret = vcs_read_buf_uni(vc, con_buf, pos, this_round,
+					viewed);
 			if (ret)
 				break;
-			p /= 4;
-			row = p / vc->vc_cols;
-			col = p % maxcol;
-			nr = maxcol - col;
-			do {
-				if (nr > this_round/4)
-					nr = this_round/4;
-				vc_uniscr_copy_line(vc, con_buf0, viewed,
-						    row, col, nr);
-				con_buf0 += nr * 4;
-				this_round -= nr * 4;
-				row++;
-				col = 0;
-				nr = maxcol;
-			} while (this_round);
 		} else if (!attr) {
 			org = screen_pos(vc, p, viewed);
 			col = p % maxcol;
-- 
2.28.0

[PATCH 13/16] vc_screen: extract vcs_read_buf_noattr

[Jiri Slaby]

Now, move the code for no-attributes handling to a separate function.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 91d2e0148a67..ad015cd4e82f 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -277,6 +277,26 @@ static int vcs_read_buf_uni(struct vc_data *vc, char *con_buf,
 	return 0;
 }
 
+static void vcs_read_buf_noattr(const struct vc_data *vc, char *con_buf,
+		unsigned int pos, unsigned int count, bool viewed)
+{
+	u16 *org;
+	unsigned int col, maxcol = vc->vc_cols;
+
+	org = screen_pos(vc, pos, viewed);
+	col = pos % maxcol;
+	pos += maxcol - col;
+
+	while (count-- > 0) {
+		*con_buf++ = (vcs_scr_readw(vc, org++) & 0xff);
+		if (++col == maxcol) {
+			org = screen_pos(vc, pos, viewed);
+			col = 0;
+			pos += maxcol;
+		}
+	}
+}
+
 static ssize_t
 vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
@@ -359,17 +379,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			if (ret)
 				break;
 		} else if (!attr) {
-			org = screen_pos(vc, p, viewed);
-			col = p % maxcol;
-			p += maxcol - col;
-			while (this_round-- > 0) {
-				*con_buf0++ = (vcs_scr_readw(vc, org++) & 0xff);
-				if (++col == maxcol) {
-					org = screen_pos(vc, p, viewed);
-					col = 0;
-					p += maxcol;
-				}
-			}
+			vcs_read_buf_noattr(vc, con_buf, pos, this_round,
+					viewed);
 		} else {
 			if (p < HEADER_SIZE) {
 				/* clamp header values if they don't fit */
-- 
2.28.0

[PATCH 14/16] vc_screen: extract vcs_read_buf

[Jiri Slaby]

And finally, move the attributes buffer handling to a separate function.
Leaving vcs_read quite compact.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 146 ++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 68 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index ad015cd4e82f..c178a1611223 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -297,14 +297,81 @@ static void vcs_read_buf_noattr(const struct vc_data *vc, char *con_buf,
 	}
 }
 
+static unsigned int vcs_read_buf(const struct vc_data *vc, char *con_buf,
+		unsigned int pos, unsigned int count, bool viewed,
+		unsigned int *skip)
+{
+	u16 *org, *con_buf16;
+	unsigned int col, maxcol = vc->vc_cols;
+	unsigned int filled = count;
+
+	if (pos < HEADER_SIZE) {
+		/* clamp header values if they don't fit */
+		con_buf[0] = min(vc->vc_rows, 0xFFu);
+		con_buf[1] = min(vc->vc_cols, 0xFFu);
+		getconsxy(vc, con_buf + 2);
+
+		*skip += pos;
+		count += pos;
+		if (count > CON_BUF_SIZE) {
+			count = CON_BUF_SIZE;
+			filled = count - pos;
+		}
+
+		/* Advance state pointers and move on. */
+		count -= min(HEADER_SIZE, count);
+		pos = HEADER_SIZE;
+		con_buf += HEADER_SIZE;
+		/* If count >= 0, then pos is even... */
+	} else if (pos & 1) {
+		/*
+		 * Skip first byte for output if start address is odd. Update
+		 * region sizes up/down depending on free space in buffer.
+		 */
+		(*skip)++;
+		if (count < CON_BUF_SIZE)
+			count++;
+		else
+			filled--;
+	}
+
+	if (!count)
+		return filled;
+
+	pos -= HEADER_SIZE;
+	pos /= 2;
+	col = pos % maxcol;
+
+	org = screen_pos(vc, pos, viewed);
+	pos += maxcol - col;
+
+	/*
+	 * Buffer has even length, so we can always copy character + attribute.
+	 * We do not copy last byte to userspace if count is odd.
+	 */
+	count = (count + 1) / 2;
+	con_buf16 = (u16 *)con_buf;
+
+	while (count) {
+		*con_buf16++ = vcs_scr_readw(vc, org++);
+		count--;
+		if (++col == maxcol) {
+			org = screen_pos(vc, pos, viewed);
+			col = 0;
+			pos += maxcol;
+		}
+	}
+
+	return filled;
+}
+
 static ssize_t
 vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	struct inode *inode = file_inode(file);
 	struct vc_data *vc;
 	struct vcs_poll_data *poll;
-	u16 *org;
-	unsigned int read, col, maxcol;
+	unsigned int read;
 	ssize_t ret;
 	char *con_buf;
 	loff_t pos;
@@ -341,8 +408,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	read = 0;
 	ret = 0;
 	while (count) {
-		char *con_buf0, *con_buf_start;
-		unsigned int this_round, orig_count, p = pos;
+		unsigned int this_round, skip = 0;
 		int size;
 
 		/* Check whether we are above size each round,
@@ -370,9 +436,6 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		 * attempt to move it to userspace.
 		 */
 
-		con_buf_start = con_buf0 = con_buf;
-		orig_count = this_round;
-		maxcol = vc->vc_cols;
 		if (uni_mode) {
 			ret = vcs_read_buf_uni(vc, con_buf, pos, this_round,
 					viewed);
@@ -382,61 +445,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			vcs_read_buf_noattr(vc, con_buf, pos, this_round,
 					viewed);
 		} else {
-			if (p < HEADER_SIZE) {
-				/* clamp header values if they don't fit */
-				con_buf0[0] = min(vc->vc_rows, 0xFFu);
-				con_buf0[1] = min(vc->vc_cols, 0xFFu);
-				getconsxy(vc, con_buf0 + 2);
-
-				con_buf_start += p;
-				this_round += p;
-				if (this_round > CON_BUF_SIZE) {
-					this_round = CON_BUF_SIZE;
-					orig_count = this_round - p;
-				}
-
-				/* Advance state pointers and move on. */
-				this_round -= min(HEADER_SIZE, this_round);
-				p = HEADER_SIZE;
-				con_buf0 = con_buf + HEADER_SIZE;
-				/* If this_round >= 0, then p is even... */
-			} else if (p & 1) {
-				/* Skip first byte for output if start address is odd
-				 * Update region sizes up/down depending on free
-				 * space in buffer.
-				 */
-				con_buf_start++;
-				if (this_round < CON_BUF_SIZE)
-					this_round++;
-				else
-					orig_count--;
-			}
-			if (this_round > 0) {
-				unsigned short *tmp_buf = (unsigned short *)con_buf0;
-
-				p -= HEADER_SIZE;
-				p /= 2;
-				col = p % maxcol;
-
-				org = screen_pos(vc, p, viewed);
-				p += maxcol - col;
-
-				/* Buffer has even length, so we can always copy
-				 * character + attribute. We do not copy last byte
-				 * to userspace if this_round is odd.
-				 */
-				this_round = (this_round + 1) >> 1;
-
-				while (this_round) {
-					*tmp_buf++ = vcs_scr_readw(vc, org++);
-					this_round --;
-					if (++col == maxcol) {
-						org = screen_pos(vc, p, viewed);
-						col = 0;
-						p += maxcol;
-					}
-				}
-			}
+			this_round = vcs_read_buf(vc, con_buf, pos, this_round,
+					viewed, &skip);
 		}
 
 		/* Finally, release the console semaphore while we push
@@ -447,18 +457,18 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		 */
 
 		console_unlock();
-		ret = copy_to_user(buf, con_buf_start, orig_count);
+		ret = copy_to_user(buf, con_buf + skip, this_round);
 		console_lock();
 
 		if (ret) {
-			read += (orig_count - ret);
+			read += this_round - ret;
 			ret = -EFAULT;
 			break;
 		}
-		buf += orig_count;
-		pos += orig_count;
-		read += orig_count;
-		count -= orig_count;
+		buf += this_round;
+		pos += this_round;
+		read += this_round;
+		count -= this_round;
 	}
 	*ppos += read;
 	if (read)
-- 
2.28.0

[PATCH 15/16] vc_screen: extract vcs_read_buf_header

[Jiri Slaby]

The attribute header handling is terrible in vcs_read_buf. Separate it
to a new function and simply do memmove (of up to 4 bytes) to the start
of the con_buf -- if user seeked.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index c178a1611223..9d68c6b36ddf 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -297,6 +297,22 @@ static void vcs_read_buf_noattr(const struct vc_data *vc, char *con_buf,
 	}
 }
 
+static unsigned int vcs_read_buf_header(const struct vc_data *vc, char *con_buf,
+		unsigned int pos, unsigned int count)
+{
+	count = min(HEADER_SIZE - pos, count);
+
+	/* clamp header values if they don't fit */
+	con_buf[0] = min(vc->vc_rows, 0xFFu);
+	con_buf[1] = min(vc->vc_cols, 0xFFu);
+	getconsxy(vc, con_buf + 2);
+
+	if (pos)
+		memmove(con_buf, con_buf + pos, count);
+
+	return count;
+}
+
 static unsigned int vcs_read_buf(const struct vc_data *vc, char *con_buf,
 		unsigned int pos, unsigned int count, bool viewed,
 		unsigned int *skip)
@@ -306,22 +322,11 @@ static unsigned int vcs_read_buf(const struct vc_data *vc, char *con_buf,
 	unsigned int filled = count;
 
 	if (pos < HEADER_SIZE) {
-		/* clamp header values if they don't fit */
-		con_buf[0] = min(vc->vc_rows, 0xFFu);
-		con_buf[1] = min(vc->vc_cols, 0xFFu);
-		getconsxy(vc, con_buf + 2);
-
-		*skip += pos;
-		count += pos;
-		if (count > CON_BUF_SIZE) {
-			count = CON_BUF_SIZE;
-			filled = count - pos;
-		}
+		count -= vcs_read_buf_header(vc, con_buf, pos, count);
 
-		/* Advance state pointers and move on. */
-		count -= min(HEADER_SIZE, count);
 		pos = HEADER_SIZE;
 		con_buf += HEADER_SIZE;
+
 		/* If count >= 0, then pos is even... */
 	} else if (pos & 1) {
 		/*
-- 
2.28.0

[PATCH 16/16] vc_screen: prune macros

[Jiri Slaby]

Do not undefine random words. I guess this was here as there were macros
with such generic names somewhere. I very doubt they still exist. So
drop these.

And remove a spare blank line.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/vt/vc_screen.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 9d68c6b36ddf..3e3a6a9a7f44 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -50,11 +50,7 @@
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
 
-#undef attr
-#undef org
-#undef addr
 #define HEADER_SIZE	4u
-
 #define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
 
 /*
-- 
2.28.0

Re: [PATCH 02/16] vt: declare xy for get/putconsxy properly

[Greg KH]

On Tue, Aug 18, 2020 at 10:56:52AM +0200, Jiri Slaby wrote:
> That is:
> 1) call the parameter 'xy' to denote what it really is, not generic 'p'
> 2) tell the compiler and users that we expect an array:
>    * with at least 2 chars (static 2)
>    * which we don't modify in putconsxy (const)
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/tty/vt/vt.c       | 10 +++++-----
>  include/linux/selection.h |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 8f283221330e..a0da7771c327 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4769,17 +4769,17 @@ unsigned short *screen_pos(const struct vc_data *vc, int w_offset, int viewed)
>  }
>  EXPORT_SYMBOL_GPL(screen_pos);
>  
> -void getconsxy(const struct vc_data *vc, unsigned char *p)
> +void getconsxy(const struct vc_data *vc, unsigned char xy[static 2])

I didn't realize we could do "[static 2]" in the kernel now, is that
thanks to the bump of the minimum gcc version?  If so, nice!

thanks,

greg k-h

Re: [PATCH 02/16] vt: declare xy for get/putconsxy properly

[Jiri Slaby]

On 18. 08. 20, 13:43, Greg KH wrote:
> On Tue, Aug 18, 2020 at 10:56:52AM +0200, Jiri Slaby wrote:
>> That is:
>> 1) call the parameter 'xy' to denote what it really is, not generic 'p'
>> 2) tell the compiler and users that we expect an array:
>>    * with at least 2 chars (static 2)
>>    * which we don't modify in putconsxy (const)
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>  drivers/tty/vt/vt.c       | 10 +++++-----
>>  include/linux/selection.h |  4 ++--
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
>> index 8f283221330e..a0da7771c327 100644
>> --- a/drivers/tty/vt/vt.c
>> +++ b/drivers/tty/vt/vt.c
>> @@ -4769,17 +4769,17 @@ unsigned short *screen_pos(const struct vc_data *vc, int w_offset, int viewed)
>>  }
>>  EXPORT_SYMBOL_GPL(screen_pos);
>>  
>> -void getconsxy(const struct vc_data *vc, unsigned char *p)
>> +void getconsxy(const struct vc_data *vc, unsigned char xy[static 2])
> 
> I didn't realize we could do "[static 2]" in the kernel now, is that
> thanks to the bump of the minimum gcc version?  If so, nice!

gcc supports it since some time around 3.x (these kinds of declaration
are defined in c99). So hopefully nothing breaks.

thanks,
-- 
js
suse labs

Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header

[Peilin Ye]

Hi all,

Link: https://syzkaller.appspot.com/bug?id=f332576321998d36cd07d09c9c1268cfed1895c9

As reported by syzbot, vcs_read_buf() is overflowing `con_buf16`, since
this patch removed the following check:

-		if (count > CON_BUF_SIZE) {
-			count = CON_BUF_SIZE;
-			filled = count - pos;
-		}

Decreasing `count` by `min(HEADER_SIZE - pos, count)` bypasses this check.
Additionally, this patch also removed updates to `skip` and `filled`.

What should we do in order to fix it?

Thank you,
Peilin Ye

Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header

[Greg KH]

On Tue, Aug 25, 2020 at 12:48:04PM -0400, Peilin Ye wrote:
> Hi all,
> 
> Link: https://syzkaller.appspot.com/bug?id=f332576321998d36cd07d09c9c1268cfed1895c9
> 
> As reported by syzbot, vcs_read_buf() is overflowing `con_buf16`, since
> this patch removed the following check:
> 
> -		if (count > CON_BUF_SIZE) {
> -			count = CON_BUF_SIZE;
> -			filled = count - pos;
> -		}
> 
> Decreasing `count` by `min(HEADER_SIZE - pos, count)` bypasses this check.
> Additionally, this patch also removed updates to `skip` and `filled`.
> 
> What should we do in order to fix it?

This patch is already reverted, and it has been discussed a bit as to
how to do this properly if you look at the email where this was reported
to us.

thanks,

greg k-h

Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header

[Peilin Ye]

On Tue, Aug 25, 2020 at 06:59:35PM +0200, Greg KH wrote:
> On Tue, Aug 25, 2020 at 12:48:04PM -0400, Peilin Ye wrote:
> > Hi all,
> > 
> > Link: https://syzkaller.appspot.com/bug?id=f332576321998d36cd07d09c9c1268cfed1895c9
> > 
> > As reported by syzbot, vcs_read_buf() is overflowing `con_buf16`, since
> > this patch removed the following check:
> > 
> > -		if (count > CON_BUF_SIZE) {
> > -			count = CON_BUF_SIZE;
> > -			filled = count - pos;
> > -		}
> > 
> > Decreasing `count` by `min(HEADER_SIZE - pos, count)` bypasses this check.
> > Additionally, this patch also removed updates to `skip` and `filled`.
> > 
> > What should we do in order to fix it?
> 
> This patch is already reverted, and it has been discussed a bit as to
> how to do this properly if you look at the email where this was reported
> to us.

Ah, syzbot has reported this issue as two different bugs...I started
looking into it without noticing your discussion under another thread.

Sorry,
Peilin Ye