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] [day] [month] [year] [list]

Message-ID: <20250325041411-mutt-send-email-mst@kernel.org>
Date: Tue, 25 Mar 2025 04:14:35 -0400
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Daniel Verkamp <dverkamp@...omium.org>
Cc: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.com>,
	linux-kernel@...r.kernel.org, virtualization@...ts.linux.dev,
	pasic@...ux.ibm.com, amit@...nel.org, schnelle@...ux.ibm.com,
	Kusanagi Kouichi <slash@...auone-net.jp>
Subject: Re: [PATCH] virtio_console: fix order of fields cols and rows

On Mon, Mar 24, 2025 at 12:53:29PM -0700, Daniel Verkamp wrote:
> 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

I agree. Cc author of the qemu patch for confirmation.

-- 
MST