From nobody Fri Nov 7 04:17:15 2025 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=nutanix.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1546884184516963.7455416383408; Mon, 7 Jan 2019 10:03:04 -0800 (PST) Received: from localhost ([127.0.0.1]:37063 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ggZEt-0005ZL-Co for importer@patchew.org; Mon, 07 Jan 2019 13:03:03 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49789) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ggZ5F-0004dZ-2i for qemu-devel@nongnu.org; Mon, 07 Jan 2019 12:53:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ggZ49-00008N-2U for qemu-devel@nongnu.org; Mon, 07 Jan 2019 12:51:59 -0500 Received: from mx0a-002c1b01.pphosted.com ([148.163.151.68]:40660) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ggZ47-0008Tf-JJ for qemu-devel@nongnu.org; Mon, 07 Jan 2019 12:51:56 -0500 Received: from pps.filterd (m0127840.ppops.net [127.0.0.1]) by mx0a-002c1b01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x07HjZbc006048; Mon, 7 Jan 2019 09:51:47 -0800 Received: from nam03-by2-obe.outbound.protection.outlook.com (mail-by2nam03lp2057.outbound.protection.outlook.com [104.47.42.57]) by mx0a-002c1b01.pphosted.com with ESMTP id 2pu3gqdd87-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 07 Jan 2019 09:51:47 -0800 Received: from BN6PR02MB2818.namprd02.prod.outlook.com (10.175.96.17) by BN6PR02MB2740.namprd02.prod.outlook.com (10.175.95.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1495.8; Mon, 7 Jan 2019 17:51:40 +0000 Received: from BN6PR02MB2818.namprd02.prod.outlook.com ([fe80::b9ad:bd19:2d6d:1205]) by BN6PR02MB2818.namprd02.prod.outlook.com ([fe80::b9ad:bd19:2d6d:1205%7]) with mapi id 15.20.1495.011; Mon, 7 Jan 2019 17:51:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nutanix.com; h=from : to : cc : subject : date : message-id : content-type : content-transfer-encoding : mime-version; s=proofpoint20171006; bh=jH/WGvH0odr6Clvi34kRG1zhirIrnNuVZ/34lELmiUY=; b=T9N/H916kEAy0KqnfqJohOAiyRaxjFShRbEGMwjqtZ3WaP8fG/hdnTZuktKvnIwrwWuz wzwjBXutBYeJGp1JK8BdC2nS9Xpmi7h+pIcxfHKnUh8FT/Q7765N9Y5WDUvr1Qh04WH7 qrrWo2UnmqiViPswURwa9DkFK4+CiZBCIEevaBwIHaBIkDRdroiCWL+GtKVj7+ZfMe4i tDuRmdYSGS79ubL+506pk/6NHt9j24JLa1hsdDKIYLOBlRjLVqLqKvNKVAc4TfvrHm98 gYvk2ZtcToajFJ7ghtNw3XaGzBfy38A0ImcLhCoTie0oT4VR6XsR/lGBR1iWWnOyIfDZ Mw== From: Jonathan Davies To: Gerd Hoffmann Thread-Topic: [PATCH] usb: drop unnecessary usb_device_post_load checks Thread-Index: AQHUprGk8EDGxqfDW0KXuehODuQRIw== Date: Mon, 7 Jan 2019 17:51:40 +0000 Message-ID: <20190107175117.23769-1-jonathan.davies@nutanix.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: BYAPR11CA0043.namprd11.prod.outlook.com (2603:10b6:a03:80::20) To BN6PR02MB2818.namprd02.prod.outlook.com (2603:10b6:404:fc::17) x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [62.254.189.133] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; BN6PR02MB2740; 6:l/7tsnrSpLGqhBwR9GSbPBox2lBoBCrHxMVGgBmBm6bqrZ2V7eLiPeZ9a5ve/1KHULZJ5Dpw4dcwGnycCt1fzDgEQgW6uiNWLa/adUZO89Mjv25tGGKRIDjvE7xZBXSEXajmlVkb7Yx6gSMtVdaiFnmF3tviB1Ip435njfoCcSMKrWdvNMgrLGiwTRGPvo9ds5Zoz8ZTNLSnBjGPo0gv+wSVpZ0ZnJ1OT+ke2M5B0Zrf0CGiromI4xZPhAXdJCfw03/tqiTTxpCT6TQtL3edWMqjfISWe1+aGdHvleDAWpzN0tbws0v5kEM0PZbD/9FEdTzai6LNen2Mjlua7ny958Q+tPAn2P/KYNpUCl1S/jnZdc9SnrBNGfI4mCKw6wKxfuHlLaTTdv9sYrPB71RXqVSGEvwe3RpFmpLen79r/bdxpVtiT8jt/bnu8ABUEhRzXpAfcYFJXuawsvMneKLamQ==; 5:1t8vn/HBaRSfX9PMmCU/pLrQWLRxT0WBZF6bIyFygPp30haCV/QqBC6NMdto6qvE2PcCUV+BKKEOjavPP304dpwzHK+Jnfnu1+cSjaK+3yDW1wHNL92iZ0mIoQZ/q2cbO6BCkTRSA3QCYl4Jp3chvmldCj+432zRmYRbNzYd4jiLGm32h/v6cmNGAr4MPaMjnFl92zWRESnJxh+XO+Uo+g==; 7:5HicrZLebwoPBpv3xm7ad1MpP+yFlTk0HELXfm6qF2z7y+JoqnByohAdPAqV9DewAVeTJ79kJisPQJFMv9hbH3ishQUewYROluwIVuEvfO+KO1swLQXMV1/45o8N0oD8ZQ9zEhZYE4epQN+QtA0T5g== x-ms-office365-filtering-correlation-id: 5d448250-6910-4f37-e7f6-08d674c8c6ea x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600109)(711020)(2017052603328)(7153060)(7193020); SRVR:BN6PR02MB2740; x-ms-traffictypediagnostic: BN6PR02MB2740: x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(3230021)(908002)(999002)(5005026)(6040522)(8220060)(2401047)(8121501046)(10201501046)(93006095)(93001095)(3002001)(3231475)(944501520)(52105112)(6041310)(20161123562045)(20161123560045)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(201708071742011)(7699051)(76991095); SRVR:BN6PR02MB2740; BCL:0; PCL:0; RULEID:; SRVR:BN6PR02MB2740; x-forefront-prvs: 0910AAF391 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(396003)(39850400004)(376002)(346002)(366004)(136003)(199004)(189003)(99286004)(386003)(2616005)(86362001)(476003)(36756003)(2906002)(486006)(44832011)(3846002)(6116002)(105586002)(106356001)(8676002)(81156014)(8936002)(81166006)(316002)(54906003)(14454004)(68736007)(1076003)(97736004)(7736002)(305945005)(6486002)(478600001)(25786009)(52116002)(6436002)(4326008)(6512007)(53936002)(66066001)(102836004)(26005)(186003)(256004)(6506007)(14444005)(5660300001)(5024004)(71200400001)(6916009)(71190400001)(64030200001); DIR:OUT; SFP:1102; SCL:1; SRVR:BN6PR02MB2740; H:BN6PR02MB2818.namprd02.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; received-spf: None (protection.outlook.com: nutanix.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: p4McSaL/c3bqo6ncjhif5tzHtJYiBJ5TosWpzmwfAqT/GSCIv4IAu59sAwymdT2al/IK2ETBTjKWKtQqSxvk1TdT/v9Kkw8chIN7D2gVPwFTTz63Un30IVSk2e3CBuevzZuyJz+gwRrxDo8wN7/9ZOOWzWUlXWeWLmLzuYMz1oODR3qXDkK3PCiFSrA8p84vUuRkNLl3spYyHo8f+qeHXQj6pEFRb78Ex6gzHq43tBokZ2WSddq6gkDUjom419XFaY5PYdEatHDy3PcTlEIhMVCk1+yCoWRmzCgkLxVzOf3oLejGZupH2JN24QwVzSpM spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nutanix.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5d448250-6910-4f37-e7f6-08d674c8c6ea X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jan 2019 17:51:40.3287 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: bb047546-786f-4de1-bd75-24e5b6f79043 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR02MB2740 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-01-07_08:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1901070152 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.151.68 X-Mailman-Approved-At: Mon, 07 Jan 2019 12:57:24 -0500 Subject: [Qemu-devel] [PATCH] usb: drop unnecessary usb_device_post_load checks X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jonathan Davies , "qemu-devel@nongnu.org" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) In usb_device_post_load, certain values of dev->setup_len or dev->setup_index can cause -EINVAL to be returned. One example is when setup_len exceeds 4096, the hard-coded value of sizeof(dev->data_buf). This can happen through legitimate guest activity and will cause all subsequent attempts to migrate the guest to fail in vmstate_load_state. The values of these variables can be set by USB packets originating in the guest. There are two ways in which they can be set: in do_token_setup and in do_parameter in hw/usb/core.c. It is easy to craft a USB packet in a guest that causes do_token_setup to set setup_len to a value larger than 4096. When this has been done once, all subsequent attempts to migrate the VM will fail in usb_device_post_load until the VM is next power-cycled or a smaller-sized USB packet is sent to the device. Sample code for achieving this in a VM started with "-device usb-tablet" running Linux with CONFIG_HIDRAW=3Dy and HID_MAX_BUFFER_SIZE > 4096: #include #include #include #include int main() { char buf[4097]; int fd =3D open("/dev/hidraw0", O_RDWR|O_NONBLOCK); buf[0] =3D 0x1; write(fd, buf, 4097); return 0; } When this code is run in the VM, qemu will output: usb_generic_handle_packet: ctrl buffer too small (4097 > 4096) A subsequent attempt to migrate the VM will fail and output the following on the destination host: qemu-kvm: error while loading state for instance 0x0 of device '0000:00:0= 6.7/1/usb-ptr' qemu-kvm: load of migration failed: Invalid argument The idea behind checking the values of setup_len and setup_index before they are used is correct, but doing it in usb_device_post_load feels arbitrary, and will cause unnecessary migration failures. Indeed, none of the commit messages for c60174e8, 9f8e9895 and 719ffe1f justify why post_load is the right place to do these checks. They correctly point out that the important thing to protect is the usb_packet_copy. Instead, the right place to do the checks is in do_token_setup and do_parameter. Indeed, there are already some checks here. We can examine each of the disjuncts currently tested in usb_device_post_load to see whether any need adding to do_token_setup or do_parameter to improve safety there: * dev->setup_index < 0 - This test is not needed because setup_index is explicitly set to 0 in do_token_setup and do_parameter. * dev->setup_len < 0 - In both do_token_setup and do_parameter, the value of setup_len is computed by (s->setup_buf[7] << 8) | s->setup_buf[6]. Since s->setup_buf is a byte array and setup_len is an int32_t, it's impossible for this arithmetic to set setup_len's top bit, so it can never be negative. * dev->setup_index > dev->setup_len - Since setup_index is 0, this is equivalent to the previous test, so is redundant. * dev->setup_len > sizeof(dev->data_buf) - This condition is already explicitly checked in both do_token_setup and do_parameter. Hence there is no need to bolster the existing checks in do_token_setup or do_parameter, and we can safely remove these checks from usb_device_post_load without reducing safety but allowing migrations to proceed regardless of what USB packets have been generated by the guest. Signed-off-by: Jonathan Davies --- hw/usb/bus.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index bf796d67e6..6fffab7bfa 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -59,12 +59,6 @@ static int usb_device_post_load(void *opaque, int versio= n_id) } else { dev->attached =3D true; } - if (dev->setup_index < 0 || - dev->setup_len < 0 || - dev->setup_index > dev->setup_len || - dev->setup_len > sizeof(dev->data_buf)) { - return -EINVAL; - } return 0; } =20 --=20 2.17.1