From: Yaroslav Borbat <yaroslav.752@gmail.com> Inside a container sysfs is usually mounted from the host and exposes all USB devices, while nodes under /dev/bus/usb/ only exist for devices passed into the container's mount namespace. virUSBDeviceSearch() enumerated devices from sysfs alone, so such a device was reported as present: a hostdev with startupPolicy='optional' was not dropped and the domain failed to start. Skip devices whose /dev node is inaccessible during enumeration. virUSBDeviceFind() now ignores them for optional hostdevs and reports an explicit "not accessible in the current mount namespace" error for mandatory ones. Extend the virusb test to cover both cases. Closes: libvirt/libvirt#894 Signed-off-by: Yaroslav Borbat <yaroslav.752@gmail.com> --- src/util/virusb.c | 41 ++++++++++++++++++++++++++++++++++++----- tests/virusbmock.c | 33 ++++++++++++++++++++++++++++++++- tests/virusbtest.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/util/virusb.c b/src/util/virusb.c index 85ad3d58ce..1d31912971 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -120,7 +120,8 @@ virUSBDeviceSearch(unsigned int vendor, unsigned int devno, const char *port, const char *vroot, - unsigned int flags) + unsigned int flags, + char **inaccessiblePath) { g_autoptr(DIR) dir = NULL; bool found = false; @@ -198,6 +199,28 @@ virUSBDeviceSearch(unsigned int vendor, if (!usb) goto cleanup; + /* In containerized environments sysfs may expose USB devices + * from the host kernel while the corresponding device node + * under /dev/bus/usb/ is not present in the current mount + * namespace. Such a device cannot be used, so skip it here. + * Remember its path so that the caller can distinguish "no + * matching device at all" from "device present on the host but + * inaccessible in our mount namespace" when reporting errors. */ + if (virUSBDeviceGetPath(usb) && + !virFileExists(virUSBDeviceGetPath(usb))) { + VIR_DEBUG("USB device %03u:%03u found in sysfs but device " + "node '%s' is not accessible in the current mount " + "namespace, skipping", + found_bus, found_devno, + virUSBDeviceGetPath(usb)); + if (inaccessiblePath && !*inaccessiblePath) + *inaccessiblePath = g_strdup(virUSBDeviceGetPath(usb)); + g_clear_pointer(&usb, virUSBDeviceFree); + if (found) + break; + continue; + } + if (virUSBDeviceListAdd(list, &usb) < 0) goto cleanup; @@ -226,10 +249,11 @@ virUSBDeviceFind(unsigned int vendor, virUSBDeviceList **devices) { g_autoptr(virUSBDeviceList) list = NULL; + g_autofree char *inaccessiblePath = NULL; int count; if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, port, - vroot, flags))) + vroot, flags, &inaccessiblePath))) return -1; count = list->count; @@ -240,9 +264,16 @@ virUSBDeviceFind(unsigned int vendor, return 0; } - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find matching USB device: vid:%1$04x, pid:%2$04x, bus:%3$u, device:%4$u, port:%5$s"), - vendor, product, bus, devno, port ? port : ""); + if (inaccessiblePath) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("USB device vid:%1$04x, pid:%2$04x, bus:%3$u, device:%4$u, port:%5$s is present on the host but its device node '%6$s' is not accessible in the current mount namespace"), + vendor, product, bus, devno, port ? port : "", + inaccessiblePath); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find matching USB device: vid:%1$04x, pid:%2$04x, bus:%3$u, device:%4$u, port:%5$s"), + vendor, product, bus, devno, port ? port : ""); + } return -1; } diff --git a/tests/virusbmock.c b/tests/virusbmock.c index c23bed4528..c67a0ba307 100644 --- a/tests/virusbmock.c +++ b/tests/virusbmock.c @@ -22,18 +22,27 @@ #include <sys/stat.h> #include <dirent.h> #include <dlfcn.h> +#include <errno.h> #include <fcntl.h> +#include <unistd.h> #include "virmock.h" #define USB_SYSFS "/sys/bus/usb" #define FAKE_USB_SYSFS "virusbtestdata/sys_bus_usb" +#define USB_DEVFS "/dev/bus/usb/" + +/* Device node of the test device that is present in the fake sysfs but is + * meant to be inaccessible in the current mount namespace (usb4, i.e. the + * root hub 1d6b:0003 on bus 4). See virusbtest.c. */ +#define INACCESSIBLE_USB_NODE USB_DEVFS "004/001" static int (*real_open)(const char *pathname, int flags, ...); #if WITH___OPEN_2 static int (*real___open_2)(const char *path, int flags); #endif static DIR *(*real_opendir)(const char *name); +static int (*real_access)(const char *path, int mode); static void init_syms(void) { @@ -49,6 +58,7 @@ static void init_syms(void) VIR_MOCK_REAL_INIT(__open_2); #endif VIR_MOCK_REAL_INIT(opendir); + VIR_MOCK_REAL_INIT(access); } static char *get_fake_path(const char *real_path) @@ -58,7 +68,7 @@ static char *get_fake_path(const char *real_path) if ((p = STRSKIP(real_path, USB_SYSFS))) path = g_strdup_printf("%s/%s/%s", abs_srcdir, FAKE_USB_SYSFS, p); - else if (!p) + else path = g_strdup(real_path); return path; @@ -75,6 +85,27 @@ DIR *opendir(const char *name) return real_opendir(path); } +int access(const char *path, int mode) +{ + init_syms(); + + /* virUSBDeviceSearch() checks whether the /dev/bus/usb node of a device + * found in sysfs is accessible. There are no such nodes in the test + * environment, so pretend every USB device node exists, except the one + * device that is meant to be inaccessible in the current mount namespace + * (as happens inside a container). This lets us exercise the code path + * without shipping empty device-node files. */ + if (STRPREFIX(path, USB_DEVFS)) { + if (STREQ(path, INACCESSIBLE_USB_NODE)) { + errno = ENOENT; + return -1; + } + return 0; + } + + return real_access(path, mode); +} + int open(const char *pathname, int flags, ...) { g_autofree char *path = NULL; diff --git a/tests/virusbtest.c b/tests/virusbtest.c index 12ac338df9..58ca769059 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -259,6 +259,40 @@ testUSBList(const void *opaque G_GNUC_UNUSED) } +/* usb4 (bus 4, device 1) is present in the fake sysfs, but the mock makes + * its /dev/bus/usb node inaccessible, i.e. the device is exposed by the host + * kernel but not accessible in our mount namespace (as happens inside a + * container). Such a device must be treated as absent: skipped for + * startupPolicy 'optional' (mandatory == false) and reported as an error + * otherwise. */ +static int +testDeviceFindInaccessible(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virUSBDeviceList) devs = NULL; + int rv; + + rv = virUSBDeviceFind(0x1d6b, 0x0003, 4, 1, NULL, NULL, false, + USB_DEVICE_FIND_BY_DEVICE, &devs); + if (rv != 0 || devs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "optional inaccessible device: expected no match " + "(rv=0 and no device list), got rv=%d", rv); + return -1; + } + + rv = virUSBDeviceFind(0x1d6b, 0x0003, 4, 1, NULL, NULL, true, + USB_DEVICE_FIND_BY_DEVICE, &devs); + if (rv >= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "mandatory inaccessible device: expected failure, " + "but the device was reported as available"); + return -1; + } + + return 0; +} + + static int mymain(void) { @@ -336,6 +370,10 @@ mymain(void) if (virTestRun("USB List test", testUSBList, NULL) < 0) rv = -1; + if (virTestRun("USBDeviceFind inaccessible node", + testDeviceFindInaccessible, NULL) < 0) + rv = -1; + if (rv < 0) return EXIT_FAILURE; return EXIT_SUCCESS; -- 2.54.0