From 74186c2968f8f756ac3226b545b598457c910c75 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:13 +0200 Subject: [PATCH 01/26] s390/vfio_ccw: Free all memory if cp_init() fails The routine cp_free() is called to unpin/free any memory once an I/O is completed successfully, or if cp_prefetch() fails. But if cp_init() fails, and cp->initialized is not enabled, the same routine cannot be used to free all the memory. An attempt to address this exists in ccwchain_handle_ccw(), where a single call to ccwchain_free() is made for the currently-processed CCW segment. But this will leak other segments (created as a result of a Transfer in Channel) that had been allocated as part of the same channel program. Address this by performing the cleanup outside of the recursive ccwchain_handle_ccw()/ccwchain_loop_tic() logic. Fixes: 8b515be512a2 ("vfio-ccw: Fix memory leak and don't call cp_free in cp_init") Cc: stable@vger.kernel.org Reviewed-by: Farhan Ali Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_cp.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 7561aa7d3e01..086d1b54bdb0 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -455,9 +455,6 @@ static int ccwchain_handle_ccw(dma32_t cda, struct channel_program *cp) /* Loop for tics on this new chain. */ ret = ccwchain_loop_tic(chain, cp); - if (ret) - ccwchain_free(chain); - return ret; } @@ -486,6 +483,23 @@ static int ccwchain_loop_tic(struct ccwchain *chain, struct channel_program *cp) return 0; } +static int ccwchain_build_ccws(dma32_t cda, struct channel_program *cp) +{ + struct ccwchain *chain, *temp; + int ret; + + ret = ccwchain_handle_ccw(cda, cp); + + if (ret) { + /* Cleanup if an error occurred */ + list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) { + ccwchain_free(chain); + } + } + + return ret; +} + static int ccwchain_fetch_tic(struct ccw1 *ccw, struct channel_program *cp) { @@ -735,7 +749,7 @@ int cp_init(struct channel_program *cp, union orb *orb) memcpy(&cp->orb, orb, sizeof(*orb)); /* Build a ccwchain for the first CCW segment */ - ret = ccwchain_handle_ccw(orb->cmd.cpa, cp); + ret = ccwchain_build_ccws(orb->cmd.cpa, cp); if (!ret) cp->initialized = true; From 5405c90d6a47b3014e74ee0618a162449abbbc93 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:14 +0200 Subject: [PATCH 02/26] s390/vfio_ccw: Limit the number of channel program segments The processing of channel programs, and the CCWs within them, is done recursively. As such, there is an arbitrary (but not architectural) limit to the number of CCWs that can exist in a single channel program. The vfio-ccw logic breaks these channel programs into segments whenever it encounters a Transfer-In-Channel (TIC) CCW, and the combined number of segments count towards the global limit. Impose an equivalent limit to the number of segments until such logic can be made non-recursive. Fixes: 0a19e61e6d4c ("vfio: ccw: introduce channel program interfaces") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_cp.c | 6 ++++++ drivers/s390/cio/vfio_ccw_cp.h | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 086d1b54bdb0..1c2890d139c6 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -332,6 +332,7 @@ static struct ccwchain *ccwchain_alloc(struct channel_program *cp, int len) goto out_err; list_add_tail(&chain->next, &cp->ccwchain_list); + cp->ccwchain_count++; return chain; @@ -441,6 +442,10 @@ static int ccwchain_handle_ccw(dma32_t cda, struct channel_program *cp) if (len < 0) return len; + /* Limit number of chains in a single channel program */ + if (cp->ccwchain_count >= CCWCHAIN_COUNT_MAX) + return -EINVAL; + /* Need alloc a new chain for this one. */ chain = ccwchain_alloc(cp, len); if (!chain) @@ -745,6 +750,7 @@ int cp_init(struct channel_program *cp, union orb *orb) vdev->dev, "Prefetching channel program even though prefetch not specified in ORB"); + cp->ccwchain_count = 0; INIT_LIST_HEAD(&cp->ccwchain_list); memcpy(&cp->orb, orb, sizeof(*orb)); diff --git a/drivers/s390/cio/vfio_ccw_cp.h b/drivers/s390/cio/vfio_ccw_cp.h index fc31eb699807..a9b1d8dbc6f6 100644 --- a/drivers/s390/cio/vfio_ccw_cp.h +++ b/drivers/s390/cio/vfio_ccw_cp.h @@ -23,11 +23,18 @@ */ #define CCWCHAIN_LEN_MAX 256 +/* + * Maximum number of chains + */ +#define CCWCHAIN_COUNT_MAX 16 + /** * struct channel_program - manage information for channel program * @ccwchain_list: list head of ccwchains * @orb: orb for the currently processed ssch request * @initialized: whether this instance is actually initialized + * @guest_cp: copy of guest channel program + * @ccwchain_count: number of channel program segments (linked by TIC) * * @ccwchain_list is the head of a ccwchain list, that contents the * translated result of the guest channel program that pointed out by @@ -38,6 +45,7 @@ struct channel_program { union orb orb; bool initialized; struct ccw1 *guest_cp; + unsigned int ccwchain_count; }; int cp_init(struct channel_program *cp, union orb *orb); From a005b7f1a491ffda61bff0fd0f6548f8986fb977 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:15 +0200 Subject: [PATCH 03/26] s390/vfio_ccw: Fix out of bounds check on CCW array The routine ccwchain_calc_length() counts the number of channel command words (CCWs) that are chained together in a single channel program, and rejects anything larger than CCWCHAIN_LEN_MAX (256) CCWs. The loop itself is "do..while (count < 257)", and while the logic in is_cpa_within_range() correctly adjusts between the 0-index array of CCWs and the count of CCWs starting at 1, this means it would look at a possible 257th CCW before ending the loop and (correctly) returning an error. Fix this by restructuring the loop to break as soon as 256 CCWs (thus indexes 0-255) are examined, without looking at memory outside the range. Fixes: 0a19e61e6d4c ("vfio: ccw: introduce channel program interfaces") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_cp.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 1c2890d139c6..af632f9d5453 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -377,11 +377,9 @@ static void ccwchain_cda_free(struct ccwchain *chain, int idx) static int ccwchain_calc_length(u64 iova, struct channel_program *cp) { struct ccw1 *ccw = cp->guest_cp; - int cnt = 0; - - do { - cnt++; + int cnt; + for (cnt = 1; cnt <= CCWCHAIN_LEN_MAX; cnt++, ccw++) { /* * We want to keep counting if the current CCW has the * command-chaining flag enabled, or if it is a TIC CCW @@ -391,15 +389,10 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp) * after the TIC, depending on the results of its operation. */ if (!ccw_is_chain(ccw) && !is_tic_within_range(ccw, iova, cnt)) - break; + return cnt; + } - ccw++; - } while (cnt < CCWCHAIN_LEN_MAX + 1); - - if (cnt == CCWCHAIN_LEN_MAX + 1) - cnt = -EINVAL; - - return cnt; + return -EINVAL; } static int tic_target_chain_exists(struct ccw1 *tic, struct channel_program *cp) From 565bef268d75bf7df665bce6923a88cd0eb74592 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:16 +0200 Subject: [PATCH 04/26] s390/vfio_ccw: Ensure first IDAW remains constant The first IDAW in a list does not need to be on a 2K/4K boundary like all others, and so is read separately to accurately calculate the size of the buffer needed to read the full IDAL. Verify that the address found in the first IDAW is unchanged between reads, to ensure a consistent set of IDAWs being worked with. Fixes: 01aa26c672c0 ("s390/cio: Combine direct and indirect CCW paths") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_cp.c | 16 ++++++++++++++++ drivers/s390/cio/vfio_ccw_cp.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index af632f9d5453..6275794751cb 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -523,6 +523,7 @@ static dma64_t *get_guest_idal(struct ccw1 *ccw, struct channel_program *cp, int &container_of(cp, struct vfio_ccw_private, cp)->vdev; dma64_t *idaws; dma32_t *idaws_f1; + u64 first_idaw; int idal_len = idaw_nr * sizeof(*idaws); int idaw_size = idal_is_2k(cp) ? PAGE_SIZE / 2 : PAGE_SIZE; int idaw_mask = ~(idaw_size - 1); @@ -539,6 +540,18 @@ static dma64_t *get_guest_idal(struct ccw1 *ccw, struct channel_program *cp, int kfree(idaws); return ERR_PTR(ret); } + + idaws_f1 = (dma32_t *)idaws; + if (cp->orb.cmd.c64) + first_idaw = dma64_to_u64(idaws[0]); + else + first_idaw = dma32_to_u32(idaws_f1[0]); + + /* Unexpected mismatch from earlier read */ + if (first_idaw != cp->guest_iova) { + kfree(idaws); + return ERR_PTR(-EINVAL); + } } else { /* Fabricate an IDAL based off CCW data address */ if (cp->orb.cmd.c64) { @@ -604,6 +617,9 @@ static int ccw_count_idaws(struct ccw1 *ccw, iova = dma32_to_u32(ccw->cda); } + /* Save the read address for later */ + cp->guest_iova = iova; + /* Format-1 IDAWs operate on 2K each */ if (!cp->orb.cmd.c64) return idal_2k_nr_words((void *)iova, bytes); diff --git a/drivers/s390/cio/vfio_ccw_cp.h b/drivers/s390/cio/vfio_ccw_cp.h index a9b1d8dbc6f6..9af98ff12d67 100644 --- a/drivers/s390/cio/vfio_ccw_cp.h +++ b/drivers/s390/cio/vfio_ccw_cp.h @@ -35,6 +35,7 @@ * @initialized: whether this instance is actually initialized * @guest_cp: copy of guest channel program * @ccwchain_count: number of channel program segments (linked by TIC) + * @guest_iova: first data address of a guest channel program * * @ccwchain_list is the head of a ccwchain list, that contents the * translated result of the guest channel program that pointed out by @@ -46,6 +47,7 @@ struct channel_program { bool initialized; struct ccw1 *guest_cp; unsigned int ccwchain_count; + u64 guest_iova; }; int cp_init(struct channel_program *cp, union orb *orb); From 4f6fdc6e1a7fbfa36b945af33c65a417948feac0 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:17 +0200 Subject: [PATCH 05/26] s390/vfio_ccw: Calculate idal length based on idaw type Sashiko pointed out that get_guest_idal() unconditionally calculates the length of the IDAL presuming everything is a Format-2 IDAW. The output of vfio-ccw is always Format-2, but the input can be either Format-1 (31-bit addresses) or Format-2 (64-bit addresses). As a result, the size of the guest IDAL may be incorrect and should be trimmed down. Reported-by: sashiko-bot Link: https://lore.kernel.org/r/20260720203400.7328E1F000E9@smtp.kernel.org/ Fixes: 1b676fe3d9d3 ("vfio/ccw: handle a guest Format-1 IDAL") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_cp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 6275794751cb..5ef082b8289a 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -233,6 +233,7 @@ static void convert_ccw0_to_ccw1(struct ccw1 *source, unsigned long len) } #define idal_is_2k(_cp) (!(_cp)->orb.cmd.c64 || (_cp)->orb.cmd.i2k) +#define get_idaw_size(_cp) ((_cp)->orb.cmd.c64 ? sizeof(u64) : sizeof(u32)) /* * Helpers to operate ccwchain. @@ -524,7 +525,7 @@ static dma64_t *get_guest_idal(struct ccw1 *ccw, struct channel_program *cp, int dma64_t *idaws; dma32_t *idaws_f1; u64 first_idaw; - int idal_len = idaw_nr * sizeof(*idaws); + int idal_len = idaw_nr * get_idaw_size(cp); int idaw_size = idal_is_2k(cp) ? PAGE_SIZE / 2 : PAGE_SIZE; int idaw_mask = ~(idaw_size - 1); int i, ret; @@ -593,7 +594,7 @@ static int ccw_count_idaws(struct ccw1 *ccw, struct vfio_device *vdev = &container_of(cp, struct vfio_ccw_private, cp)->vdev; u64 iova; - int size = cp->orb.cmd.c64 ? sizeof(u64) : sizeof(u32); + int size = get_idaw_size(cp); int ret; int bytes = 1; From 9f5f9a78fedc45bc29d6a0a64e3a3472361afae5 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:18 +0200 Subject: [PATCH 06/26] s390/vfio_ccw: Ensure index for read/write regions are within range The introduction of the capability chain rightly clamped the region indexes to the range of the capabilities itself, but neglected to do so for the existing read/write regions which should also be enforced. Fixes: db8e5d17ac03 ("vfio-ccw: add capabilities chain") Cc: stable@vger.kernel.org Cc: Cornelia Huck Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_async.c | 16 ++++++++++++++++ drivers/s390/cio/vfio_ccw_chp.c | 15 +++++++++++++++ drivers/s390/cio/vfio_ccw_ops.c | 7 +++---- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_async.c b/drivers/s390/cio/vfio_ccw_async.c index 420d89ba7f83..4aff0b58fa5d 100644 --- a/drivers/s390/cio/vfio_ccw_async.c +++ b/drivers/s390/cio/vfio_ccw_async.c @@ -8,6 +8,7 @@ */ #include +#include #include "vfio_ccw_private.h" @@ -24,11 +25,20 @@ static ssize_t vfio_ccw_async_region_read(struct vfio_ccw_private *private, return -EINVAL; mutex_lock(&private->io_mutex); + + if (i >= private->num_regions) { + ret = -EINVAL; + goto out_unlock; + } + + i = array_index_nospec(i, private->num_regions); region = private->region[i].data; if (copy_to_user(buf, (void *)region + pos, count)) ret = -EFAULT; else ret = count; + +out_unlock: mutex_unlock(&private->io_mutex); return ret; } @@ -48,6 +58,12 @@ static ssize_t vfio_ccw_async_region_write(struct vfio_ccw_private *private, if (!mutex_trylock(&private->io_mutex)) return -EAGAIN; + if (i >= private->num_regions) { + ret = -EINVAL; + goto out_unlock; + } + + i = array_index_nospec(i, private->num_regions); region = private->region[i].data; if (copy_from_user((void *)region + pos, buf, count)) { ret = -EFAULT; diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c index 38c176cf6295..f3015132d4b5 100644 --- a/drivers/s390/cio/vfio_ccw_chp.c +++ b/drivers/s390/cio/vfio_ccw_chp.c @@ -9,6 +9,7 @@ */ #include +#include #include #include "vfio_ccw_private.h" @@ -26,6 +27,13 @@ static ssize_t vfio_ccw_schib_region_read(struct vfio_ccw_private *private, return -EINVAL; mutex_lock(&private->io_mutex); + + if (i >= private->num_regions) { + ret = -EINVAL; + goto out; + } + + i = array_index_nospec(i, private->num_regions); region = private->region[i].data; if (cio_update_schib(sch)) { @@ -97,6 +105,12 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, list_del(&crw->next); mutex_lock(&private->io_mutex); + if (i >= private->num_regions) { + ret = -EINVAL; + goto out; + } + + i = array_index_nospec(i, private->num_regions); region = private->region[i].data; if (crw) @@ -109,6 +123,7 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, region->crw = 0; +out: mutex_unlock(&private->io_mutex); kfree(crw); diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 45ec722d25ea..032a1cdf4df7 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -243,6 +243,7 @@ static ssize_t vfio_ccw_mdev_read(struct vfio_device *vdev, return vfio_ccw_mdev_read_io_region(private, buf, count, ppos); default: index -= VFIO_CCW_NUM_REGIONS; + index = array_index_nospec(index, private->num_regions); return private->region[index].ops->read(private, buf, count, ppos); } @@ -295,6 +296,7 @@ static ssize_t vfio_ccw_mdev_write(struct vfio_device *vdev, return vfio_ccw_mdev_write_io_region(private, buf, count, ppos); default: index -= VFIO_CCW_NUM_REGIONS; + index = array_index_nospec(index, private->num_regions); return private->region[index].ops->write(private, buf, count, ppos); } @@ -338,11 +340,8 @@ static int vfio_ccw_mdev_ioctl_get_region_info(struct vfio_device *vdev, VFIO_CCW_NUM_REGIONS + private->num_regions) return -EINVAL; - info->index = array_index_nospec(info->index, - VFIO_CCW_NUM_REGIONS + - private->num_regions); - i = info->index - VFIO_CCW_NUM_REGIONS; + i = array_index_nospec(i, private->num_regions); info->offset = VFIO_CCW_INDEX_TO_OFFSET(info->index); info->size = private->region[i].size; From 79c60b2c61105368dcc8444eb45847e21734f7c4 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:19 +0200 Subject: [PATCH 07/26] s390/vfio_ccw: Cancel existing workqueues The initialization of the io_work and crw_work workqueues begs the question of whether they should be un-initialized. Add the corresponding cleanup tags in _release_dev to ensure work isn't dispatched after the private struct is free'd. Suggested-by: Matthew Rosato Fixes: e5f84dbaea59 ("vfio: ccw: return I/O results asynchronously") Fixes: 3f02cb2fd9d2 ("vfio-ccw: Wire up the CRW irq and CRW region") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_ops.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 032a1cdf4df7..d361d1fde3a0 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -131,6 +131,13 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) container_of(vdev, struct vfio_ccw_private, vdev); struct vfio_ccw_crw *crw, *temp; + /* + * Ensure these work items are fully drained, so none can + * fire after being released. + */ + cancel_work_sync(&private->io_work); + cancel_work_sync(&private->crw_work); + list_for_each_entry_safe(crw, temp, &private->crw, next) { list_del(&crw->next); kfree(crw); @@ -202,6 +209,14 @@ static void vfio_ccw_mdev_close_device(struct vfio_device *vdev) container_of(vdev, struct vfio_ccw_private, vdev); vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE); + + /* + * Ensure these work items are drained, in the event the + * device is re-opened instead of released. + */ + cancel_work_sync(&private->io_work); + cancel_work_sync(&private->crw_work); + vfio_ccw_unregister_dev_regions(private); } From 0c11f61a876ed6fcca53d442ed3f33ea8362a0f9 Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:20 +0200 Subject: [PATCH 08/26] s390/vfio_ccw: Move cp cleanup out of not operational The fsm_notoper() routine is called when the device has been lost, and is (by definition) no longer operational. Since this can happen asynchronously from the normal behavior of the driver, the cleanup may happen when holding other locks in the calling sequence (notably, the cio subchannel lock). Push the cleanup of the private->cp resources to a workqueue, where it can be done out from under that lock sequence and a future patch can safely manage the locking requirements. Fixes: 204b394a23ad ("vfio/ccw: Move FSM open/close to MDEV open/close") Cc: stable@vger.kernel.org Signed-off-by: Eric Farman Reviewed-by: Matthew Rosato Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_drv.c | 9 +++++++++ drivers/s390/cio/vfio_ccw_fsm.c | 3 +-- drivers/s390/cio/vfio_ccw_ops.c | 13 +++++++++++++ drivers/s390/cio/vfio_ccw_private.h | 3 +++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 1a095085bc72..c197ad5ab580 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -125,6 +125,15 @@ void vfio_ccw_crw_todo(struct work_struct *work) eventfd_signal(private->crw_trigger); } +void vfio_ccw_notoper_todo(struct work_struct *work) +{ + struct vfio_ccw_private *private; + + private = container_of(work, struct vfio_ccw_private, notoper_work); + + cp_free(&private->cp); +} + /* * Css driver callbacks */ diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index 4d7988ea47ef..4d47a3c7b9a0 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -170,8 +170,7 @@ static void fsm_notoper(struct vfio_ccw_private *private, css_sched_sch_todo(sch, SCH_TODO_UNREG); private->state = VFIO_CCW_STATE_NOT_OPER; - /* This is usually handled during CLOSE event */ - cp_free(&private->cp); + queue_work(vfio_ccw_work_q, &private->notoper_work); } /* diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index d361d1fde3a0..1df6d649565b 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -54,6 +54,7 @@ static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev) INIT_LIST_HEAD(&private->crw); INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo); INIT_WORK(&private->crw_work, vfio_ccw_crw_todo); + INIT_WORK(&private->notoper_work, vfio_ccw_notoper_todo); private->cp.guest_cp = kzalloc_objs(struct ccw1, CCWCHAIN_LEN_MAX); if (!private->cp.guest_cp) @@ -134,9 +135,16 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) /* * Ensure these work items are fully drained, so none can * fire after being released. + * + * notoper_work should have nothing to do here, because only + * open devices could have channel_program resources in use + * and those would be released during close. Nevertheless, + * call flush here as well to be certain anything that was + * allocated is freed. */ cancel_work_sync(&private->io_work); cancel_work_sync(&private->crw_work); + flush_work(&private->notoper_work); list_for_each_entry_safe(crw, temp, &private->crw, next) { list_del(&crw->next); @@ -213,9 +221,14 @@ static void vfio_ccw_mdev_close_device(struct vfio_device *vdev) /* * Ensure these work items are drained, in the event the * device is re-opened instead of released. + * + * notoper_work needs to be given a chance to run if it + * is queued, so any memory associated with the channel + * program can be returned. */ cancel_work_sync(&private->io_work); cancel_work_sync(&private->crw_work); + flush_work(&private->notoper_work); vfio_ccw_unregister_dev_regions(private); } diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 0501d4bbcdbd..e2256402b089 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -102,6 +102,7 @@ struct vfio_ccw_parent { * @req_trigger: eventfd ctx for signaling userspace to return device * @io_work: work for deferral process of I/O handling * @crw_work: work for deferral process of CRW handling + * @notoper_work: work for deferred processing in not-operational state */ struct vfio_ccw_private { struct vfio_device vdev; @@ -125,11 +126,13 @@ struct vfio_ccw_private { struct eventfd_ctx *req_trigger; struct work_struct io_work; struct work_struct crw_work; + struct work_struct notoper_work; } __aligned(8); int vfio_ccw_sch_quiesce(struct subchannel *sch); void vfio_ccw_sch_io_todo(struct work_struct *work); void vfio_ccw_crw_todo(struct work_struct *work); +void vfio_ccw_notoper_todo(struct work_struct *work); extern struct mdev_driver vfio_ccw_mdev_driver; From 34f4feff3e90bd09308fad0974e97113b23b812a Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:21 +0200 Subject: [PATCH 09/26] s390/vfio_ccw: Selectively expand io_mutex The io_mutex was defined to serialize the io_regions, but then has also sort of been associated with the I/O themselves because of the close relationship they share. With the handful of races that are possible, the choices are either to: A) expand the scope of io_mutex to close these remaining windows, or B) reduce the scope of io_mutex to just io_region, and introduce a new lock mechanism for the remaining I/O resources This patch implements A, since B brings with it a lot more interactions that would need to be tracked and kept in a correct hierarchy. It also takes advantage of the workqueue element for cp_free() that now gets called out of fsm_notoper(), which could be invoked out of an interrupt context and thus cannot acquire a mutex itself. Fixes: 4f76617378ee ("vfio-ccw: protect the I/O region") Cc: stable@vger.kernel.org Signed-off-by: Eric Farman Reviewed-by: Matthew Rosato Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_chp.c | 2 +- drivers/s390/cio/vfio_ccw_cp.c | 8 +++++++- drivers/s390/cio/vfio_ccw_drv.c | 6 ++++-- drivers/s390/cio/vfio_ccw_fsm.c | 5 +++++ drivers/s390/cio/vfio_ccw_private.h | 3 ++- 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c index f3015132d4b5..9269b54f5cfd 100644 --- a/drivers/s390/cio/vfio_ccw_chp.c +++ b/drivers/s390/cio/vfio_ccw_chp.c @@ -98,13 +98,13 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, if (pos + count > sizeof(*region)) return -EINVAL; + mutex_lock(&private->io_mutex); crw = list_first_entry_or_null(&private->crw, struct vfio_ccw_crw, next); if (crw) list_del(&crw->next); - mutex_lock(&private->io_mutex); if (i >= private->num_regions) { ret = -EINVAL; goto out; diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 5ef082b8289a..58722c4baa25 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -977,17 +977,23 @@ void cp_update_scsw(struct channel_program *cp, union scsw *scsw) */ bool cp_iova_pinned(struct channel_program *cp, u64 iova, u64 length) { + struct vfio_ccw_private *private = + container_of(cp, struct vfio_ccw_private, cp); struct ccwchain *chain; int i; if (!cp->initialized) return false; + mutex_lock(&private->io_mutex); list_for_each_entry(chain, &cp->ccwchain_list, next) { for (i = 0; i < chain->ch_len; i++) - if (page_array_iova_pinned(&chain->ch_pa[i], iova, length)) + if (page_array_iova_pinned(&chain->ch_pa[i], iova, length)) { + mutex_unlock(&private->io_mutex); return true; + } } + mutex_unlock(&private->io_mutex); return false; } diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index c197ad5ab580..757ff5b2556e 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -91,6 +91,7 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) is_final = !(scsw_actl(&irb->scsw) & (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); + mutex_lock(&private->io_mutex); if (scsw_is_solicited(&irb->scsw)) { cp_update_scsw(&private->cp, &irb->scsw); if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) { @@ -98,9 +99,7 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) cp_is_finished = true; } } - mutex_lock(&private->io_mutex); memcpy(private->io_region->irb_area, irb, sizeof(*irb)); - mutex_unlock(&private->io_mutex); /* * Reset to IDLE only if processing of a channel program @@ -110,6 +109,7 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) */ if (cp_is_finished) private->state = VFIO_CCW_STATE_IDLE; + mutex_unlock(&private->io_mutex); if (private->io_trigger) eventfd_signal(private->io_trigger); @@ -131,7 +131,9 @@ void vfio_ccw_notoper_todo(struct work_struct *work) private = container_of(work, struct vfio_ccw_private, notoper_work); + mutex_lock(&private->io_mutex); cp_free(&private->cp); + mutex_unlock(&private->io_mutex); } /* diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index 4d47a3c7b9a0..5fd94e9d5c61 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -170,6 +170,7 @@ static void fsm_notoper(struct vfio_ccw_private *private, css_sched_sch_todo(sch, SCH_TODO_UNREG); private->state = VFIO_CCW_STATE_NOT_OPER; + /* This routine could be called from IRQ context, so defer */ queue_work(vfio_ccw_work_q, &private->notoper_work); } @@ -409,7 +410,11 @@ static void fsm_close(struct vfio_ccw_private *private, private->state = VFIO_CCW_STATE_STANDBY; spin_unlock_irq(&sch->lock); + + mutex_lock(&private->io_mutex); cp_free(&private->cp); + mutex_unlock(&private->io_mutex); + return; err_unlock: diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index e2256402b089..739121116ab6 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -88,7 +88,8 @@ struct vfio_ccw_parent { * @state: internal state of the device * @completion: synchronization helper of the I/O completion * @io_region: MMIO region to input/output I/O arguments/results - * @io_mutex: protect against concurrent update of I/O regions + * @io_mutex: protect against concurrent update of I/O resources + * and @cp lifecycle * @region: additional regions for other subchannel operations * @cmd_region: MMIO region for asynchronous I/O commands other than START * @schib_region: MMIO region for SCHIB information From 16b0798024c0e9117e395829ddbbe70981c79d9c Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Tue, 28 Jul 2026 05:30:22 +0200 Subject: [PATCH 10/26] s390/vfio_ccw: Implement a crw lock Unlike the channel_program struct, which covers synchronous I/O submissions and asynchronous interrupts, the CRW region relies exclusively on asynchronous events coming from hardware. Implement a lock to manage the list of those payloads, to ensure they are read cohesively. Fixes: 3f02cb2fd9d2 ("vfio-ccw: Wire up the CRW irq and CRW region") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Reviewed-by: Farhan Ali Signed-off-by: Eric Farman Signed-off-by: Christian Borntraeger --- drivers/s390/cio/vfio_ccw_chp.c | 26 ++++++++++++++++---------- drivers/s390/cio/vfio_ccw_drv.c | 6 ++++++ drivers/s390/cio/vfio_ccw_ops.c | 4 ++++ drivers/s390/cio/vfio_ccw_private.h | 4 ++++ 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c index 9269b54f5cfd..7708eb4d6de0 100644 --- a/drivers/s390/cio/vfio_ccw_chp.c +++ b/drivers/s390/cio/vfio_ccw_chp.c @@ -93,18 +93,13 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, loff_t pos = *ppos & VFIO_CCW_OFFSET_MASK; struct ccw_crw_region *region; struct vfio_ccw_crw *crw; + unsigned long flags; int ret; if (pos + count > sizeof(*region)) return -EINVAL; mutex_lock(&private->io_mutex); - crw = list_first_entry_or_null(&private->crw, - struct vfio_ccw_crw, next); - - if (crw) - list_del(&crw->next); - if (i >= private->num_regions) { ret = -EINVAL; goto out; @@ -113,6 +108,16 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, i = array_index_nospec(i, private->num_regions); region = private->region[i].data; + spin_lock_irqsave(&private->crw_lock, flags); + crw = list_first_entry_or_null(&private->crw, + struct vfio_ccw_crw, next); + + if (crw) + list_del(&crw->next); + + /* Drop CRW lock while copying to userspace */ + spin_unlock_irqrestore(&private->crw_lock, flags); + if (crw) memcpy(®ion->crw, &crw->crw, sizeof(region->crw)); @@ -122,15 +127,16 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private, ret = count; region->crw = 0; - -out: - mutex_unlock(&private->io_mutex); - kfree(crw); /* Notify the guest if more CRWs are on our queue */ + spin_lock_irqsave(&private->crw_lock, flags); if (!list_empty(&private->crw) && private->crw_trigger) eventfd_signal(private->crw_trigger); + spin_unlock_irqrestore(&private->crw_lock, flags); + +out: + mutex_unlock(&private->io_mutex); return ret; } diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 757ff5b2556e..ab6b518cc353 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -118,11 +118,14 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) void vfio_ccw_crw_todo(struct work_struct *work) { struct vfio_ccw_private *private; + unsigned long flags; private = container_of(work, struct vfio_ccw_private, crw_work); + spin_lock_irqsave(&private->crw_lock, flags); if (!list_empty(&private->crw) && private->crw_trigger) eventfd_signal(private->crw_trigger); + spin_unlock_irqrestore(&private->crw_lock, flags); } void vfio_ccw_notoper_todo(struct work_struct *work) @@ -286,6 +289,7 @@ static void vfio_ccw_queue_crw(struct vfio_ccw_private *private, unsigned int rsid) { struct vfio_ccw_crw *crw; + unsigned long flags; /* * If unable to allocate a CRW, just drop the event and @@ -303,7 +307,9 @@ static void vfio_ccw_queue_crw(struct vfio_ccw_private *private, crw->crw.erc = erc; crw->crw.rsid = rsid; + spin_lock_irqsave(&private->crw_lock, flags); list_add_tail(&crw->next, &private->crw); + spin_unlock_irqrestore(&private->crw_lock, flags); queue_work(vfio_ccw_work_q, &private->crw_work); } diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 1df6d649565b..5ce91285c7d5 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -55,6 +55,7 @@ static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev) INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo); INIT_WORK(&private->crw_work, vfio_ccw_crw_todo); INIT_WORK(&private->notoper_work, vfio_ccw_notoper_todo); + spin_lock_init(&private->crw_lock); private->cp.guest_cp = kzalloc_objs(struct ccw1, CCWCHAIN_LEN_MAX); if (!private->cp.guest_cp) @@ -131,6 +132,7 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) struct vfio_ccw_private *private = container_of(vdev, struct vfio_ccw_private, vdev); struct vfio_ccw_crw *crw, *temp; + unsigned long flags; /* * Ensure these work items are fully drained, so none can @@ -146,10 +148,12 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) cancel_work_sync(&private->crw_work); flush_work(&private->notoper_work); + spin_lock_irqsave(&private->crw_lock, flags); list_for_each_entry_safe(crw, temp, &private->crw, next) { list_del(&crw->next); kfree(crw); } + spin_unlock_irqrestore(&private->crw_lock, flags); kmem_cache_free(vfio_ccw_crw_region, private->crw_region); kmem_cache_free(vfio_ccw_schib_region, private->schib_region); diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 739121116ab6..3bd0171d38d0 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -98,6 +98,8 @@ struct vfio_ccw_parent { * @cp: channel program for the current I/O operation * @irb: irb info received from interrupt * @scsw: scsw info + * @crw_lock: serialization of CRW list information + * @crw: list of Channel Report Word elements * @io_trigger: eventfd ctx for signaling userspace I/O results * @crw_trigger: eventfd ctx for signaling userspace CRW information * @req_trigger: eventfd ctx for signaling userspace to return device @@ -120,6 +122,8 @@ struct vfio_ccw_private { struct channel_program cp; struct irb irb; union scsw scsw; + + spinlock_t crw_lock; struct list_head crw; struct eventfd_ctx *io_trigger; From 496e0f706b8ad7831bf7189ff9deb8701d2ebc2b Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:28 +0200 Subject: [PATCH 11/26] KVM: s390: Fix unlikely NULL gmap dereference When creating a new vCPU, kvm_vm_ioctl_create_vcpu() will call kvm_arch_vcpu_postcreate() after the file descriptor for the new vCPU has been created. The new file descriptor has not been returned yet, but a malicious userspace program could try to guess it. If a malicious userspace program manages to start the newly created vCPU before kvm_arch_vcpu_postcreate() is called, __vcpu_run() will try to dereference vcpu->arch.gmap and trigger a NULL pointer dereference. Fix this by adding a new field to struct kvm_vcpu_arch to keep track of the initialization status of the vCPU. Refuse to run a vCPU that is not fully initialized. Fixes: dafd032a15f8 ("KVM: s390: move vcpu specific initalization to a later point") Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Reviewed-by: Steffen Eiden Reviewed-by: Janosch Frank Reviewed-by: Christian Borntraeger Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-2-imbrenda@linux.ibm.com> --- arch/s390/include/asm/kvm_host.h | 1 + arch/s390/kvm/kvm-s390.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index c172f9b212d1..b4182ca4435f 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -440,6 +440,7 @@ struct kvm_vcpu_arch { bool skey_enabled; /* Indicator if the access registers have been loaded from guest */ bool acrs_loaded; + bool initialized; struct kvm_s390_pv_vcpu pv; union diag318_info diag318_info; struct kvm_s390_mmu_cache *mc; diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 150b5dd2170e..f86b4b0b356f 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3613,6 +3613,9 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) if (test_kvm_facility(vcpu->kvm, 74) || vcpu->kvm->arch.user_instr0 || vcpu->kvm->arch.user_operexec) vcpu->arch.sie_block->ictl |= ICTL_OPEREXC; + + /* Pairs with smp_load_acquire() in kvm_arch_vcpu_ioctl_run() and kvm_arch_vcpu_ioctl() */ + smp_store_release(&vcpu->arch.initialized, true); } static bool kvm_has_pckmo_subfunc(struct kvm *kvm, unsigned long nr) @@ -5039,6 +5042,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS) return -EINVAL; + /* Pairs with smp_store_release() in kvm_arch_vcpu_postcreate() */ + if (!smp_load_acquire(&vcpu->arch.initialized)) + return -EINVAL; + vcpu_load(vcpu); if (guestdbg_exit_pending(vcpu)) { @@ -5523,6 +5530,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp, long r; u16 rc, rrc; + /* Pairs with smp_store_release() in kvm_arch_vcpu_postcreate() */ + if (!smp_load_acquire(&vcpu->arch.initialized)) + return -EINVAL; + vcpu_load(vcpu); switch (ioctl) { From f8e370058e9e1396fea1a8d11906fa92ec9bad88 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:29 +0200 Subject: [PATCH 12/26] KVM: s390: Do not free SCA if it was not allocated If VM creation fails early in kvm_arch_init_vm(), the cleanup code tries to free up the SCA, even though the address is 0. Due to using free_pages_exact(), only the first page is skipped, accidentally freeing pages 1, 2, and 3. Fix by checking whether the pointer is NULL before attempting to free the SCA in sca_dispose(). Fixes: e72753ed1267 ("KVM: s390: Use ESCA instead of BSCA at VM init") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-3-imbrenda@linux.ibm.com> Reviewed-by: Christian Borntraeger Reviewed-by: Janosch Frank --- arch/s390/kvm/kvm-s390.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f86b4b0b356f..1b3290a5ad1a 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3247,7 +3247,8 @@ static void kvm_s390_crypto_init(struct kvm *kvm) static void sca_dispose(struct kvm *kvm) { - free_pages_exact(kvm->arch.sca, sizeof(*kvm->arch.sca)); + if (kvm->arch.sca) + free_pages_exact(kvm->arch.sca, sizeof(*kvm->arch.sca)); kvm->arch.sca = NULL; } From b050f741fd0d636f2daab72b74aeccea97abdb5f Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:30 +0200 Subject: [PATCH 13/26] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma() In some cases kvm_s390_vcpu_unsetup_cmma() can be called with a 0 cbrlo; in such cases, if running with V != R, free_page() will attempt to free physical page 0. Fix by freeing cbrlo only if it's non-zero. Fixes: b31605c12f4e ("KVM: s390: make cmma usage conditionally") Fixes: 29b40f105ec8 ("KVM: s390: protvirt: Add initial vm and cpu lifecycle handling") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-4-imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 1b3290a5ad1a..9be27db0a21e 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3678,7 +3678,8 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu) void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu) { - free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo)); + if (vcpu->arch.sie_block->cbrlo) + free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo)); vcpu->arch.sie_block->cbrlo = 0; } From a0496b40a4ab346052aaa59a5163c10224dda01f Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:31 +0200 Subject: [PATCH 14/26] KVM: s390: Fix overclearing ESCA in case of error If an attempt is made to create a vCPU with an already existing ID, the duplicated vCPU will be destroyed. When destroying a vCPU, its ESCA entry will be cleared. In the above scenario, the spurious duplicate vCPU is destroyed, but the ESCA entry corresponding to the original vCPU is cleared. Fix by skipping clearing the ESCA entry if the vCPU creation was not successful, i.e. if the vcpu->arch.initialized is still zero. Reviewed-by: Janosch Frank Signed-off-by: Claudio Imbrenda Fixes: abf4a71ed95f ("KVM: s390: Unlink vcpu on destroy - v2") [ Added Fixes tag while picking -- Claudio ] Message-ID: <20260803124040.126471-5-imbrenda@linux.ibm.com> --- arch/s390/kvm/interrupt.c | 2 +- arch/s390/kvm/kvm-s390.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index 7514d9e2403c..bb4e72466043 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -86,7 +86,7 @@ static void sca_clear_ext_call(struct kvm_vcpu *vcpu) struct esca_block *sca = vcpu->kvm->arch.sca; union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl; - if (!kvm_s390_use_sca_entries()) + if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized) return; kvm_s390_clear_cpuflags(vcpu, CPUSTAT_ECALL_PEND); diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 9be27db0a21e..5b2727d7dfd1 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3462,7 +3462,7 @@ static void sca_del_vcpu(struct kvm_vcpu *vcpu) { struct esca_block *sca = vcpu->kvm->arch.sca; - if (!kvm_s390_use_sca_entries()) + if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized) return; clear_bit_inv(vcpu->vcpu_id, (unsigned long *)sca->mcn); From d301ade41831e746783bdb697fae3afba47841ba Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:32 +0200 Subject: [PATCH 15/26] KVM: s390: ucontrol: Fix sca_clear_ext_call() When cleaning up a UCONTROL VM, sca_clear_ext_call() will touch memory outside of the allocated ESCA block, and UCONTROL VMs don't even use ESCA. Fix by not touching ESCA for UCONTROL VMs, and fence the KVM_S390_INTERRUPT ioctl altogether. Add extra checks in sca_ext_call_pending() and sca_inject_ext_call() to make sure UCONTROL VMs won't touch ESCA. Fencing does not cause regressions with userspace, since UCONTROL VMs never used KVM_S390_INTERRUPT ioctls. Fixes: 7d43bafcff17 ("KVM: s390: Make provisions for ESCA utilization") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-6-imbrenda@linux.ibm.com> --- arch/s390/kvm/interrupt.c | 19 ++++++++++++++----- arch/s390/kvm/kvm-s390.c | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index bb4e72466043..8f24bcd1a6d3 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -45,13 +45,16 @@ static struct kvm_s390_gib *gib; static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id) { struct esca_block *sca = vcpu->kvm->arch.sca; - union esca_sigp_ctrl sigp_ctrl = sca->cpu[vcpu->vcpu_id].sigp_ctrl; + union esca_sigp_ctrl sigp_ctrl; if (!kvm_s390_test_cpuflags(vcpu, CPUSTAT_ECALL_PEND)) return 0; + if (kvm_is_ucontrol(vcpu->kvm)) + return 0; BUG_ON(!kvm_s390_use_sca_entries()); + sigp_ctrl = sca->cpu[vcpu->vcpu_id].sigp_ctrl; if (src_id) *src_id = sigp_ctrl.scn; @@ -60,13 +63,16 @@ static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id) static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id) { - struct esca_block *sca = vcpu->kvm->arch.sca; - union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl; union esca_sigp_ctrl old_val, new_val = {.scn = src_id, .c = 1}; + struct esca_block *sca = vcpu->kvm->arch.sca; + union esca_sigp_ctrl *sigp_ctrl; int expect, rc; BUG_ON(!kvm_s390_use_sca_entries()); + if (kvm_is_ucontrol(vcpu->kvm)) + return -EINVAL; + sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl; old_val = READ_ONCE(*sigp_ctrl); old_val.c = 0; @@ -84,10 +90,13 @@ static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id) static void sca_clear_ext_call(struct kvm_vcpu *vcpu) { struct esca_block *sca = vcpu->kvm->arch.sca; - union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl; + union esca_sigp_ctrl *sigp_ctrl; - if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized) + if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized || kvm_is_ucontrol(vcpu->kvm)) return; + + /* Initialize after the above check, to prevent going out of bounds */ + sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl; kvm_s390_clear_cpuflags(vcpu, CPUSTAT_ECALL_PEND); WRITE_ONCE(sigp_ctrl->value, 0); diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 5b2727d7dfd1..21574f57be72 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2934,6 +2934,9 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) case KVM_S390_INTERRUPT: { struct kvm_s390_interrupt s390int; + r = -EINVAL; + if (kvm_is_ucontrol(kvm)) + break; r = -EFAULT; if (copy_from_user(&s390int, argp, sizeof(s390int))) break; @@ -5456,6 +5459,8 @@ long kvm_arch_vcpu_unlocked_ioctl(struct file *filp, unsigned int ioctl, struct kvm_s390_interrupt s390int; struct kvm_s390_irq s390irq = {}; + if (kvm_is_ucontrol(vcpu->kvm)) + return -EINVAL; if (copy_from_user(&s390int, argp, sizeof(s390int))) return -EFAULT; if (s390int_to_s390irq(&s390int, &s390irq)) From d699986f11bf854bddecd6b1bfdf53e05ef945d6 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:33 +0200 Subject: [PATCH 16/26] KVM: s390: Fix leaking of PGM_ADDRESSING to userspace If kvm_s390_set_cmma_bits() is asked to set CMMA values outside of a memslot, PGM_ADDRESSING (5) is returned, instead of a negative error value. Same issue with kvm_s390_{g,s}et_skeys(), kvm_s390_keyop(), and dat_reset_reference_bit(). Fix by returning -EFAULT whenever the return value would be > 0, which is consistent with the behaviour before the gmap rewrite. Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-7-imbrenda@linux.ibm.com> --- arch/s390/kvm/dat.c | 16 ++++++++++------ arch/s390/kvm/dat.h | 2 +- arch/s390/kvm/kvm-s390.c | 16 ++++++++-------- arch/s390/kvm/priv.c | 5 +++-- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c index ed4259d17629..171b61959908 100644 --- a/arch/s390/kvm/dat.c +++ b/arch/s390/kvm/dat.c @@ -755,13 +755,15 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf return rc; } -int dat_reset_reference_bit(union asce asce, gfn_t gfn) +int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey) { union pgste pgste, old; union crste *crstep; union pte *ptep; int rc; + skey->skey = 0; + rc = dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep); if (rc) return rc; @@ -771,21 +773,23 @@ int dat_reset_reference_bit(union asce asce, gfn_t gfn) if (!crste.h.fc || !crste.s.fc1.pr) return 0; - return page_reset_referenced(large_crste_to_phys(*crstep, gfn)); + skey->skey = page_reset_referenced(large_crste_to_phys(*crstep, gfn)) << 1; + return 0; } old = pgste_get_lock(ptep); pgste = old; if (!ptep->h.i) { - rc = page_reset_referenced(pte_origin(*ptep)); - pgste.hr = rc >> 1; + skey->skey = page_reset_referenced(pte_origin(*ptep)) << 1; + pgste.hr = skey->r; } - rc |= (pgste.gr << 1) | pgste.gc; + skey->r |= pgste.gr; + skey->c |= pgste.gc; pgste.gr = 0; dat_update_ptep_sd(old, pgste, ptep); pgste_set_unlock(ptep, pgste); - return rc; + return 0; } static long dat_reset_skeys_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk) diff --git a/arch/s390/kvm/dat.h b/arch/s390/kvm/dat.h index fad605305e05..141ee7b9f019 100644 --- a/arch/s390/kvm/dat.h +++ b/arch/s390/kvm/dat.h @@ -537,7 +537,7 @@ int dat_set_storage_key(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t gf union skey skey, bool nq); int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gfn_t gfn, union skey skey, union skey *oldkey, bool nq, bool mr, bool mc); -int dat_reset_reference_bit(union asce asce, gfn_t gfn); +int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey); long dat_reset_skeys(union asce asce, gfn_t start); unsigned long dat_get_ptval(struct page_table *table, struct ptval_param param); diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 21574f57be72..e162efaa35b8 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -571,7 +571,7 @@ static int kvm_s390_keyop(struct kvm_s390_mmu_cache *mc, struct kvm *kvm, int op switch (op) { case KVM_S390_KEYOP_SSKE: r = dat_cond_set_storage_key(mc, asce, gfn, skey, &skey, 0, 0, 0); - if (r >= 0) + if (r == 0 || r == 1) return skey.skey; break; case KVM_S390_KEYOP_ISKE: @@ -580,14 +580,14 @@ static int kvm_s390_keyop(struct kvm_s390_mmu_cache *mc, struct kvm *kvm, int op return skey.skey; break; case KVM_S390_KEYOP_RRBE: - r = dat_reset_reference_bit(asce, gfn); - if (r > 0) - return r << 1; + r = dat_reset_reference_bit(asce, gfn, &skey); + if (!r) + return skey.skey; break; default: return -EINVAL; } - return r; + return r > 0 ? -EFAULT : r; } /* Section: device related */ @@ -2214,7 +2214,7 @@ static int kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args) } kvfree(keys); - return r; + return r <= 0 ? r : -EFAULT; } static int kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args) @@ -2276,7 +2276,7 @@ static int kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args) kvm_s390_free_mmu_cache(mc); out: kvfree(keys); - return r; + return r <= 0 ? r : -EFAULT; } /* @@ -2386,7 +2386,7 @@ static int kvm_s390_set_cmma_bits(struct kvm *kvm, set_bit(GMAP_FLAG_USES_CMM, &kvm->arch.gmap->flags); - return r; + return r <= 0 ? r : -EFAULT; } /** diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c index ad0ddc433a73..ea5a99537346 100644 --- a/arch/s390/kvm/priv.c +++ b/arch/s390/kvm/priv.c @@ -289,6 +289,7 @@ static int handle_iske(struct kvm_vcpu *vcpu) static int handle_rrbe(struct kvm_vcpu *vcpu) { unsigned long gaddr; + union skey skey; int reg1, reg2; int rc; @@ -307,12 +308,12 @@ static int handle_rrbe(struct kvm_vcpu *vcpu) gaddr = kvm_s390_logical_to_effective(vcpu, gaddr); gaddr = kvm_s390_real_to_abs(vcpu, gaddr); scoped_guard(read_lock, &vcpu->kvm->mmu_lock) - rc = dat_reset_reference_bit(vcpu->arch.gmap->asce, gpa_to_gfn(gaddr)); + rc = dat_reset_reference_bit(vcpu->arch.gmap->asce, gpa_to_gfn(gaddr), &skey); if (rc > 0) return kvm_s390_inject_program_int(vcpu, rc); if (rc < 0) return rc; - kvm_s390_set_psw_cc(vcpu, rc); + kvm_s390_set_psw_cc(vcpu, (skey.skey >> 1) & 3); return 0; } From 4db7207052874f13dec1e9be384cca279c6693e8 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:34 +0200 Subject: [PATCH 17/26] KVM: s390: Fix race in __do_essa() An unlikely race between __do_essa() and kvm_s390_vm_start_migration(), kvm_s390_vm_stop_migration(), or dat_get_cmma() was possible. Fix by locking kvm->slots_arch_lock. Since this is not a hot path, the overhead of an additional mutex is negligible. Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-8-imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 20 ++++++++++---------- arch/s390/kvm/priv.c | 5 +++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index e162efaa35b8..e5c5e9f61cb2 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1219,8 +1219,8 @@ static void kvm_s390_sync_request_broadcast(struct kvm *kvm, int req) /* * Must be called with kvm->srcu held to avoid races on memslots, and with - * kvm->slots_lock to avoid races with ourselves, kvm_s390_vm_stop_migration(), - * and kvm_s390_get_cmma_bits(). + * kvm->slots_arch_lock to avoid races with ourselves, + * kvm_s390_vm_stop_migration(), and kvm_s390_get_cmma_bits(). */ static int kvm_s390_vm_start_migration(struct kvm *kvm) { @@ -1265,7 +1265,7 @@ static int kvm_s390_vm_start_migration(struct kvm *kvm) } /* - * Must be called with kvm->slots_lock to avoid races with ourselves, + * Must be called with kvm->slots_arch_lock to avoid races with ourselves, * kvm_s390_vm_start_migration() and kvm_s390_get_cmma_bits(). */ static int kvm_s390_vm_stop_migration(struct kvm *kvm) @@ -1300,7 +1300,9 @@ static int kvm_s390_vm_set_migration(struct kvm *kvm, { int res = -ENXIO; - mutex_lock(&kvm->slots_lock); + guard(srcu)(&kvm->srcu); + guard(mutex)(&kvm->slots_arch_lock); + switch (attr->attr) { case KVM_S390_VM_MIGRATION_START: res = kvm_s390_vm_start_migration(kvm); @@ -1311,7 +1313,6 @@ static int kvm_s390_vm_set_migration(struct kvm *kvm, default: break; } - mutex_unlock(&kvm->slots_lock); return res; } @@ -3001,9 +3002,8 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) r = -EFAULT; if (copy_from_user(&args, argp, sizeof(args))) break; - mutex_lock(&kvm->slots_lock); - r = kvm_s390_get_cmma_bits(kvm, &args); - mutex_unlock(&kvm->slots_lock); + scoped_guard(mutex, &kvm->slots_arch_lock) + r = kvm_s390_get_cmma_bits(kvm, &args); if (!r) { r = copy_to_user(argp, &args, sizeof(args)); if (r) @@ -3017,9 +3017,9 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) r = -EFAULT; if (copy_from_user(&args, argp, sizeof(args))) break; - mutex_lock(&kvm->slots_lock); + mutex_lock(&kvm->slots_arch_lock); r = kvm_s390_set_cmma_bits(kvm, &args); - mutex_unlock(&kvm->slots_lock); + mutex_unlock(&kvm->slots_arch_lock); break; } case KVM_S390_PV_COMMAND: { diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c index ea5a99537346..b1ba24c346ef 100644 --- a/arch/s390/kvm/priv.c +++ b/arch/s390/kvm/priv.c @@ -1261,8 +1261,9 @@ static int handle_essa(struct kvm_vcpu *vcpu) /* Retry the ESSA instruction */ kvm_s390_retry_instr(vcpu); } else { - scoped_guard(read_lock, &vcpu->kvm->mmu_lock) - i = __do_essa(vcpu, orc); + scoped_guard(mutex, &vcpu->kvm->slots_arch_lock) + scoped_guard(read_lock, &vcpu->kvm->mmu_lock) + i = __do_essa(vcpu, orc); if (i < 0) return i; /* Account for the possible extra cbrl entry */ From ec215346270512db60478e45e8dff2de950225aa Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:35 +0200 Subject: [PATCH 18/26] KVM: s390: cmma: Fix dirty tracking when removing memslot When a memslot is removed, all ptes that mapped the slot are cleared or even deallocated. If this happens while the system is in migration mode, and if cmma-dirty pages are removed, the cmma-dirty counter will not reflect reality. Fix by appropriately decrementing the cmma-dirty counter when removing a memslot. Opportunistically improve kvm_arch_commit_memory_region() to use __free() for the struct kvm_s390_mmu_cache. Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-9-imbrenda@linux.ibm.com> --- arch/s390/kvm/dat.c | 7 ++++++- arch/s390/kvm/kvm-s390.c | 25 +++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c index 171b61959908..3f2d6e8902d7 100644 --- a/arch/s390/kvm/dat.c +++ b/arch/s390/kvm/dat.c @@ -850,6 +850,7 @@ static long _dat_slot_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_wal struct slot_priv *p = walk->priv; union crste dummy = { .val = p->token }; union pte new_pte, pte = READ_ONCE(*ptep); + union pgste pgste; new_pte = _PTE_TOK(dummy.tok.type, dummy.tok.par); @@ -857,7 +858,11 @@ static long _dat_slot_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_wal if (pte.val == new_pte.val) return 0; - dat_ptep_xchg(ptep, new_pte, gfn, walk->asce, false); + pgste = pgste_get_lock(ptep); + pgste = __dat_ptep_xchg(ptep, pgste, new_pte, gfn, walk->asce, false); + pgste.cmma_d = 0; + pgste_set_unlock(ptep, pgste); + return 0; } diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index e5c5e9f61cb2..ba811f0673d1 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -5812,14 +5812,30 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, return 0; } +static long cmma_d_count_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk) +{ + union pgste pgste; + + pgste = pgste_get_lock(ptep); + if (pgste.cmma_d) { + pgste.cmma_d = 0; + atomic64_dec(walk->priv); + } + pgste_set_unlock(ptep, pgste); + return 0; +} + void kvm_arch_commit_memory_region(struct kvm *kvm, struct kvm_memory_slot *old, const struct kvm_memory_slot *new, enum kvm_mr_change change) { - struct kvm_s390_mmu_cache *mc = NULL; + const struct dat_walk_ops ops = { .pte_entry = cmma_d_count_pte, }; + struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL; int rc = 0; + guard(mutex)(&kvm->slots_arch_lock); + if (change == KVM_MR_FLAGS_ONLY) return; @@ -5830,6 +5846,12 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, } scoped_guard(write_lock, &kvm->mmu_lock) { + if (kvm->arch.migration_mode && kvm->arch.use_cmma && old) { + _dat_walk_gfn_range(old->base_gfn, old->base_gfn + old->npages, + kvm->arch.gmap->asce, &ops, DAT_WALK_IGN_HOLES, + &kvm->arch.cmma_dirty_pages); + } + switch (change) { case KVM_MR_DELETE: rc = dat_delete_slot(mc, kvm->arch.gmap->asce, old->base_gfn, old->npages); @@ -5851,7 +5873,6 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, out: if (rc) pr_warn("failed to commit memory region\n"); - kvm_s390_free_mmu_cache(mc); return; } From dab62d218754e00a22aff45d2b0116c5ee30cbd2 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:36 +0200 Subject: [PATCH 19/26] KVM: s390: ucontrol: Add missing locking around gmap_remove_child() gmap_remove_child() needs to be called while holding the children_lock of the parent gmap. This was not the case in the error handling path of kvm_arch_vcpu_create() for UCONTROL guests. Fix by adding the missing lock. Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Reviewed-by: Steffen Eiden Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-10-imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index ba811f0673d1..2741ca323ede 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3875,7 +3875,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) out_ucontrol_uninit: if (kvm_is_ucontrol(vcpu->kvm)) { + spin_lock(&vcpu->kvm->arch.gmap->children_lock); gmap_remove_child(vcpu->arch.gmap); + spin_unlock(&vcpu->kvm->arch.gmap->children_lock); vcpu->arch.gmap = gmap_put(vcpu->arch.gmap); } out_free_sie_block: From e4d678900a1ae66112812fe4d2aff16044ca37f2 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:37 +0200 Subject: [PATCH 20/26] KVM: s390: Free the mmu cache when kvm_arch_vcpu_create() fails The mmu cache is the first thing that is allocated in kvm_arch_vcpu_create(), but in case of failure it was not freed. Fix by freeing the mmu cache in case of failure. Refactor kvm_arch_vcpu_create() to use scope-based cleanup instead of gotos. Fixes: e38c884df921 ("KVM: s390: Switch to new gmap") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-11-imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 2741ca323ede..9b8a35e7dc82 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3800,21 +3800,21 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id) return 0; } +DEFINE_FREE(sie_page, struct sie_page *, if (_T) free_page((unsigned long)(_T))) + int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) { - struct sie_page *sie_page; + struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL; + struct sie_page *sie_page __free(sie_page) = NULL; int rc; BUILD_BUG_ON(sizeof(struct sie_page) != 4096); - vcpu->arch.mc = kvm_s390_new_mmu_cache(); - if (!vcpu->arch.mc) + mc = kvm_s390_new_mmu_cache(); + if (!mc) return -ENOMEM; sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL_ACCOUNT); - if (!sie_page) { - kvm_s390_free_mmu_cache(vcpu->arch.mc); - vcpu->arch.mc = NULL; + if (!sie_page) return -ENOMEM; - } vcpu->arch.sie_block = &sie_page->sie_block; vcpu->arch.sie_block->itdba = virt_to_phys(&sie_page->itdb); @@ -3856,10 +3856,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) vcpu->run->kvm_valid_regs |= KVM_SYNC_FPRS; if (kvm_is_ucontrol(vcpu->kvm)) { - rc = -ENOMEM; vcpu->arch.gmap = gmap_new_child(vcpu->kvm->arch.gmap, -1UL); if (!vcpu->arch.gmap) - goto out_free_sie_block; + return -ENOMEM; } VM_EVENT(vcpu->kvm, 3, "create cpu %d at 0x%p, sie block at 0x%p", @@ -3867,22 +3866,19 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) trace_kvm_s390_create_vcpu(vcpu->vcpu_id, vcpu, vcpu->arch.sie_block); rc = kvm_s390_vcpu_setup(vcpu); - if (rc) - goto out_ucontrol_uninit; + if (rc) { + if (kvm_is_ucontrol(vcpu->kvm)) { + scoped_guard(spinlock, &vcpu->kvm->arch.gmap->children_lock) + gmap_remove_child(vcpu->arch.gmap); + vcpu->arch.gmap = gmap_put(vcpu->arch.gmap); + } + return rc; + } + vcpu->arch.mc = no_free_ptr(mc); + sie_page = NULL; kvm_s390_update_topology_change_report(vcpu->kvm, 1); return 0; - -out_ucontrol_uninit: - if (kvm_is_ucontrol(vcpu->kvm)) { - spin_lock(&vcpu->kvm->arch.gmap->children_lock); - gmap_remove_child(vcpu->arch.gmap); - spin_unlock(&vcpu->kvm->arch.gmap->children_lock); - vcpu->arch.gmap = gmap_put(vcpu->arch.gmap); - } -out_free_sie_block: - free_page((unsigned long)(vcpu->arch.sie_block)); - return rc; } int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) From 216c5289dd66d9cde6b42cbe68ee8dec5669678a Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:38 +0200 Subject: [PATCH 21/26] KVM: s390: Return -EINTR if a signal is pending while faulting-in If a fatal signal is pending while trying to fault-in a page, return -EINTR instead of -EAGAIN. Also fix unpack_one() to handle -EINTR properly. Fixes: e907ae530133 ("KVM: s390: Add helper functions for fault handling") Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-12-imbrenda@linux.ibm.com> --- arch/s390/kvm/faultin.c | 6 +++--- arch/s390/kvm/pv.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/s390/kvm/faultin.c b/arch/s390/kvm/faultin.c index fee80047bd94..3cc45f7f5b2d 100644 --- a/arch/s390/kvm/faultin.c +++ b/arch/s390/kvm/faultin.c @@ -91,9 +91,9 @@ int kvm_s390_faultin_gfn(struct kvm_vcpu *vcpu, struct kvm *kvm, struct guest_fa /* Access outside memory, addressing exception. */ if (is_noslot_pfn(f->pfn)) return PGM_ADDRESSING; - /* Signal pending: try again. */ - if (f->pfn == KVM_PFN_ERR_SIGPENDING) - return -EAGAIN; + /* Fatal signal pending: bail out. */ + if (is_sigpending_pfn(f->pfn)) + return -EINTR; /* Check if it's read-only memory; don't try to actually handle that case. */ if (f->pfn == KVM_PFN_ERR_RO_FAULT) return -EOPNOTSUPP; diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index 1beacc841ca8..dc204b521052 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -809,7 +809,7 @@ static int unpack_one(struct kvm *kvm, unsigned long addr, u64 tweak, return -EAGAIN; } - if (ret && ret != -EAGAIN) + if (ret && ret != -EAGAIN && ret != -EINTR) KVM_UV_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx with rc %x rrc %x", uvcb.gaddr, *rc, *rrc); return ret; From 9187a9186d0ac7b260b07917aabc672a80d37c3c Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:39 +0200 Subject: [PATCH 22/26] KVM: s390: Fix ordering when adding to SCA When adding a new vCPU to the SCA area, the validity bit in the MCN was set before the pointer to the state description, potentially allowing for a race. Fix by setting the pointer before setting the bit. Fixes: 14542a0a54c5 ("KVM: S390: Remove sca_lock") Reviewed-by: Steffen Eiden Reviewed-by: Janosch Frank Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-13-imbrenda@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 9b8a35e7dc82..518a69c55e85 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -3485,8 +3485,8 @@ static void sca_add_vcpu(struct kvm_vcpu *vcpu) if (!kvm_s390_use_sca_entries()) return; + WRITE_ONCE(sca->cpu[vcpu->vcpu_id].sda, virt_to_phys(vcpu->arch.sie_block)); set_bit_inv(vcpu->vcpu_id, (unsigned long *)sca->mcn); - sca->cpu[vcpu->vcpu_id].sda = virt_to_phys(vcpu->arch.sie_block); } static int sca_can_add_vcpu(struct kvm *kvm, unsigned int id) From feadc5e84dcb53422a437556c35af9efd9826fd5 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Mon, 3 Aug 2026 14:40:40 +0200 Subject: [PATCH 23/26] KVM: s390: Fix cleanup in kvm_s390_pv_create_cpu() If creating a protected vCPU in kvm_s390_pv_create_cpu() fails, kvm_s390_pv_destroy_cpu() was called, which checks whether the vCPU has a PV handle and exits doing nothing otherwise. At that point, due to not having created the protected vCPU, the PV handle will not be set, and kvm_s390_pv_destroy_cpu() will do nothing, thus leaking the allocated memory. Fix by factoring out the code to free and reset a PV vCPU; call it from kvm_s390_pv_destroy_cpu() and kvm_s390_pv_create_cpu(). Opportunistically fix the return value of kvm_s390_pv_destroy_cpu() in case of errors: return -EIO instead if EIO. Fixes: d4074324b07a ("KVM: s390: pv: avoid double free of sida page") Reviewed-by: Steffen Eiden Reviewed-by: Janosch Frank Signed-off-by: Claudio Imbrenda Message-ID: <20260803124040.126471-14-imbrenda@linux.ibm.com> --- arch/s390/kvm/pv.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index dc204b521052..b02e0159d3cd 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -244,6 +244,24 @@ static void kvm_s390_clear_pv_state(struct kvm *kvm) kvm->arch.pv.stor_var = NULL; } +static void kvm_s390_pv_dispose_cpu(struct kvm_vcpu *vcpu, bool free_stor_base) +{ + if (free_stor_base) + free_pages(vcpu->arch.pv.stor_base, get_order(uv_info.guest_cpu_stor_len)); + free_page((unsigned long)sida_addr(vcpu->arch.sie_block)); + vcpu->arch.sie_block->pv_handle_cpu = 0; + vcpu->arch.sie_block->pv_handle_config = 0; + memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv)); + vcpu->arch.sie_block->sdf = 0; + /* + * The sidad field (for sdf == 2) is now the gbea field (for sdf == 0). + * Use the reset value of gbea to avoid leaking the kernel pointer of + * the just freed sida. + */ + vcpu->arch.sie_block->gbea = 1; + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); +} + int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) { int cc; @@ -258,24 +276,9 @@ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) WARN_ONCE(cc, "protvirt destroy cpu failed rc %x rrc %x", *rc, *rrc); /* Intended memory leak for something that should never happen. */ - if (!cc) - free_pages(vcpu->arch.pv.stor_base, - get_order(uv_info.guest_cpu_stor_len)); + kvm_s390_pv_dispose_cpu(vcpu, !cc); - free_page((unsigned long)sida_addr(vcpu->arch.sie_block)); - vcpu->arch.sie_block->pv_handle_cpu = 0; - vcpu->arch.sie_block->pv_handle_config = 0; - memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv)); - vcpu->arch.sie_block->sdf = 0; - /* - * The sidad field (for sdf == 2) is now the gbea field (for sdf == 0). - * Use the reset value of gbea to avoid leaking the kernel pointer of - * the just freed sida. - */ - vcpu->arch.sie_block->gbea = 1; - kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); - - return cc ? EIO : 0; + return cc ? -EIO : 0; } int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) @@ -319,9 +322,7 @@ int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) uvcb.header.rrc); if (cc) { - u16 dummy; - - kvm_s390_pv_destroy_cpu(vcpu, &dummy, &dummy); + kvm_s390_pv_dispose_cpu(vcpu, true); return -EIO; } From 26505e1b5b546e2fa9a0296b951ca158460c72d8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 23 Jul 2026 10:15:22 +0200 Subject: [PATCH 24/26] KVM: SVM: make svm_flush_tlb_gva do a full asid flush if NPT enabled Red Hat is seeing multiple reports of Windows memory corruptions (and consequent BSODs) with hv-tlbflush=on, on AMD processors only. The crashes, while extremely rare, happen even with a stock configuration, but with Driver Verifier enabled they can be detected after approximately 200 VM hours. In particular, Alexander Lougovski measured the following: - on AMD Turin, 15 crashes in 3300 VM hours - on AMD Milan, 2 crashes in 500 VM hours (there are fewer hours here due to the host being smaller) - on Intel Sapphire Rapids, 0 crashes in 8000 VM hours - on AMD Turin with full TLB flush (not exactly this patch but similar), no crashes in ~2 weeks of run time which should also be ~7000 VM hours For Turin, the microcode version was 0x0b002162, which (assuming this is the same issue) should not be affected by the problem listed in https://knowledge.broadcom.com/external/article/419026/bsod-on-virtual-machines-running-on-amd.html; on the other hand that problem should not apply to earlier processors. AMD has not provided any information or analysis yet, and when we asked we didn't know yet that it reproduced on Milan as well. As to the workload, Alexander threw more or less everything at the same time at the VM: - a full Windows Defender scan every 30 minutes - a disk I/O job - a loop doing repeated mmap of system files (mostly to hope that it triggers some consistency check in the Windows memory manager) - SQL Express 2022 + StressDB (1.6M rows), with the host doing queries (75% write/25% read) via sqlcmd Driver Verifier is able to detect BSODs more or less at the same time as the pages are freed. They mostly happen in the Windows Defender filter driver, but occasionally also in the networking stack (e.g., afd.sys) or elsewhere in the filesystem stack (e.g., fltmgr.sys). The flush is issued from kvm_hv_vcpu_flush_tlb(), which receives the cross-CPU requests from the Hyper-V TLB flush hypercalls via a kfifo and is invoked by the KVM_REQ_HV_TLB_FLUSH request. The mechanism is the same for both Intel and AMD, and the handler for both vendors is a simple INVVPID(ADDR)/INVLPGA instruction. Because the request is handled on the destination CPU, there is a question of what happens if the VM is migrated across physical CPUs. In that case, the INVLPGA instruction would use a stale svm->vmcb->control.asid; but if anything that might do an *unnecessary* flush (on an asid that's being used for another VM) and then pre_svm_run() would force a full TLB rebuild. So, for lack of better ideas, this patch forces a full ASID bump in svm_flush_tlb_gva(). To avoid paying the price on Intel and also to avoid unnecessary loops on AMD, the flush_tlb_gva op now returns whether it did a full flush or not; kvm_hv_vcpu_flush_tlb() takes note and exits its loops immediately. While there is an obvious performance impact, about half of the benefit from Hyper-V tlbflush is preserved (10% vs. 20% on the SQL Server workload). kvm_mmu_invalidate_addr() is the only other caller of the flush_tlb_gva op. The change would have a performance impact on every intercepted INVLPG and, for nested SVM, on every L1 INVLPGA. For INVLPGA specifically, this covers the same suspected issue but for nested hypervisors, so it is correct to apply the workaround; for INVLPG on shadow paging, instead, the impact would be stronger and, due to lack of data, for now the use of INVLPGA is left in place in svm_flush_tlb_gva(). Analyzed-by: Vitaly Kuznetsov Analyzed-by: Alexander Lougovski Signed-off-by: Paolo Bonzini --- arch/x86/include/asm/kvm_host.h | 2 +- arch/x86/kvm/hyperv.c | 7 ++++--- arch/x86/kvm/mmu/mmu.c | 2 +- arch/x86/kvm/svm/svm.c | 27 ++++++++++++++++++++------- arch/x86/kvm/vmx/main.c | 4 ++-- arch/x86/kvm/vmx/vmx.c | 2 +- arch/x86/kvm/vmx/x86_ops.h | 2 +- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 5f6c1ce9673b..6db5b5f79df9 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1858,7 +1858,7 @@ struct kvm_x86_ops { * Can potentially get non-canonical addresses through INVLPGs, which * the implementation may choose to ignore if appropriate. */ - void (*flush_tlb_gva)(struct kvm_vcpu *vcpu, gva_t addr); + void (*flush_tlb_gva)(struct kvm_vcpu *vcpu, gva_t addr, bool *full); /* * Flush any TLB entries created by the guest. Like tlb_flush_gva(), diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index fd4eb1e561f7..d8736b5dfcd3 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -1974,6 +1974,7 @@ int kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu) u64 entries[KVM_HV_TLB_FLUSH_FIFO_SIZE]; int i, j, count; gva_t gva; + bool full = false; if (!tdp_enabled || !hv_vcpu) return -EINVAL; @@ -1982,7 +1983,7 @@ int kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu) count = kfifo_out(&tlb_flush_fifo->entries, entries, KVM_HV_TLB_FLUSH_FIFO_SIZE); - for (i = 0; i < count; i++) { + for (i = 0; i < count && !full; i++) { if (entries[i] == KVM_HV_TLB_FLUSHALL_ENTRY) goto out_flush_all; @@ -1991,11 +1992,11 @@ int kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu) * pages to flush. */ gva = entries[i] & PAGE_MASK; - for (j = 0; j < (entries[i] & ~PAGE_MASK) + 1; j++) { + for (j = 0; j < (entries[i] & ~PAGE_MASK) + 1 && !full; j++) { if (is_noncanonical_invlpg_address(gva + j * PAGE_SIZE, vcpu)) continue; - kvm_x86_call(flush_tlb_gva)(vcpu, gva + j * PAGE_SIZE); + kvm_x86_call(flush_tlb_gva)(vcpu, gva + j * PAGE_SIZE, &full); } ++vcpu->stat.tlb_flush; diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 66e69d2a41b3..c9e4739b26d7 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6652,7 +6652,7 @@ void kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, if (is_noncanonical_invlpg_address(addr, vcpu)) return; - kvm_x86_call(flush_tlb_gva)(vcpu, addr); + kvm_x86_call(flush_tlb_gva)(vcpu, addr, NULL); } if (!mmu->sync_spte) diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index d0971685034b..60495447f0ca 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4227,13 +4227,6 @@ static void svm_flush_tlb_all(struct kvm_vcpu *vcpu) svm_flush_tlb_asid(vcpu); } -static void svm_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t gva) -{ - struct vcpu_svm *svm = to_svm(vcpu); - - invlpga(gva, svm->vmcb->control.asid); -} - static void svm_flush_tlb_guest(struct kvm_vcpu *vcpu) { kvm_register_mark_dirty(vcpu, VCPU_REG_ERAPS); @@ -4241,6 +4234,26 @@ static void svm_flush_tlb_guest(struct kvm_vcpu *vcpu) svm_flush_tlb_asid(vcpu); } +static void svm_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t gva, bool *full) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + /* + * INVLPGA has had errata on Genoa and Turin, and even on older + * generations there were reports of Windows BSODs if INVLPGA + * was used for Hyper-V tlbflush. Use it only for shadow paging + * where it seems to be okay. + */ + if (!npt_enabled) { + invlpga(gva, svm->vmcb->control.asid); + return; + } + + svm_flush_tlb_guest(vcpu); + if (full) + *full = true; +} + static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 83d9921277ea..f204a0fc0a57 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -535,12 +535,12 @@ static void vt_flush_tlb_current(struct kvm_vcpu *vcpu) vmx_flush_tlb_current(vcpu); } -static void vt_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr) +static void vt_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr, bool *full) { if (is_td_vcpu(vcpu)) return; - vmx_flush_tlb_gva(vcpu, addr); + vmx_flush_tlb_gva(vcpu, addr, full); } static void vt_flush_tlb_guest(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index cc75feec05da..b8d745f6fd22 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -3361,7 +3361,7 @@ void vmx_flush_tlb_current(struct kvm_vcpu *vcpu) vpid_sync_context(vmx_get_current_vpid(vcpu)); } -void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr) +void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr, bool *full) { /* * vpid_sync_vcpu_addr() is a nop if vpid==0, see the comment in diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 409858074246..17595d52985c 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -82,7 +82,7 @@ void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags); bool vmx_get_if_flag(struct kvm_vcpu *vcpu); void vmx_flush_tlb_all(struct kvm_vcpu *vcpu); void vmx_flush_tlb_current(struct kvm_vcpu *vcpu); -void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr); +void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr, bool *full); void vmx_flush_tlb_guest(struct kvm_vcpu *vcpu); void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask); u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu); From 1d78d33275ef2a16c6d080910b291d0a97a0e613 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Aug 2026 16:44:17 +0200 Subject: [PATCH 25/26] KVM: SVM: Serialize accesses to the owner and mirror list with separate lock Interaction between KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM and KVM_CAP_VM_COPY_ENC_CONTEXT_FROM can cause two separate issues: - in sev_migrate_from(), when the destination KVM is a mirror, the mirror entry is moved from the source's list to the owner's mirror_vms list, without holding the owner's lock unlike other writers of the owner's mirror list (sev_vm_copy_enc_context_from(), sev_vm_destroy()). A concurrent COPY or destroy can race with sev_migrate_from() and corrupt the list. - In sev_vm_destroy(), the *owner* is still active and could receive concurrently a KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM that causes sev->enc_context_owner to change. In this case the incorrect VM receives kvm_put_kvm(). The second issue needs particular care because the owner could disappear altogether (even though the race window is impossibly small) between reading it and locking it. There is thus no way to perform the checks under the owner lock without putting struct kvm under SLAB_TYPESAFE_BY_RCU (which would allow kvm_get_kvm_safe() under RCU critical section). It is much simpler to just use a global lock, since the critical sections are so small and the new lock is always a leaf lock. Fixes: b2125513dfc0 ("KVM: SEV: Allow SEV intra-host migration of VM with mirrors") Cc: stable@vger.kernel.org Reported-by: Shen Yongchao Link: https://lore.kernel.org/kvm/tencent_625C0F42824E542C72B34733392AF2C49709@qq.com/ Link: https://lore.kernel.org/kvm/tencent_DDC4E4352EC91CAC05A9A8F4E55E8C96730A@qq.com/ Signed-off-by: Paolo Bonzini --- arch/x86/kvm/svm/sev.c | 34 +++++++++++++++++++++++++--------- arch/x86/kvm/svm/svm.h | 1 + 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 944aaea6501f..0f0ea7896af5 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -97,6 +97,8 @@ static u64 sev_supported_vmsa_features __ro_after_init; static u8 sev_enc_bit; static DECLARE_RWSEM(sev_deactivate_lock); static DEFINE_MUTEX(sev_bitmap_lock); +/* Protects kvm_sev_info's enc_context_owner, mirror_vms and mirror_entry. */ +static DEFINE_MUTEX(sev_mirror_lock); unsigned int max_sev_asid; static unsigned int min_sev_asid; static unsigned int max_sev_es_asid; @@ -2018,7 +2020,6 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm) dst->asid = src->asid; dst->handle = src->handle; dst->pages_locked = src->pages_locked; - dst->enc_context_owner = src->enc_context_owner; dst->es_active = src->es_active; dst->vmsa_features = src->vmsa_features; @@ -2026,11 +2027,12 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm) src->active = false; src->handle = 0; src->pages_locked = 0; - src->enc_context_owner = NULL; src->es_active = false; list_cut_before(&dst->regions_list, &src->regions_list, &src->regions_list); + mutex_lock(&sev_mirror_lock); + /* * If this VM has mirrors, "transfer" each mirror's refcount of the * source to the destination (this KVM). The caller holds a reference @@ -2047,12 +2049,15 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm) * If this VM is a mirror, remove the old mirror from the owners list * and add the new mirror to the list. */ - if (is_mirroring_enc_context(dst_kvm)) { - struct kvm_sev_info *owner_sev_info = to_kvm_sev_info(dst->enc_context_owner); + if (is_mirroring_enc_context(src_kvm)) { + struct kvm_sev_info *owner_sev_info = to_kvm_sev_info(src->enc_context_owner); + dst->enc_context_owner = src->enc_context_owner; + src->enc_context_owner = NULL; list_del(&src->mirror_entry); list_add_tail(&dst->mirror_entry, &owner_sev_info->mirror_vms); } + mutex_unlock(&sev_mirror_lock); kvm_for_each_vcpu(i, dst_vcpu, dst_kvm) { dst_svm = to_svm(dst_vcpu); @@ -2871,11 +2876,14 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd) * disappear until we're done with it */ source_sev = to_kvm_sev_info(source_kvm); - kvm_get_kvm(source_kvm); - list_add_tail(&mirror_sev->mirror_entry, &source_sev->mirror_vms); /* Set enc_context_owner and copy its encryption context over */ + mutex_lock(&sev_mirror_lock); + kvm_get_kvm(source_kvm); + list_add_tail(&mirror_sev->mirror_entry, &source_sev->mirror_vms); mirror_sev->enc_context_owner = source_kvm; + mutex_unlock(&sev_mirror_lock); + mirror_sev->active = true; mirror_sev->asid = source_sev->asid; mirror_sev->fd = source_sev->fd; @@ -2963,11 +2971,19 @@ void sev_vm_destroy(struct kvm *kvm) * Note, mirror VMs don't support registering encrypted regions. */ if (is_mirroring_enc_context(kvm)) { - struct kvm *owner_kvm = sev->enc_context_owner; + struct kvm *owner_kvm; - mutex_lock(&owner_kvm->lock); + mutex_lock(&sev_mirror_lock); + owner_kvm = sev->enc_context_owner; list_del(&sev->mirror_entry); - mutex_unlock(&owner_kvm->lock); + sev->enc_context_owner = NULL; + + /* + * The reference to owner_kvm cannot move after sev_mirror_lock is + * released. Release it before kvm_put_kvm() so that owner_kvm is + * never destroyed inside sev_mirror_lock. + */ + mutex_unlock(&sev_mirror_lock); kvm_put_kvm(owner_kvm); return; } diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 716be21fba33..d63e5878988a 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -109,6 +109,7 @@ struct kvm_sev_info { u64 ap_jump_table; /* SEV-ES AP Jump Table address */ u64 vmsa_features; u16 ghcb_version; /* Highest guest GHCB protocol version allowed */ + /* The three fields below are protected by sev_mirror_lock */ struct kvm *enc_context_owner; /* Owner of copied encryption context */ struct list_head mirror_vms; /* List of VMs mirroring */ struct list_head mirror_entry; /* Use as a list entry of mirrors */ From 5ec42d57655c690234c14aece6dd3f209778c1d8 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Mon, 13 Jul 2026 08:25:49 -0700 Subject: [PATCH 26/26] KVM: x86/mmu: WARN and clear role.invalid when creating a child shadow page Explicitly clear role.invalid when deriving a child shadow page's role from its parent to harden against bugs elsewhere in KVM, as violating KVM's invariant that invalid pages are NOT on the list of active MMU pages leads to use-after-free due to __kvm_mmu_prepare_zap_page() using list_add() instead of list_move() when processing an invalid shadow page, i.e. makes a bad situation far worse. Yell loudly if the parent is invalid, as it means KVM has missed a validity check, i.e. KVM is attempting to map memory using an invalid/obsolete root, but continue on as the child is otherwise still a valid shadow page. ================================================================== BUG: KASAN: slab-use-after-free in __kvm_mmu_get_shadow_page+0x1817/0x1860 [kvm] Write of size 8 at addr ff11000153dd1368 by task repro/853 CPU: 1 UID: 1000 PID: 853 Comm: repro Not tainted 7.2.0-rc2-3aec122bdcaf-next-vm #5 PREEMPT Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 Call Trace: dump_stack_lvl+0x4b/0x70 print_report+0x153/0x49c kasan_report+0xbc/0xf0 __kvm_mmu_get_shadow_page+0x1817/0x1860 [kvm] mmu_alloc_root+0x141/0x320 [kvm] kvm_mmu_load+0x612/0x20f0 [kvm] kvm_arch_vcpu_ioctl_run+0x3dd5/0x6150 [kvm] kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm] __x64_sys_ioctl+0x131/0x1b0 do_syscall_64+0x67/0x5f0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 Allocated by task 853: kasan_save_stack+0x20/0x40 kasan_save_track+0x14/0x30 __kasan_slab_alloc+0x5f/0x70 kmem_cache_alloc_noprof+0xfe/0x2e0 __kvm_mmu_topup_memory_cache+0x135/0x530 [kvm] paging64_page_fault+0x318/0x1e30 [kvm] kvm_mmu_do_page_fault+0x21d/0x630 [kvm] kvm_mmu_page_fault+0x18c/0x17b0 [kvm] kvm_arch_vcpu_ioctl_run+0x1f35/0x6150 [kvm] kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm] __x64_sys_ioctl+0x131/0x1b0 do_syscall_64+0x67/0x5f0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 Freed by task 853: kasan_save_stack+0x20/0x40 kasan_save_track+0x14/0x30 kasan_save_free_info+0x3b/0x60 __kasan_slab_free+0x43/0x70 kmem_cache_free+0xe2/0x400 kvm_mmu_commit_zap_page.part.0+0x1e2/0x310 [kvm] kvm_mmu_free_roots+0x283/0x560 [kvm] kvm_arch_vcpu_ioctl_run+0x33c8/0x6150 [kvm] kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm] __x64_sys_ioctl+0x131/0x1b0 do_syscall_64+0x67/0x5f0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 Reported-by: Hyunwoo Kim Fixes: a770f6f28b1a ("KVM: MMU: Inherit a shadow page's guest level count from vcpu setup") Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson Signed-off-by: Paolo Bonzini --- arch/x86/kvm/mmu/mmu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index c9e4739b26d7..a61750f8e1e3 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -2442,6 +2442,9 @@ static union kvm_mmu_page_role kvm_mmu_child_role(u64 *sptep, bool direct, role.direct = direct; role.passthrough = 0; + WARN_ON_ONCE(role.invalid); + role.invalid = 0; + /* * If the guest has 4-byte PTEs then that means it's using 32-bit, * 2-level, non-PAE paging. KVM shadows such guests with PAE paging