linux-kernel - Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console size and cursor position

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3o3q5896-8540-nro6-534o-307nn81r7r5r@syhkavp.arg>
Date: Thu, 15 May 2025 12:02:26 -0400 (EDT)
From: Nicolas Pitre <nico@...xnic.net>
To: Jiri Slaby <jirislaby@...nel.org>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] vt: add VT_GETCONSIZECSRPOS to retrieve console
 size and cursor position

On Thu, 15 May 2025, Jiri Slaby wrote:

> On 14. 05. 25, 21:42, Nicolas Pitre wrote:
> > From: Nicolas Pitre <npitre@...libre.com>
> > 
> > The console dimension and cursor position are available through the
> > /dev/vcsa interface already. However the /dev/vcsa header format uses
> > single-byte fields therefore those values are clamped to 255.
> > 
> > As surprizing as this may seem, some people do use 240-column 67-row
> > screens (a 1920x1080 monitor with 8x16 pixel fonts) which is getting
> > close to the limit. Monitors with higher resolution are not uncommon
> > these days (3840x2160 producing a 480x135 character display) and it is
> > just a matter of time before someone with, say, a braille display using
> > the Linux VT console and BRLTTY on such a screen reports a bug about
> > missing and oddly misaligned screen content.
> > 
> > Let's add VT_GETCONSIZECSRPOS for the retrieval of console size and cursor
> > position without byte-sized limitations. The actual console size limit as
> > encoded in vt.c is 32767x32767 so using a short here is appropriate. Then
> > this can be used to get the cursor position when /dev/vcsa reports 255.
> > 
> > The screen dimension may already be obtained using TIOCGWINSZ and adding
> > the same information to VT_GETCONSIZECSRPOS might be redundant. However
> > applications that care about cursor position also care about display
> > size and having 2 separate system calls to obtain them separately is
> > wasteful. Also, the cursor position can be queried by writing "\e[6n" to
> > a tty and reading back the result but that may be done only by the actual
> > application using that tty and not a sideline observer.
> > 
> > Signed-off-by: Nicolas Pitre <npitre@...libre.com>
> > ---
> >   drivers/tty/vt/vt_ioctl.c | 16 ++++++++++++++++
> >   include/uapi/linux/vt.h   |  9 +++++++++
> >   2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
> > index 4b91072f3a4e..83a3d49535e5 100644
> > --- a/drivers/tty/vt/vt_ioctl.c
> > +++ b/drivers/tty/vt/vt_ioctl.c
> > @@ -951,6 +951,22 @@ int vt_ioctl(struct tty_struct *tty,
> >    				(unsigned short __user *)arg);
> >    case VT_WAITEVENT:
> >   		return vt_event_wait_ioctl((struct vt_event __user *)arg);
> > +
> > +	case VT_GETCONSIZECSRPOS:
> > +	{
> > +		struct vt_consizecsrpos concsr;
> > +
> > +		console_lock();
> > +		concsr.con_cols = vc->vc_cols;
> > +		concsr.con_rows = vc->vc_rows;
> > +		concsr.csr_col = vc->state.x;
> > +		concsr.csr_row = vc->state.y;
> > +		console_unlock();
> 
> Makes a lot of sense!
> 
> > +		if (copy_to_user(up, &concsr, sizeof(concsr)))
> > +			return -EFAULT;
> > +		return 0;
> > +	}
> > +
> >    default:
> >    	return -ENOIOCTLCMD;
> >   	}
> > diff --git a/include/uapi/linux/vt.h b/include/uapi/linux/vt.h
> > index e9d39c48520a..e93c8910133b 100644
> > --- a/include/uapi/linux/vt.h
> > +++ b/include/uapi/linux/vt.h
> > @@ -84,4 +84,13 @@ struct vt_setactivate {
> >   
> >   #define VT_SETACTIVATE	0x560F	/* Activate and set the mode of a
> >   console */
> >   
> > +struct vt_consizecsrpos {
> > +	unsigned short con_rows;	/* number of console rows */
> > +	unsigned short con_cols;	/* number of console columns */
> > +	unsigned short csr_row;		/* current cursor's row */
> > +	unsigned short csr_col;		/* current cursor's column */
> 
> Use __u16 pls.

I beg to differ. Not because __u16 is fundamentally wrong. But 
everything else in this file uses only basic C types already and adding 
one struct with __u16 would look odd. And adding some include to define 
that type would be needed since there are currently no such includes in 
that file currently, and that could potentially cause issues with 
existing consumers of that header file that didn't expect extra 
definitions, etc. So I think that such a change, if it is to happen, 
should be done for the whole file at once and in a separate patch.

> > +};
> > +
> > +#define VT_GETCONSIZECSRPOS 0x5610  /* get console size and cursor position
> > */
> 
> Can we define that properly as
>   _IOR(0x56, 0x10, struct vt_consizecsrpos)
> ? Note this would still differ from "conflicting":
> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)

Similarly as the reason above: given that no other definitions in that 
file use the _IO*() scheme for historical reasons, it is preferable to 
follow what's already there to avoid unsuspected confusion. The VT layer 
is pretty much unlykely to grow many additional ioctls in the 
foreseeable future so I'd lean towards keeping things simple and in line 
with the existing code.


Nicolas