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: <20250319172004-mutt-send-email-mst@kernel.org>
Date: Wed, 19 Mar 2025 17:21:06 -0400
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Halil Pasic <pasic@...ux.ibm.com>
Cc: Maximilian Immanuel Brandtner <maxbr@...ux.ibm.com>,
	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, Mar 19, 2025 at 06:13:08PM +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

Agreed, still, please do a bit of research on open source hypervisors
at least (rust-vmm?) and include the info which ones you checked.