Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing check for fatal signals in retry loop handling HMM faults. -- commit 4f92a5b1f4b0160351016ede4e0c8d47d308cbfa Author: Stanislav Kinsburskii <[email protected]> mshv: Use hmm_range_fault_unlocked() for region faults Convert mshv_region_hmm_fault_and_lock() to use hmm_range_fault_unlocked() instead of taking mmap_read_lock() around hmm_range_fault() directly. This allows the MSHV fault path to handle mappings whose fault handlers may drop mmap_lock. > diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c > index 6d65e5b421529..04676f06c5c74 100644 > --- a/drivers/hv/mshv_regions.c > +++ b/drivers/hv/mshv_regions.c > @@ -388,13 +388,11 @@ int mshv_region_get(struct mshv_mem_region *region) > * > * This function performs the following steps: > * 1. Reads the notifier sequence for the HMM range. > - * 2. Acquires a read lock on the memory map. > - * 3. Handles HMM faults for the specified range. > - * 4. Releases the read lock on the memory map. > - * 5. If successful, locks the memory region mutex. > - * 6. Verifies if the notifier sequence has changed during the operation. > + * 2. Handles HMM faults for the specified range. > + * 3. If successful, locks the memory region mutex. > + * 4. Verifies if the notifier sequence has changed during the operation. > * If it has, releases the mutex and returns -EBUSY to match with > - * hmm_range_fault() return code for repeating. > + * hmm_range_fault_unlocked() so the caller retries the range fault. [Severity: High] This isn't a bug introduced by this patch, but since we are modifying the fault retry logic, does the retry loop in mshv_region_range_fault() need to check for fatal signals? When mmu_interval_read_retry() detects a collision, this function returns -EBUSY, which bubbles up to the caller in drivers/hv/mshv_regions.c: mshv_region_range_fault() { ... do { ret = mshv_region_hmm_fault_and_lock(region, &range); } while (ret == -EBUSY); ... } If a userspace program continuously invalidates the memory range concurrently (for example, via madvise(MADV_DONTNEED)), mmu_interval_read_retry() can repeatedly return true. Without a check for fatal_signal_pending(current) inside the loop or before returning -EBUSY, could this cause the kernel thread to spin indefinitely and ignore fatal signals like SIGKILL? > * > * Return: 0 on success, a negative error code otherwise. > */ > @@ -404,9 +402,7 @@ static int mshv_region_hmm_fault_and_lock(struct > mshv_mem_region *region, > int ret; > > range->notifier_seq = mmu_interval_read_begin(range->notifier); > - mmap_read_lock(region->mreg_mni.mm); > - ret = hmm_range_fault(range); > - mmap_read_unlock(region->mreg_mni.mm); > + ret = hmm_range_fault_unlocked(range); > if (ret) > return ret; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/178336023903.504354.7500950448226027718.stgit@skinsburskii?part=4
