linux-kernel - Re: [PATCH] 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: <CABVzXAn1iNHP_8h-sj1mTJDuu9dxOBhwi+nbhhr9d27NTo-6wg@mail.gmail.com>
Date: Mon, 24 Mar 2025 12:53:29 -0700
From: Daniel Verkamp <dverkamp@...omium.org>
To: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, virtualization@...ts.linux.dev, 
	mst@...hat.com, pasic@...ux.ibm.com, amit@...nel.org, schnelle@...ux.ibm.com
Subject: Re: [PATCH] virtio_console: fix order of fields cols and rows

On Mon, Mar 24, 2025 at 7:43 AM Maximilian Immanuel Brandtner
<maxbr@...ux.ibm.com> wrote:
>
> According to section 5.3.6.2 (Multiport Device Operation) of the virtio
> spec(version 1.2) a control buffer with the event VIRTIO_CONSOLE_RESIZE
> is followed by a virtio_console_resize struct containing cols then rows.
> The kernel implements this the wrong way around (rows then cols) resulting
> in the two values being swapped.
>
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.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 21de774996ad..38af3029da39 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1579,8 +1579,8 @@ static void handle_control_message(struct virtio_device *vdev,
>                 break;
>         case VIRTIO_CONSOLE_RESIZE: {
>                 struct {
> -                       __virtio16 rows;
>                         __virtio16 cols;
> +                       __virtio16 rows;
>                 } size;

The order of the fields after the patch matches the spec, so from that
perspective, looks fine:
Reviewed-by: Daniel Verkamp <dverkamp@...omium.org>

Since the driver code has been using the wrong order since support for
this message was added in 2010, but there is no support for sending
this message in the current qemu device implementation, I wondered
what device code was used to test this when it was originally added. I
dug up what I assume is the corresponding qemu device change from the
same era, which sends the VIRTIO_CONSOLE_RESIZE message using the
rows, cols order that matches the kernel driver (and differs from the
spec):

https://lore.kernel.org/qemu-devel/1273092505-22783-1-git-send-email-amit.shah@redhat.com/
("[Qemu-devel] [PATCH] virtio-serial: Send per-console port resize
notifications to guest", May 6, 2010)

However, I don't believe that patch ever made it into an actual qemu
release, so it's probably not a compatibility concern. (If there are
any other device implementations that use the kernel driver order
rather than the spec order, then maybe this would need more
consideration, but I don't personally know of any.)

Thanks,
-- Daniel