linux-kernel - Re: [PATCH] virtio: console: Make resizing compliant with virtio spec

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: <fe72c90a59354f2507c1d528c4e5b8562b20a3e8.camel@linux.ibm.com>
Date: Thu, 20 Mar 2025 08:12:23 +0100
From: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.com>
To: Halil Pasic <pasic@...ux.ibm.com>, "Michael S. Tsirkin" <mst@...hat.com>
Cc: Amit Shah <amit@...nel.org>, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux.dev, arnd@...db.de,
        gregkh@...uxfoundation.org, brueckner@...ux.ibm.com,
        schnelle@...ux.ibm.com
Subject: Re: [PATCH] virtio: console: Make resizing compliant with virtio
 spec

On Wed, 2025-03-19 at 18:13 +0100, Halil Pasic wrote:
> On Wed, 19 Mar 2025 11:00:06 -0400
> "Michael S. Tsirkin" <mst@...hat.com> wrote:
> 
> > > > I was mistaken in my earlier reply - I had missed this
> > > > virtio_console_resize definition in the spec.  So indeed
> > > > there's a
> > > > discrepancy in Linux kernel and the spec's ordering for the
> > > > control
> > > > message.
> > > > 
> > > > OK, that needs fixing someplace.  Perhaps in the kernel (like
> > > > your
> > > > orig. patch), but with an accurate commit message.  
> > > 
> > > So should I send a patch v2 or should the spec be changed
> > > instead? Or
> > > would you like to first await the opinion of the spec
> > > maintainers?
> > > 
> > > The mail I initially sent to the virtio mailing list seems to
> > > have
> > > fallen on deaf ears. I now added Michael Tsirkin to this thread
> > > so that
> > > things might get going.  
> > 
> > 
> > If we can fix the driver to fit the spec, that's best.
> > We generally try to avoid changing the spec just because
> > drivers are buggy.
> 
> I think the call if fixing the driver is possible needs to be made by
> the maintainers of the driver. Fixing the driver IMHO implies that
> if this is seeing any usage in the wild where it properly works a
> fix on the driver side would imply a function regression. But any
> implementers should have complained. So IMHO it is not unreasonable
> to
> assume that this is not seeing any usage in the wild.
> 
> And people would still have the opportunity to catch the regression
> during testing and complain about it.
> 
> I agree with Michael, changing the spec because of a buggy
> implementation should rather be the exception than the rule. And
> AFAIK
> it is not like we have declared something a reference implementation,
> so in that sense the implementation in Linux is just one
> implementation.
> 
> I suppose making it runtime configurable via module parameter is an
> overkill at this point.
> 
> So based no what we know I'm slightly in favor of let us just fix it
> in Linux and see if anybody complains.
> 
> Another thing I noticed during looking at this. AFAICT Linux does not
> seem to handle endiannes here. And AFAIU the message is supposed to
> hold
> le16 that is little endian u16! Maximilian, is this in your opinion
> something we need to fix as well? Or am I just missing the
> conversion?
> 
> Regards,
> Halil

Thanks, I didn't notice that, as I only tested this feature on x86. I
double checked struct virtio_console_config as it also defines cols
before rows, but there the kernel follows the spec (including
endianness).
I'll send a patch v2 shortly.