* [PATCH 02/16] vt: declare xy for get/putconsxy properly
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 11:43 ` Greg KH
2020-08-18 8:56 ` [PATCH 03/16] vc: propagate "viewed as bool" from screenpos up Jiri Slaby
` (13 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 02/16] vt: declare xy for get/putconsxy properly
2020-08-18 8:56 ` [PATCH 02/16] vt: declare xy for get/putconsxy properly Jiri Slaby
@ 2020-08-18 11:43 ` Greg KH
2020-08-19 7:09 ` Jiri Slaby
0 siblings, 1 reply; 21+ messages in thread
From: Greg KH @ 2020-08-18 11:43 UTC (permalink / raw)
To: Jiri Slaby; +Cc: linux-serial, linux-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 02/16] vt: declare xy for get/putconsxy properly
2020-08-18 11:43 ` Greg KH
@ 2020-08-19 7:09 ` Jiri Slaby
0 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-19 7:09 UTC (permalink / raw)
To: Greg KH; +Cc: linux-serial, linux-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 03/16] vc: propagate "viewed as bool" from screenpos up
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
2020-08-18 8:56 ` [PATCH 02/16] vt: declare xy for get/putconsxy properly Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 04/16] vc_screen: document and cleanup vcs_vc Jiri Slaby
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/16] vc_screen: document and cleanup vcs_vc
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
2020-08-18 8:56 ` [PATCH 02/16] vt: declare xy for get/putconsxy properly Jiri Slaby
2020-08-18 8:56 ` [PATCH 03/16] vc: propagate "viewed as bool" from screenpos up Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 05/16] vc_screen: rewrite vcs_size to accept vc, not inode Jiri Slaby
` (11 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/16] vc_screen: rewrite vcs_size to accept vc, not inode
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (2 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 04/16] vc_screen: document and cleanup vcs_vc Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 06/16] vc_screen: sanitize types in vcs_write Jiri Slaby
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/16] vc_screen: sanitize types in vcs_write
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (3 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 05/16] vc_screen: rewrite vcs_size to accept vc, not inode Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 07/16] vc_screen: extract vcs_write_buf_noattr Jiri Slaby
` (9 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/16] vc_screen: extract vcs_write_buf_noattr
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (4 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 06/16] vc_screen: sanitize types in vcs_write Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 08/16] vc_screen: extract vcs_write_buf Jiri Slaby
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/16] vc_screen: extract vcs_write_buf
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (5 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 07/16] vc_screen: extract vcs_write_buf_noattr Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:56 ` [PATCH 09/16] vc_screen: eliminate ifdefs from vcs_write_buf Jiri Slaby
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/16] vc_screen: eliminate ifdefs from vcs_write_buf
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (6 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 08/16] vc_screen: extract vcs_write_buf Jiri Slaby
@ 2020-08-18 8:56 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 10/16] vc_screen: sanitize types in vcs_read Jiri Slaby
` (6 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:56 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/16] vc_screen: sanitize types in vcs_read
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (7 preceding siblings ...)
2020-08-18 8:56 ` [PATCH 09/16] vc_screen: eliminate ifdefs from vcs_write_buf Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 11/16] vs_screen: kill tmp_count from vcs_read Jiri Slaby
` (5 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/16] vs_screen: kill tmp_count from vcs_read
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (8 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 10/16] vc_screen: sanitize types in vcs_read Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 12/16] vc_screen: extract vcs_read_buf_uni Jiri Slaby
` (4 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 12/16] vc_screen: extract vcs_read_buf_uni
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (9 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 11/16] vs_screen: kill tmp_count from vcs_read Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 13/16] vc_screen: extract vcs_read_buf_noattr Jiri Slaby
` (3 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/16] vc_screen: extract vcs_read_buf_noattr
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (10 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 12/16] vc_screen: extract vcs_read_buf_uni Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 14/16] vc_screen: extract vcs_read_buf Jiri Slaby
` (2 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 14/16] vc_screen: extract vcs_read_buf
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (11 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 13/16] vc_screen: extract vcs_read_buf_noattr Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-18 8:57 ` [PATCH 15/16] vc_screen: extract vcs_read_buf_header Jiri Slaby
2020-08-18 8:57 ` [PATCH 16/16] vc_screen: prune macros Jiri Slaby
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 15/16] vc_screen: extract vcs_read_buf_header
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (12 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 14/16] vc_screen: extract vcs_read_buf Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
2020-08-25 16:48 ` Peilin Ye
2020-08-18 8:57 ` [PATCH 16/16] vc_screen: prune macros Jiri Slaby
14 siblings, 1 reply; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header
2020-08-18 8:57 ` [PATCH 15/16] vc_screen: extract vcs_read_buf_header Jiri Slaby
@ 2020-08-25 16:48 ` Peilin Ye
2020-08-25 16:59 ` Greg KH
0 siblings, 1 reply; 21+ messages in thread
From: Peilin Ye @ 2020-08-25 16:48 UTC (permalink / raw)
To: jslaby
Cc: gregkh, linux-kernel, linux-serial, syzkaller-bugs, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header
2020-08-25 16:48 ` Peilin Ye
@ 2020-08-25 16:59 ` Greg KH
2020-08-25 17:12 ` Peilin Ye
0 siblings, 1 reply; 21+ messages in thread
From: Greg KH @ 2020-08-25 16:59 UTC (permalink / raw)
To: Peilin Ye
Cc: jslaby, linux-kernel, linux-serial, syzkaller-bugs, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 15/16] vc_screen: extract vcs_read_buf_header
2020-08-25 16:59 ` Greg KH
@ 2020-08-25 17:12 ` Peilin Ye
0 siblings, 0 replies; 21+ messages in thread
From: Peilin Ye @ 2020-08-25 17:12 UTC (permalink / raw)
To: Greg KH
Cc: jslaby, linux-kernel, linux-serial, syzkaller-bugs, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 16/16] vc_screen: prune macros
2020-08-18 8:56 [PATCH 01/16] vt: make vc_data pointers const in selection.h Jiri Slaby
` (13 preceding siblings ...)
2020-08-18 8:57 ` [PATCH 15/16] vc_screen: extract vcs_read_buf_header Jiri Slaby
@ 2020-08-18 8:57 ` Jiri Slaby
14 siblings, 0 replies; 21+ messages in thread
From: Jiri Slaby @ 2020-08-18 8:57 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread