From nobody Thu Feb 22 10:31:17 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TgTvT4QsDz5CMDN; Thu, 22 Feb 2024 10:31:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TgTvT3sVDz4kRZ; Thu, 22 Feb 2024 10:31:17 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1708597877; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=VlSGbcSbBXMkwVRdyYQMTDsbdwCPSxSfGCDNAYTZLcI=; b=P84P8i7qHPz1NzhUN+o6IvNQDMK+8wFWhK+D641CGFbdF4TZ9yytJMIWxZRsRKYa+BmefZ me+Nyvy5xmq+N2QVAejCZ/ahDaUAyaEKlx/nDzx4oDhJlhNLUfwzANqyMzdpwHWsLMhhyz CM+S++Npv6IMtdUxi6mJXr2ktyoJ+BD8TYpUQxu2w2C2sK7BbpTynh3uosC0mpX/2WzOwH HX/Up+gm3hVoIaBPVkGvMq+66JVEikwWJ6NJpO68AKl5I8GEMDeSjZS7BhafE8bOHuqdYY lG5GWSbm3DGY0u/WWEv/WonE9SR/RfNouelvELK0WxnFDshRKS6UiH/UA2lEeA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1708597877; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=VlSGbcSbBXMkwVRdyYQMTDsbdwCPSxSfGCDNAYTZLcI=; b=Juq2c1MIM6kCQu1Vyz6VZB4PnHWM7m4xod8viYgoj5LVM3ehJ+Se1rBi08/9r6iahY00ao MUrc0Qex4uowUs9yM23+nK+izezcUavGjEuYIvGG3Wvalf4yd1Ix8L6Ed2s+TDytXPsD/i JZ1TQAa/A7JGc7OWBd5h2Z9CGd6mYyxCAWqzJ8/il/vXsiUZRp+xRZw97bKQUlQhFBGDRg vPB288MGs6N6zPZPcHBO8+SM05FW+PFFc0xnegGwr1bTsHuo0klguIxLoG+TxiO9WjsNe7 x/qETKS9gNb4Yiwm+NEu6vqcvdiiPuowkwVgZz8F2hu8FBwQNpO2oeRrlVUcPA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1708597877; a=rsa-sha256; cv=none; b=qswvuT6JAYnMD4PDgSZWX6XpmZDncF4GYwX0fI3zfEthgPjw/OIq6HVjK5/qx6SFg8oroa cXCtGaSt1LYGJ347m+DAM4+Hq/aDb0ShIr23vh2YNBwapXcoop2b3dvywPCFmhWKLZmbIj eP5EA3uq+dI1Z6pxbYQKUXrZQvzvSu49LT8ZtPKFyjwORJ6ToCqe8VnClPowJSpQZ/0wFw cKbeIrlCD9A6Xp6zfuLhlwoEh35a9zbTq/rWzBVOeA9lK2bIwKnkEQ5yi+uKcmHXEDFYMX I0g5vU3kr3YMQ/0wOUv+vA/0E4XJ97EJvic+fAFQXBneJuvBDanqm/bAUskxhA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4TgTvT2xCFzSK9; Thu, 22 Feb 2024 10:31:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 41MAVHML016538; Thu, 22 Feb 2024 10:31:17 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 41MAVHj1016535; Thu, 22 Feb 2024 10:31:17 GMT (envelope-from git) Date: Thu, 22 Feb 2024 10:31:17 GMT Message-Id: <202402221031.41MAVHj1016535@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Roger Pau =?utf-8?Q?Monn=C3=A9?= Subject: git: 4ece79968e70 - main - x86/xen: fix out of bounds access to the event channel masks on resume List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: royger X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 4ece79968e70b96d3283f202635d49a4bf04a7e7 Auto-Submitted: auto-generated The branch main has been updated by royger: URL: https://cgit.FreeBSD.org/src/commit/?id=4ece79968e70b96d3283f202635d49a4bf04a7e7 commit 4ece79968e70b96d3283f202635d49a4bf04a7e7 Author: Roger Pau Monné AuthorDate: 2024-02-05 10:47:25 +0000 Commit: Roger Pau Monné CommitDate: 2024-02-22 10:08:03 +0000 x86/xen: fix out of bounds access to the event channel masks on resume When resuming from migration or suspension all regular event channels ports are reset to the INVALID_EVTCHN value, and drivers should re-initialize them according to the new value provided by the other end of the connection. However, the driver would first attempt to unbind the event channel handler before attempting to bind it using the newly provided port. This unbind uses the stale event channel port that has been set to INVALID_EVTCHN for some operations (notably as a result of the handler removal the interrupt subsystem ends up calling disable intr and source PIC hooks). This was fine when INVALID_EVTCHN was 0, as then the operation would just result in pointless setting of the 0 bit in the different event channel related control arrays (evtchn_{pending,mask} for example). However with the change to define INVALID_EVTCHN as ~0 the write is no longer pointless, and we end up triggering a page-fault, or corrupting random data that happens to be mapped at the array position + ~0 bits. In hindsight the change of INVALID_EVTCHN from 0 to ~0 was way more risky than initially assessed, and I believe has end up resulting in more fragile code for no real benefit. Fix the disable intr and source wrappers to check whether the event channel is valid before attempting to use it. Also introduce some extra KASSERTs in several array accesses in order to avoid out of bounds accesses if INVALID_EVTCHN ever reaches those functions. Fixes: 1797ff962769 ('xen/intr: cleanup event channel number use') MFC after: 1 week Sponsored by: Cloud Software Group Reviewed by: markj Differential revision: https://reviews.freebsd.org/D43928 --- sys/dev/xen/bus/xen_intr.c | 8 ++++++-- sys/xen/evtchn/evtchnvar.h | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/sys/dev/xen/bus/xen_intr.c b/sys/dev/xen/bus/xen_intr.c index 3fc8fb0fe83a..609f31b5418a 100644 --- a/sys/dev/xen/bus/xen_intr.c +++ b/sys/dev/xen/bus/xen_intr.c @@ -166,6 +166,7 @@ evtchn_cpu_mask_port(u_int cpu, evtchn_port_t port) struct xen_intr_pcpu_data *pcpu; pcpu = DPCPU_ID_PTR(cpu, xen_intr_pcpu); + KASSERT(is_valid_evtchn(port), ("Invalid event channel port")); xen_clear_bit(port, pcpu->evtchn_enabled); } @@ -188,6 +189,7 @@ evtchn_cpu_unmask_port(u_int cpu, evtchn_port_t port) struct xen_intr_pcpu_data *pcpu; pcpu = DPCPU_ID_PTR(cpu, xen_intr_pcpu); + KASSERT(is_valid_evtchn(port), ("Invalid event channel port")); xen_set_bit(port, pcpu->evtchn_enabled); } @@ -619,7 +621,8 @@ void xen_intr_disable_intr(struct xenisrc *isrc) { - evtchn_mask_port(isrc->xi_port); + if (__predict_true(is_valid_evtchn(isrc->xi_port))) + evtchn_mask_port(isrc->xi_port); } /** @@ -706,7 +709,8 @@ xen_intr_disable_source(struct xenisrc *isrc) * unmasked by the generic interrupt code. The event channel * device will unmask them when needed. */ - isrc->xi_masked = !!evtchn_test_and_set_mask(isrc->xi_port); + if (__predict_true(is_valid_evtchn(isrc->xi_port))) + isrc->xi_masked = !!evtchn_test_and_set_mask(isrc->xi_port); } /* diff --git a/sys/xen/evtchn/evtchnvar.h b/sys/xen/evtchn/evtchnvar.h index 455f7bcbd620..3e6575748f6b 100644 --- a/sys/xen/evtchn/evtchnvar.h +++ b/sys/xen/evtchn/evtchnvar.h @@ -37,8 +37,11 @@ #include /* Macros for accessing event channel values */ -#define EVTCHN_PTR(type, port) \ - (HYPERVISOR_shared_info->evtchn_##type + ((port) / __LONG_BIT)) +#define EVTCHN_PTR(type, port) ({ \ + KASSERT(port < nitems(HYPERVISOR_shared_info->evtchn_##type) * \ + sizeof(xen_ulong_t) * 8, ("Invalid event channel port")); \ + (HYPERVISOR_shared_info->evtchn_##type + ((port) / __LONG_BIT));\ +}) #define EVTCHN_BIT(port) ((port) & (__LONG_BIT - 1)) #define EVTCHN_MASK(port) (1UL << EVTCHN_BIT(port))