| Message ID | 20211109124702.2369200-1-frank.heimes@canonical.com |
|---|---|
| Headers | show |
| Series | kernel: unable to read partitions on virtio-block dasd (kvm) (LP: 1950144) | expand |
On Tue, Nov 09, 2021 at 01:47:01PM +0100, frank.heimes@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@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@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 09.11.21 13:47, frank.heimes@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()'. > > * 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(+) > Applied to hirsute,impish:linux/master-next. For focal this patch was just applied for v5.4.155 upstream stable. I have added the bug reference for this SRU to it. Thanks. -Stefan
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()'. * 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(+)