linux-kernel - Re: [PATCH] Revert "virtio_console: fix order of fields cols and rows"

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <32bc2a49fabd619ea7dcafd7f5d50fca206b38ac.camel@linux.ibm.com>
Date: Mon, 15 Sep 2025 18:44:50 +0200
From: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.com>
To: "Michael S. Tsirkin" <mst@...hat.com>, linux-kernel@...r.kernel.org
Cc: Filip Hejsek <filip.hejsek@...il.com>, Amit Shah <amit@...nel.org>,
        Arnd
 Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        virtualization@...ts.linux.dev
Subject: Re: [PATCH] Revert "virtio_console: fix order of fields cols and
 rows"

On Fri, 2025-09-12 at 04:40 -0400, Michael S. Tsirkin wrote:
> This reverts commit 5326ab737a47278dbd16ed3ee7380b26c7056ddd.
> 
> The problem is that for a long time, the
> Linux kernel used a different field order from what was specified in
> the
> virtio spec. The kernel implementation was apparently merged around
> 2010,
> while the virtio spec came in 2014, so when a previous version of
> this
> patch series was being discussed here on this mailing list in 2020,
> it
> was decided that QEMU should match the Linux implementation, and
> ideally,
> the virtio spec should be changed.
> 
> There are about 15 years' worth
> of kernel versions with the swapped field order, including the kernel
> currently shipped in Debian stable. The effects of the swapped
> dimensions
> can sometimes be quite annoying - e.g. if you have a terminal with
> 24 rows, this will be interpreted as 24 columns, and your shell may
> limit
> line editing to this small space, most of which will be taken by your
> prompt.
> 
> NB: the command structures really should move to the UAPI header so
> it
> is easier to notice when a change is breaking the guest/host ABI.

As I already mentioned in the QEMU discussion, I proposed the fix,
because I was working on a similar implementation to bring resizing to
QEMU. Unfortunately, the patch-set was stuck in limbo for a while and
now that someone else has picked up the slack, I've descided that it's
better to contribute to the patch-set that is upstream instead of
sending a competing patch-set that does the same thing. Accordingly, I
no longer have any skin in the game of implementing resizing for virtio
console in QEMU as the other patch-set takes care of that task.

On a related note, during the initial discussion of this changing the
virtio spec was proposed as well (as can be read from the commit mgs),
however at the time on the viritio mailing list people were resistent
to the idea of changing the virtio spec to conform to the kernel
implementation.
I don't really care if this discrepancy is fixed one way or the other,
but it should most definitely be fixed.

Kind regards,
Max Brandtner

> 
> Reported-by: Filip Hejsek <filip.hejsek@...il.com>
> Fixes: 5326ab737a47 ("virtio_console: fix order of fields cols and
> rows")
> Cc: "Maximilian Immanuel Brandtner" <maxbr@...ux.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> ---
>  drivers/char/virtio_console.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/virtio_console.c
> b/drivers/char/virtio_console.c
> index 088182e54deb..216c5115637d 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1576,8 +1576,8 @@ static void handle_control_message(struct
> virtio_device *vdev,
>  		break;
>  	case VIRTIO_CONSOLE_RESIZE: {
>  		struct {
> -			__virtio16 cols;
>  			__virtio16 rows;
> +			__virtio16 cols;
>  		} size;
>  
>  		if (!is_console_port(port))