ACK: [SRU][I][H][F][PATCH 0/1] kernel: unable to read partitions on virtio-block dasd (kvm) (LP: 1950144)

[Thadeu Lima de Souza Cascardo]

On Tue, Nov 09, 2021 at 01:47:01PM +0100, frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/1950144
> 
> SRU Justification:
> 
> [Impact]
> 
> * The kernel is unable to read partitions on virtio-block DASD (on KVM).
>   That's a severe situation, since it prevents Ubuntu from starting, if installed on a DASD.
>   This issue can either occur after a fresh installation or after an upgrade.
> 
> * The virtio specification virtio-v1.1-cs01 states: "Transitional devices
>   MUST detect Legacy drivers by detecting that VIRTIO_F_VERSION_1 has not
>   been acknowledged by the driver."
>   And this is what QEMU as of 6.1 has done relying solely on
>   VIRTIO_F_VERSION_1 for detecting that.
> 
> * But the specification also says: "... the driver MAY read (but MUST
>   NOT write) the device-specific configuration fields to check that it can
>   support the device ..." before setting FEATURES_OK.
>     
> * In this case, any transitional device relying solely on VIRTIO_F_VERSION_1
>   for detecting legacy drivers will return data in legacy format.
>   In particular, this implies that it's in big endian format for
>   big endian guests. This naturally confuses the driver that expects
>   little endian in the modern mode.
> 
> * VIRTIO_F_VERSION_1 can only be relied on after the feature negotiation is done.
> 
> * 'verify' is called before virtio_finalize_features(), so a transitional
>   s390 virtio device still serves native endian (i.e. big endian) config space,
>   while the driver knows that it is going to accept VERSION_1,
>   so when reading the config space, it assumes it got little endian, and byteswaps.
> 
> * For QEMU, we can work around the issue by writing out the feature bits with
>   VIRTIO_F_VERSION_1 bit set. We (ab)use the finalize_features config op for
>   this. This isn't enough to address all vhost devices since these do not get
>   the features until FEATURES_OK, however it looks like the affected devices
>   actually never handled the endianness for legacy mode correctly, so at least
>   that's not a regression.
> 
> [Fix]
> 
> * 2f9a174f918e29608564c7a4e8329893ab604fb4 2f9a174f918e "virtio: write back F_VERSION_1 before validate"
> 
> [Test Case]
> 
> * Setup an IBM Z or LinuxONE LPAR with Ubuntu Server 20.04 as KVM host.
> 
> * This Ubuntu KVM host can either be installed on FCP or DASD storage,
>   but at least one DASD disk need to be reserved for a KVM guest.
> 
> * Now hand over the reserved DASD disk   (low-level formatted using dasdfmt
>   and partitioned using fdasd) using 'virtio-block' to a KVM virtual machine
>   (e.g. using a virsh VM config).
> 
> * Try to install an Ubuntu KVM virtual machine using this DASD disk,
>   that includes the check and read of the partition table.
> 
> [Where problems could occur]
> 
> * First of all requested commit contains one additional if statement;
>   and is due tothat relatively traceable.
> 
> * But the change is in /drivers/virtio/virtio.c, means in common code!
> 
> * This issue obviously affects big endian systems only.
> 
> * But if done wrong, it may effect in worst-case little endian systems, too!
> 
> * But the if statement explicitly checks for '!virtio_legacy_is_little_endian()'.
> 

Yeah, given this restriction, I am less concerned with regressions on other platforms.

Thanks.
Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>

> * Only virtio net and virtio blk devices seem to be affected.
> 
> * And the commit/solutions was in-depth discussed upstream here:
>   https://lore.kernel.org/all/20211011053921.1198936-1-pasic@linux.ibm.com/t/#u
> 
> [Other]
> 
> * Patches are upstream accepted with since 5.15-rc6
>   and tagged for upstream stable #v4.11.
>   Hence jammy is not affected.
> 
> * Request was to add the patches to focal / 20.04,
>   but to avoid potential regressions on upgrades,
>   the patches need to be added to impish and hirsute, too.
> 
> * Fortunately cherry-picking the commit works cleanly
>   from all the affected Ubuntu releases.
> 
> Halil Pasic (1):
>   virtio: write back F_VERSION_1 before validate
> 
>  drivers/virtio/virtio.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team