From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <010701a110dc8b19-4408e202-87b3-4574-adfc-00c50bc066b8-000000@eu-central-1.amazonses.com> Received: from DUZPR83CU001.outbound.protection.outlook.com (mail-northeuropeazon11022073.outbound.protection.outlook.com [52.101.66.73]) by lore.virtuozzo.com (Postfix) with ESMTPS id 871F5802A7 for ; Tue, 6 Oct 2026 10:57:35 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=ElPS3oBUijbrrX0UDDieVE5/Q+Q2kfm7EW92fUfkyraxJ+IE/GNdTwJrlCdhcv3XcfHshMCst4cwZbC+fMoTdMNDPSm0ST7ubA6lquS6tymLUm4qHsbIKDzYAk7Ise7U9Gw9Axjcy88Ja238eFUoXgyQKGTCxF18wWq/2UnuIrktNFZ4jqkTKzq3K+d0u5yljNOC+HXksHw3AIaI8pHnzZCrHKOQikOdB+/Iw8V+IapsRYTgp/OZ4xvrMYY5KPoSb9awDmARKfxt6a1WJj3bgi5i/Ztbly54IvdFB7QNxgreFcRU3PXQbg2xQ9mgbXHD+aKWozMiMcvyFsD0PFRz8Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=hqLLlTgtloUo5K26q9scsMIWtsf+/9No4cOrlW+AO3E=; b=AIpNoV9eizHiDedAMGJUSar4uDv69sCwriqsRJ1FbYWLLkOjYtfZy3SnrLwLSO2v6r1kfKhUeM0G++xVssRIPqtaMOAP3QoFlAalKmNWwY63vcRf9MYorXHOu2slOi/uB13VV6HF4PUIRfc75Si4i8MmLcLmBrtF71CvbdF3C5ecW0VHSrz+dmzrqO2bhpe/vSjxNPSabeGFs+UQVsf+RelGs46ZJvu/Im/1l6BkD7YTEKB2KSxj5toJvaM4006ddiqQS94NWnmABO0BWU/xxyAUBcDETphOsuG2DVXqyuY8Q30EiZ/6JHkSHb/AmvRFAQx7U68RYczZRgSZgnXjwQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 69.169.224.6) smtp.rcpttodomain=virtuozzo.com smtp.mailfrom=eu-central-1.amazonses.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=sashiko.virtuozzo.com; dkim=pass (signature was verified) header.d=sashiko.virtuozzo.com; dkim=pass (signature was verified) header.d=amazonses.com; arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=virtuozzo.onmicrosoft.com; s=selector2-virtuozzo-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hqLLlTgtloUo5K26q9scsMIWtsf+/9No4cOrlW+AO3E=; b=BxZjZg/WKrn/F7e/giI1ne0CzpyAHOAxvvimh94OernzX/5ewL5VsumXaMYV+4UFxrcnvgACcq8ECWHF+XLZvMehHotQU2Hypj+Ja2cy5PlsAO0uD33gow8uZcgvknnk5HnP9JozlrVz5X12pyBB4HiT1iNSgMj9KJ30k897SYw= Received: from PA7P264CA0080.FRAP264.PROD.OUTLOOK.COM (2603:10a6:102:349::10) by AM0PR08MB5313.eurprd08.prod.outlook.com (2603:10a6:208:17f::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.496.15; Tue, 6 Oct 2026 10:57:32 +0000 Received: from MI3PEPF00008558.eurprd05.prod.outlook.com (2603:10a6:102:349:cafe::62) by PA7P264CA0080.outlook.office365.com (2603:10a6:102:349::10) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.496.5 via Frontend Transport; Tue, 6 Oct 2026 10:57:32 +0000 Authentication-Results: mx.microsoft.com 1; spf=pass (sender IP is 69.169.224.6) smtp.mailfrom=eu-central-1.amazonses.com; dkim=pass (signature was verified) header.d=sashiko.virtuozzo.com;dkim=pass (signature was verified) header.d=amazonses.com;dmarc=pass action=none header.from=sashiko.virtuozzo.com; Received-SPF: Pass (protection.outlook.com: domain of eu-central-1.amazonses.com designates 69.169.224.6 as permitted sender) receiver=protection.outlook.com; client-ip=69.169.224.6; helo=b224-6.smtp-out.eu-central-1.amazonses.com; pr=C Received: from b224-6.smtp-out.eu-central-1.amazonses.com (69.169.224.6) by MI3PEPF00008558.mail.protection.outlook.com (10.167.240.20) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.496.14 via Frontend Transport; Tue, 6 Oct 2026 10:57:31 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1791284251; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID; bh=gQkEGwBrGY15YKFhZ9z/3E7K051snjtlMjwy2sMkweo=; b=wTZJxcXZuOjlpCx2eHCAbA6/yFerQ4lVUmvUOeoaX5+TLrlTvKHt/oV9ijBanuBp bHjXaW3+/ziPWbTedSsLAcb7VjafbWCMQxpKjqM8C4oEeXHfqR9efCwkFWxF2wU0pBD jwkenKRZQd/pimppRtGNuk0IiCotVto00Kf3qbFKj8VzNeK54GPIFhU+V2Ubd9cHn+Y 1c2Kq888nUeLTLYoN/lmYPJL066iMX/s/2+1X28hWMc5rCEjAJneScXisjBDG/07iIU XLXwzPBqOvY+KgP/lD2kW3uO1hz3P42YxYLzn9vUL/ERC0dghrTQ+gm+TooQ58pyB1T WYIJFVQKAw== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=sokbgaaqhfgd6qjht2wmdajpuuanpimv; d=amazonses.com; t=1791284251; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID:Feedback-ID; bh=gQkEGwBrGY15YKFhZ9z/3E7K051snjtlMjwy2sMkweo=; b=NzRYu/HfARjOgdxuU5O03lRA3e4x4wTCe/Mq/48SaGbu+pJsbxMqjy1uu3on1Rm6 7BAeCyVtJvaHm++7vITJCXSwdxODy39odF+eG998/VVlTjcXGMSM93+lMJeXo8Ax+C3 mcc98qBhDC/6ItQMnw4EmjjE/HkAt7s0+/92KuRo= From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.0 v6 10/12] usb-host: reconfigure endpoints on CPR incoming #VSTOR-137800 To: Andrey Drobyshev , svt-core@virtuozzo.com Cc: andrey.drobyshev@virtuozzo.com, den@openvz.org In-Reply-To: <20261006-usb-host_v3-v6-10-8688ec9576d1@virtuozzo.com> References: <20261006-usb-host_v3-v6-0-8688ec9576d1@virtuozzo.com> <20261006-usb-host_v3-v6-10-8688ec9576d1@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 6 Oct 2026 10:57:31 +0000 Message-ID: <010701a110dc8b19-4408e202-87b3-4574-adfc-00c50bc066b8-000000@eu-central-1.amazonses.com> Feedback-ID: ::1.eu-central-1.2wRQJBpg9g1fGRYexx3dUuYGS25Bux+2TEOx72/m6U4=:AmazonSES X-SES-Outgoing: 2026.10.06-69.169.224.6 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MI3PEPF00008558:EE_|AM0PR08MB5313:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: dbcdfc98-dbc5-4ded-b002-08df23989ef6 X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr X-MS-Exchange-AtpMessageProperties: SA List-Id: svt-core@virtuozzo.com X-MS-Exchange-Transport-Rules-Loop: 1 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|704161411799003|32142699015|61400799027|9140799003|48200799018|376014|10067099003|5063699009|56012099006|4143699003|5023799004|6133799003|5113699003|4113699003|55112099003|22082099003|16102099003|18002099003; X-Microsoft-Antispam-Message-Info: SU7iP76leh9Sq/DJsoapxtdqwPCH+9MnZuubMpQP++fJaM1tSiUmKOlLY8wUQ6IZr1tbMaRydK+QTIq2MgmqRbin0qZHJL2dQU+T9of39UFaz3J8bxC0lwbOIUKL1f3LFNPryWYCruPkAyIwWEXq63UeSE2Y9qW+ptoRe1kZWq4vjmhgS+wspYsqz6hdTrvxdD5i3FovFyZzguFweCgN9G4O1fXUpSSqOZddZfGfbItOKkZ/i6YLDvACd8QduY9JIEngz9G0iDJeBHPTBLmJTqDv8ngmaBJJ57noJaLWc9CkgeminGf2uZ/JUvv8235mzz6tB20sO2eYG/I3aTdUh/ajzA2N+8vzqhtbykj+oHblClqO0Dnr2Jt8aq4W6GW0WfdXjbJFw5EmiJlJmGUKjmongDKRH710MxeZKpSP3feJfjOv08Bx2bkYz43qy9F2VXQaS4Z5krbWJmh1bvM5c7JTdWz6sFJbUwCnS4G8oi+Ytxhs60cKXfZh/a9lwKA2PaD3ii6j96KeqAfqPsQ/CmdYLCiZE7tnnsHF6NWCyYSD2zly5NUkp0WA9tOcv85rKBpI2CLMf8FGZ1tvlbcXJgkt2Fk7Xpk50CxNkDYDFOS63VivO+ZjcSnMJUuU75fMLEm4c0xJNJ+Vq9o4W9+FZjJbJ8SIBapzXtEZ5DSSf733v9A0khF8+ms4OrdxS4HfP9r3NtV3G6fKqc1r64RtGqgfZPv2b5QRbYyRN8ezBR8isPupikCFAMzhfO6ukIgHeiZXRcXb/dfSQxiTV7h5kQ== X-Forefront-Antispam-Report: CIP:69.169.224.6;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:b224-6.smtp-out.eu-central-1.amazonses.com;PTR:b224-6.smtp-out.eu-central-1.amazonses.com;CAT:NONE;SFS:(13230040)(704161411799003)(32142699015)(61400799027)(9140799003)(48200799018)(376014)(10067099003)(5063699009)(56012099006)(4143699003)(5023799004)(6133799003)(5113699003)(4113699003)(55112099003)(22082099003)(16102099003)(18002099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: b+tGauSjn6MkrF1qpD9k0CXfgdZ5M6UTvjVnbId0G3D5iOizRodz9R+za8jZGqZ3tStwOvT4ky91byGsnmxt8LlZAT2iVAwLMdJFmMstAr5NzM+op/mcU0ljQtUxZPstDZx7guPoMywOp0c+FXb+zd7AU505guJmRdHlz9lAMUslHLde15358keNusyM6nc7GZ6CeLqER/JjhvSSN913GJ+F5e4cKpUFefkeDpn/sFDqLlwCiWQkE4xuNTKjy688k49j5QnGOXiWALyvRLmfj/la4ojFwn5IolR3W0aPPwx9f2fyz3WQ5trltHvdmqAYcu66HLab4bF4OaSuYAXaLzTenvirIQoEgrI2z2qLrEV0c7yMPK0NsGAvl670Wrg72Q7sMdfIIPWXSnTZi/wzvdg8d2OV/92SCcDHnrYMGHiDxZD/eGGautV7IwxIw1zH X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2026 10:57:31.8162 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: dbcdfc98-dbc5-4ded-b002-08df23989ef6 X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: MI3PEPF00008558.eurprd05.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB5313 Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: - [Medium] usb-host: CPR post_load fails on an unconfigured device (NOT_FOU= ND treated as fatal) - [Medium] usb-host: re-claim loop uses array index instead of bInterfaceNu= mber - [Medium] usb-host: device-supplied bInterfaceNumber indexes altsetting[16= ] unchecked; ninterfaces unclamped - [Low] usb-host: usb_host_ep_update() result ignored on CPR path, so parti= al rebuild reports success -- commit 7fc1fbcbe45c4e899a6393d8acc4e2e9151dac29 Author: Andrey Drobyshev usb-host: reconfigure endpoints on CPR incoming #VSTOR-137800 This commit makes the CPR incoming path in usb_host_post_load() re-claim the interfaces through libusb and call usb_host_ep_update(). The goal is to rebuild the endpoints for a non-zero altsetting restored by vmload. It also adds an "alt >=3D 0" check to an assert in usb_host_ep_update(). > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c [ ... ] > +static int usb_host_cpr_reclaim_interfaces(USBHostDevice *s) > +{ > + USBDevice *udev =3D USB_DEVICE(s); > + struct libusb_config_descriptor *conf; > + int n, rc; > + > + rc =3D libusb_get_active_config_descriptor(s->dev, &conf); > + if (rc !=3D 0) { > + usb_host_libusb_error("libusb_get_active_config_descriptor", rc); > + return -1; > + } [Severity: Medium] Can this abort the whole incoming CPR vmload for a device that has no active configuration? For a device in the address state (for example after SET_CONFIGURATION(0) or a device reset), libusb_get_active_config_descriptor() returns LIBUSB_ERROR_NOT_FOUND. Here any rc !=3D 0 returns -1, and usb_host_post_load() propagates it: if (usb_host_cpr_reclaim_interfaces(dev) < 0) { return -1; } Before this patch the CPR branch always returned 0. The existing usb_host_claim_interfaces() treats the same condition as benign: if (rc =3D=3D LIBUSB_ERROR_NOT_FOUND) { /* address state - ignore */ return USB_RET_SUCCESS; } and usb_host_ep_update() simply returns. Should NOT_FOUND be tolerated here as well? Also, if the host still has an active configuration but the guest never configured the device, does this function claim every interface and then overwrite the guest-visible state? udev->ninterfaces =3D conf->bNumInterfaces; udev->configuration =3D conf->bConfigurationValue; The source had both of these at 0. The later patch "usb-host: make CPR migration blocker conditional" only changes the blocker and does not touch this helper. > + > + for (int i =3D 0; i < conf->bNumInterfaces; i++) { > + int ifnum =3D conf->interface[i].altsetting[0].bInterfaceNumber; > + int alt =3D udev->altsetting[ifnum]; [Severity: Medium] Can this read past the end of udev->altsetting[]? ifnum comes from the device's bInterfaceNumber (a uint8_t, up to 255), while altsetting[] has USB_MAX_INTERFACES (16) entries. This validation loop runs over all conf->bNumInterfaces entries with no bound on ifnum. The claim loop below is clamped with MIN(conf->bNumInterfaces, USB_MAX_INTERFACES), but the function then stores the unclamped value and returns success: udev->ninterfaces =3D conf->bNumInterfaces; Can a device with more than 16 interfaces then trip this assert in a later usb_desc_set_config()? assert(dev->ninterfaces <=3D USB_MAX_INTERFACES); usb_host_claim_interfaces() would have returned a stall in that case. usb_host_ep_update(), which is now also called from the CPR path, has the same unchecked access: alt =3D udev->altsetting[intf->bInterfaceNumber]; This isn't a bug introduced by this patch, but the new call site makes it reachable on the CPR path. The patch only adds "alt >=3D 0" to the assert there. > + > + if (alt < 0 || alt >=3D conf->interface[i].num_altsetting) { > + error_report("usb-host: migrated altsetting %d out of range = for " > + "interface %d", alt, ifnum); > + libusb_free_config_descriptor(conf); > + return -1; > + } > + } > + > + n =3D MIN(conf->bNumInterfaces, USB_MAX_INTERFACES); > + for (int i =3D 0; i < n; i++) { > + rc =3D libusb_claim_interface(s->dh, i); > + if (rc !=3D 0) { > + usb_host_libusb_error("libusb_claim_interface", rc); > + libusb_free_config_descriptor(conf); > + return -1; > + } > + s->ifs[i].claimed =3D true; > + } [Severity: Medium] Is the loop index i the right value to pass to libusb_claim_interface()? i is the position in conf->interface[], but libusb and usbfs expect the interface number, which is altsetting[0].bInterfaceNumber. The validation loop above already uses bInterfaceNumber for this. The two can differ for devices whose interface numbers don't start at 0 or aren't contiguous, such as {0,2} or {1,2}. Claiming a non-existent number fails, so the function returns -1 and the migration aborts. Where the claim does succeed, the real interfaces are not recorded in libusb's claimed set, so a later SET_INTERFACE can still fail with LIBUSB_ERROR_NOT_FOUND. That is the problem this patch sets out to fi= x. s->ifs[i].claimed is also indexed by position, so usb_host_release_interfac= es() would act on the wrong numbers. By contrast, usb_host_claim_interfaces() probes by interface number and tolerates gaps: for (i =3D 0; i < USB_MAX_INTERFACES; i++) { trace_usb_host_claim_interface(s->bus_num, s->addr, configuration, = i); rc =3D libusb_claim_interface(s->dh, i); if (rc =3D=3D 0) { s->ifs[i].claimed =3D true; Should the new code claim by bInterfaceNumber and index ifs[] the same way? The later patches in the series do not change this. [ ... ] > if (cpr_is_incoming()) { > + /* > + * The device stayed open, so rebuild libusb's claims and endpoi= nts, > + * and start reaping now that the source has handed the fd off. > + */ > + if (usb_host_cpr_reclaim_interfaces(dev) < 0) { > + return -1; > + } > + usb_host_ep_update(dev); > usb_host_cpr_restart_events(); > return 0; > } [Severity: Low] Should the result of usb_host_ep_update() be checked here? usb_host_ep_update() is void. After usb_ep_reset() it returns silently on "invalid endpoint address", "duplicate endpoint address", or a missing active configuration, which leaves a partial endpoint table. usb_host_post_load() then returns 0, so guest transfers to the missing endpoints would stall without any report. usb_host_ep_update() also calls usb_ep_set_halted(udev, pid, ep, 0) for eve= ry endpoint, so any halted state the guest had on the source is lost, since it is not migrated. This second point may be speculative. The failure conditions would normally already have occurred in usb_host_open() on the same device, so this is probably a rare edge case. --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261006-usb-= host_v3-v6-0-8688ec9576d1@virtuozzo.com?part=3D10