[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#988776: Bug#983357: Netinst crashes xen domU when loading kernel



On 8/24/2021 7:12 PM, Ben Hutchings wrote:
On Tue, Aug 24, 2021 at 03:27:19PM -0400, Phillip Susi wrote:
Ben Hutchings <ben@decadent.org.uk> writes:

I think a proper fix would be one of:

a. If the Xen virtual keyboard driver is advertising capabilities it
    doesn't have, stop it doing that.
b. Change the implementation of modalias attributes to allow longer
    values.

It's not clear to me whether the Xen driver is advertising correctly or
not.  If it is, then�the solution should be b, but that may be too
disruptive a change to the kernel.  So a reasonable workaround might
be:

c. Change the input subsystem to limit the length of the
    capabilities part of the modalias.
The problem with a) is that the Xen keyboard is not a physical keyboard
and so it has no way of knowing what keys it actually has.  It is a fake
input device designed to pass through whatever input the Xen hypervisor
sends down.  As such, any key could come in.  If it doesn't advertise
that it has all of these keys, then they would not be accepted by
libinput when the hypervisor sends them down.
Right, that's what I feared.

xen-kbdfront is setting the bits for keys in the ranges [KEY_ESC,
KEY_UNKNOWN) and [KEY_OK, KEY_MAX), which I think works out to 654
keys and 2362 bytes in the modalias.

This seems to be the heart of the problem: libinput was designed
assuming that all keyboards can and must report what keys are actually
present, and then libinput tries to cram that information into the
modalias rather than some other sysfs attribute as it should ( or not at
all... I still don't see how this information is actually supposed to be
useful to userspace ).
I think modaliases aren't intended to be interpreted by user-space,
other than processing wildcards when matching to modules.

For input devices, the same information is available through other
variables in the uevent, in a more compact form.  The information *is*
useful for user-space; e.g. in initramfs-tools we recognise keyboard
devices and add their drivers to the initramfs but ignore other input
devices.

As for b), the problem isn't with the modalias attribute itself, but
when the kernel tries to copy it into the environment block for the udev
callout.  The environment block is only a single page, and so limited to
4 KB.  And that's for everything else that goes into the environment,
not just the modalias.
Text-based sysfs attributes are limited to a page, but udev receives
uevents through netlink, not sysfs.

The current limit on the environment of a uevent appears to be 2 KB
(UEVENT_BUFFER_SIZE defined in <linux/kobject.h>).  That seems like it
*might* be easier to change, so long as user-space doesn't have a
similar limit.

I looked into systemd/udev, and it seems to use an 8 KB buffer for
receiving uevents:

https://sources.debian.org/src/systemd/247.9-1/src/libsystemd/sd-device/device-monitor.c/?hl=390#L390

But as a first step I think increasing the kernel buffer size to 4 KB
would be enough.  Perhaps someone could test whether this patch to the
domU kernel makes udev happier:

--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -30,7 +30,7 @@
#define UEVENT_HELPER_PATH_LEN 256
  #define UEVENT_NUM_ENVP			64	/* number of env pointers */
-#define UEVENT_BUFFER_SIZE		2048	/* buffer for the variables */
+#define UEVENT_BUFFER_SIZE		4096	/* buffer for the variables */
#ifdef CONFIG_UEVENT_HELPER
  /* path to the userspace helper executed on an event */
--- END ---

?

Ben.



I will try it in my bullseye Xen HVM DomU.

I am not sure how to rebuild the installation media with a patched
systemd, but I can patch my installed Xen HVM DomU system
with a patched systemd with the increased buffer size and see if the
Coldplug failure early in the boot process goes away. If so, then it
is likely this patch to systemd would also fix the installation media.

If it doesn't work, I am also willing to try approach a by patching
the Linux kernel xen-kbdfront driver by removing the for loops that
advertise those 654 keys. I tend to agree with Philip that this is
totally unnecessary, but I suppose I could be wrong about that.
I read the discussion Philip had with the Xen developers and they
seemed to want to keep the Xen keyboard driver as it is.

Chuck


Reply to: