From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CABE130B51E for ; Wed, 17 Jun 2026 01:42:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781660558; cv=none; b=X3ML8il7fmjsMPd3NC9IzYeqzDx8EQq3XZcaS6F8iIvye+wUNUXPDOsLgzCZZU10NT011QOeB1OfXHNEHuYxwM1S4i5r4ZZKwA6jKGxjnUAtLHh+MWFLMJ0hl9B0HZBK704zMcHU9Zv+yhIKSRwMaM0nSCXWqteNNPScR6kCDAk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781660558; c=relaxed/simple; bh=rkpcOvO+znE6lpdqY88ep7pX0qUzouGy7c0zlyBGrgY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=i6/P3wUXwoZR2U1+F/N8W0aQYBO8isuqf8frbSqvW3hzMghSlBr9XDYB2P3lrdXXlpuLyYe8En9A26ikPyeipkxGiQbo8e1yUTcQatFVNubtq6ixQXmPa8+jQV9lK1oRiAkEF/InF0NUahMI1wtshtVyyVKKiMFNTHUw9Ao4YDE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=dfKjW9Of; arc=none smtp.client-ip=209.85.160.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dfKjW9Of" Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-517654b8e28so37796191cf.3 for ; Tue, 16 Jun 2026 18:42:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781660549; x=1782265349; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=1ltgoW4HzxXM53BHwLsRBi+lA49LaXw3fkOJPn5XnrI=; b=dfKjW9OflyQD9UK9MtYtfzKE7GZlEbACubHSrQKkUp4nrKgn/ijMlLYxGexHLMUUN3 1VBVs4HyAzxIvXq4mR3VH7IwrBnjlJfPEz0OMsFF1X3tjY3FMkcT9RImyL0UvLLlBB40 V/RaQcpDhTcp0cj752tn/T1VP/C9GoJvZPxFuknp3n2rfcLIEUFOyDbgfbh6z22dSMjK VCxDS+RdiNTr7PUyvViD3znXnMnMk3hZurVPGFoNZwNhbYWY5FbcbCUO6nqwUcEyWGWX AVe/aUBW2+AOqjh1dzzSebBPb2ekxphcxWsK3ep0wpyEL4QrHRBb9SKSws/FotPvmS// iDRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781660549; x=1782265349; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=1ltgoW4HzxXM53BHwLsRBi+lA49LaXw3fkOJPn5XnrI=; b=tDxEpeVKsiHNHurM1LtBHX72HvjDtjIWr4T5Dj5e4jzq40s4TwRklfVTm9QiyekWum Z5iXScMSXoQ8oiF6MlIIL5j7Jtarz2hsA527ejN+XgSDRn2kEUu0Z8zcadk1r0OdJrI5 QV8Zncu4lKO+h+x6OWORG3q4oDdHda+nuWmnqz/lsxZR+FDale3/cs5/CzYTylyq9Tka XhAgyqOnvzjGvFPjvMxkuV5HCF/Fcy+Jg9dkCXK+Sv5kDNUwqrCxPUgyvEIY8F+djjQt 7p78dE5AbxgQt4LydQqU/8qRd5iYew32l1nLL4Y9oVHWAqx/4TGPzZxQ1Md1fF7Pzhlz TXHw== X-Forwarded-Encrypted: i=1; AFNElJ+m8sM3S1GlHK2NXCEExikdwuvT9Fejp4Mvd1OODWsHqpQ+xUdb4Nsc2fO5NC8flGeoIaluwvffCiU/F0E=@vger.kernel.org X-Gm-Message-State: AOJu0YyXDKyz4GDqLBt23KxRWIAN++J/uYcIkXXkvVQ/omZRr99CSj2x pP85XwzldpRf3uwPg3CPgDSWQE3qCurbPZXiFIdMp/ATe0ctGkylRGC9mrP794l/ X-Gm-Gg: Acq92OEz4x7ZxZSbZVsJxKg2plETQYNklGw0Z9MRB0OGvmroIHSGOOtfHZpesngGPVl Wwbv+QcxikSJRKMXXPPSCRHLNY37h5qUSIAVhs6Dv7U9/R5jpsPCSQUhNvuPWCaVRqKBfq96HJ3 ees/7mE1ltKwIloxO/glteCeF9EaSu1qEEcscNS8re8UOUjE2aMrp2WIJ78YWtLbPFMk+RGnu8k eOy5/az2N0JXTvZk7j0QaDVxZgUmYiA0RJAxGSBTpK2ynEEylVvPn9zC2+D4KyYO//y9oPD10NT FtWObLYkW6uHvThcdw1J39HzhyhWv4yy4gEHMbMDVV5XcoQDStyz0wCF1gVZG3ZvzPsOLuwaKZx TfMGSjkEX0G+M4eQe89g6+QsjrqPTHhA+5/Y/UZJ//6eWd9C3ZNpXSCItvrrAoQJ9NCptm3U7hA zjnO9gy6dkWydSX9x+2T0k6dwLQX4AQSDArGt4ym5/Fk40SDkatS/5RWM19K8d+KlZkTguxZN5k Dm3qTXoNUY9CBY7g+x2fIuOEF+ZF/UN X-Received: by 2002:a05:622a:56cc:b0:517:6350:ed50 with SMTP id d75a77b69052e-519a8faa207mr25326481cf.45.1781660549330; Tue, 16 Jun 2026 18:42:29 -0700 (PDT) Received: from server0.tail6e7dd.ts.net (c-68-48-65-54.hsd1.mi.comcast.net. [68.48.65.54]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-517fb7ec47asm156567221cf.24.2026.06.16.18.42.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jun 2026 18:42:28 -0700 (PDT) From: Michael Bommarito To: Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] xen/pvcalls: bound backend response req_id before indexing rsp[] Date: Tue, 16 Jun 2026 21:41:49 -0400 Message-ID: <20260617014149.2647404-1-michael.bommarito@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260610114137.3749027-1-michael.bommarito@gmail.com> References: <20260610114137.3749027-1-michael.bommarito@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7bit pvcalls_front_event_handler() takes req_id directly from the backend-supplied ring response and uses it to index the fixed-size bedata->rsp[] array for a memcpy() and a store, with no range check. A malicious or buggy backend can set req_id past PVCALLS_NR_RSP_PER_RING and drive an out-of-bounds write past the bedata allocation. req_id was also declared int while the wire field rsp->req_id is u32, so a range check on the signed value alone is insufficient: a backend req_id of 0xffffffff becomes -1, passes a >= PVCALLS_NR_RSP_PER_RING test and indexes bedata->rsp[-1]. Declare req_id as u32 so a single bound covers both ends. A backend that sends an out-of-range req_id has violated the wire protocol, so rather than silently dropping the response, log once and stop trusting the backend: set bedata->disabled. The event handler then ignores further responses, and the request paths that wait for a response return -EIO instead of blocking forever. This mirrors the fatal-error handling xen-netback uses (xenvif_fatal_tx_err()). The pvcalls frontend currently trusts its backend, so this is not a classic-Xen security issue, but it matters for hardening PV frontends against malicious backends (confidential and disaggregated deployments). Fixes: 2195046bfd69 ("xen/pvcalls: implement socket command and handle events") Suggested-by: Juergen Gross Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Michael Bommarito --- v2: per Juergen Gross's review (https://lore.kernel.org/all/ecb43fc6-e821-4532-9f75-06c86a6ac76c@suse.com/): - Log the out-of-range req_id once with pr_err_once() instead of silently dropping the response. - Stop trusting the backend on a protocol violation: set bedata->disabled so the event handler ignores further responses and the request paths waiting for a response return -EIO instead of blocking forever, following the xen-netback xenvif_fatal_tx_err() pattern you pointed to. - Declare req_id as u32 (was int) so a single bound covers both ends. - pvcalls_front_accept() has a second waiter (a concurrent accept blocked on PVCALLS_FLAG_ACCEPT_INFLIGHT). On the disabled path, clear that flag and wake inflight_accept_req so the queued accept also returns -EIO rather than waiting for a response that the disabled handler will never deliver. - Corrected the Fixes: tag to 2195046bfd69, the commit that introduced the unbounded bedata->rsp[req_id] indexing in the event handler; the previously cited 235a71c53903 (release command) only added a waiter and is a descendant. v1: https://lore.kernel.org/all/20260610114137.3749027-1-michael.bommarito@gmail.com/ drivers/xen/pvcalls-front.c | 88 ++++++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 12 deletions(-) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 50ce4820f7eeb..3e7aa807c3173 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -32,6 +32,7 @@ struct pvcalls_bedata { struct xen_pvcalls_front_ring ring; grant_ref_t ref; int irq; + bool disabled; struct list_head socket_mappings; spinlock_t socket_lock; @@ -131,6 +132,20 @@ static inline int get_request(struct pvcalls_bedata *bedata, int *req_id) return 0; } +/* + * Wait for the backend's response to req_id, or for the frontend to be + * disabled because the backend violated the wire protocol. Returns 0 once + * the response has arrived, or -EIO if the frontend was disabled. + */ +static int pvcalls_front_wait_rsp(struct pvcalls_bedata *bedata, u32 req_id) +{ + wait_event(bedata->inflight_req, + READ_ONCE(bedata->rsp[req_id].req_id) == req_id || + READ_ONCE(bedata->disabled)); + + return READ_ONCE(bedata->disabled) ? -EIO : 0; +} + static bool pvcalls_front_write_todo(struct sock_mapping *map) { struct pvcalls_data_intf *intf = map->active.ring; @@ -168,7 +183,8 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) struct pvcalls_bedata *bedata; struct xen_pvcalls_response *rsp; uint8_t *src, *dst; - int req_id = 0, more = 0, done = 0; + u32 req_id = 0; + int more = 0, done = 0; if (dev == NULL) return IRQ_HANDLED; @@ -179,12 +195,31 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) pvcalls_exit(); return IRQ_HANDLED; } + if (READ_ONCE(bedata->disabled)) { + pvcalls_exit(); + return IRQ_HANDLED; + } again: while (RING_HAS_UNCONSUMED_RESPONSES(&bedata->ring)) { rsp = RING_GET_RESPONSE(&bedata->ring, bedata->ring.rsp_cons); req_id = rsp->req_id; + if (req_id >= PVCALLS_NR_RSP_PER_RING) { + /* + * The backend supplied a req_id that would index + * bedata->rsp[] out of bounds: a protocol violation + * from a malicious or buggy backend. Log once, stop + * trusting this backend and disable the frontend rather + * than silently dropping the response and continuing. + */ + pr_err_once("pvcalls: backend sent out-of-range req_id %u, disabling frontend\n", + req_id); + WRITE_ONCE(bedata->disabled, true); + bedata->ring.rsp_cons++; + done = 1; + break; + } if (rsp->cmd == PVCALLS_POLL) { struct sock_mapping *map = (struct sock_mapping *)(uintptr_t) rsp->u.poll.id; @@ -217,7 +252,7 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id) } RING_FINAL_CHECK_FOR_RESPONSES(&bedata->ring, more); - if (more) + if (more && !READ_ONCE(bedata->disabled)) goto again; if (done) wake_up(&bedata->inflight_req); @@ -330,8 +365,11 @@ int pvcalls_front_socket(struct socket *sock) if (notify) notify_remote_via_irq(bedata->irq); - wait_event(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + ret = pvcalls_front_wait_rsp(bedata, req_id); + if (ret) { + pvcalls_exit(); + return ret; + } /* read req_id, then the content */ smp_rmb(); @@ -477,8 +515,11 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, if (notify) notify_remote_via_irq(bedata->irq); - wait_event(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + ret = pvcalls_front_wait_rsp(bedata, req_id); + if (ret) { + pvcalls_exit_sock(sock); + return ret; + } /* read req_id, then the content */ smp_rmb(); @@ -711,8 +752,11 @@ int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len) if (notify) notify_remote_via_irq(bedata->irq); - wait_event(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + ret = pvcalls_front_wait_rsp(bedata, req_id); + if (ret) { + pvcalls_exit_sock(sock); + return ret; + } /* read req_id, then the content */ smp_rmb(); @@ -761,8 +805,11 @@ int pvcalls_front_listen(struct socket *sock, int backlog) if (notify) notify_remote_via_irq(bedata->irq); - wait_event(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + ret = pvcalls_front_wait_rsp(bedata, req_id); + if (ret) { + pvcalls_exit_sock(sock); + return ret; + } /* read req_id, then the content */ smp_rmb(); @@ -820,6 +867,14 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, } } + if (READ_ONCE(bedata->disabled)) { + clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, + (void *)&map->passive.flags); + wake_up(&map->passive.inflight_accept_req); + pvcalls_exit_sock(sock); + return -EIO; + } + map2 = kzalloc_obj(*map2); if (map2 == NULL) { clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, @@ -880,10 +935,18 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, } if (wait_event_interruptible(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id)) { + READ_ONCE(bedata->rsp[req_id].req_id) == req_id || + READ_ONCE(bedata->disabled))) { pvcalls_exit_sock(sock); return -EINTR; } + if (READ_ONCE(bedata->disabled)) { + clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT, + (void *)&map->passive.flags); + wake_up(&map->passive.inflight_accept_req); + pvcalls_exit_sock(sock); + return -EIO; + } /* read req_id, then the content */ smp_rmb(); @@ -1054,7 +1117,8 @@ int pvcalls_front_release(struct socket *sock) notify_remote_via_irq(bedata->irq); wait_event(bedata->inflight_req, - READ_ONCE(bedata->rsp[req_id].req_id) == req_id); + READ_ONCE(bedata->rsp[req_id].req_id) == req_id || + READ_ONCE(bedata->disabled)); if (map->active_socket) { /* -- 2.53.0