From: David Laight <david.laight.linux@gmail.com>

The row/column bounds (for a screen window box) are changed from
'offset one' to 'offset zero' and bound to the screen size using:
	v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1);
This has the side effect of converting zero to the limit.

A check I'm adding to min_t() reports that (u16)(v->xs - 1) (etc)
discards signiticant bits (because v->xs is promoted to 'int' before
the addition).
If v->xs is zero (it comes from userspace) it converts -1 to 0xffff.
This is then bounded to 'vc->vc_cols - 1' which will be fine.

Replace with:
	v->xs = umin(v->xs - 1, vc->vc_cols - 1);
which again converts a -1 to unsigned - this time to 0xffffffff,
with the same overall effect.

Whether zero is meant to mean the 'maximum size' is unknown.
I can't find any documentation for the ioctl and it pre-dates git.

Detected by an extra check added to min_t().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 drivers/tty/vt/selection.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 07d3b93975d3..13f4e48b4142 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -348,10 +348,11 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v,
 		return 0;
 	}
 
-	v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1);
-	v->ys = min_t(u16, v->ys - 1, vc->vc_rows - 1);
-	v->xe = min_t(u16, v->xe - 1, vc->vc_cols - 1);
-	v->ye = min_t(u16, v->ye - 1, vc->vc_rows - 1);
+	/* Historically 0 => max value */
+	v->xs = umin(v->xs - 1, vc->vc_cols - 1);
+	v->ys = umin(v->ys - 1, vc->vc_rows - 1);
+	v->xe = umin(v->xe - 1, vc->vc_cols - 1);
+	v->ye = umin(v->ye - 1, vc->vc_rows - 1);
 
 	if (mouse_reporting() && (v->sel_mode & TIOCL_SELMOUSEREPORT)) {
 		mouse_report(tty, v->sel_mode & TIOCL_SELBUTTONMASK, v->xs,
-- 
2.39.5

On 19. 11. 25, 23:41, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> The row/column bounds (for a screen window box) are changed from
> 'offset one' to 'offset zero' and bound to the screen size using:
> 	v->xs = min_t(u16, v->xs - 1, vc->vc_cols - 1);
> This has the side effect of converting zero to the limit.
> 
> A check I'm adding to min_t() reports that (u16)(v->xs - 1) (etc)
> discards signiticant bits (because v->xs is promoted to 'int' before
> the addition).
> If v->xs is zero (it comes from userspace) it converts -1 to 0xffff.
> This is then bounded to 'vc->vc_cols - 1' which will be fine.
> 
> Replace with:
> 	v->xs = umin(v->xs - 1, vc->vc_cols - 1);
> which again converts a -1 to unsigned - this time to 0xffffffff,
> with the same overall effect.

LGTM:

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

> Whether zero is meant to mean the 'maximum size' is unknown.
> I can't find any documentation for the ioctl and it pre-dates git.

Behavior of zero is unspecified AFAICT (the impl can do whatever is easy 
:)).

thanks,
-- 
js
suse labs