Skip to content

Commit 2dd23cc

Browse files
2045geminigregkh
authored andcommitted
usb: mon: Fix atomicity violation in mon_bin_vma_fault
In mon_bin_vma_fault(): offset = vmf->pgoff << PAGE_SHIFT; if (offset >= rp->b_size) return VM_FAULT_SIGBUS; chunk_idx = offset / CHUNK_SIZE; pageptr = rp->b_vec[chunk_idx].pg; The code is executed without holding any lock. In mon_bin_vma_close(): spin_lock_irqsave(&rp->b_lock, flags); rp->mmap_active--; spin_unlock_irqrestore(&rp->b_lock, flags); In mon_bin_ioctl(): spin_lock_irqsave(&rp->b_lock, flags); if (rp->mmap_active) { ... } else { ... kfree(rp->b_vec); rp->b_vec = vec; rp->b_size = size; ... } spin_unlock_irqrestore(&rp->b_lock, flags); Concurrent execution of mon_bin_vma_fault() with mon_bin_vma_close() and mon_bin_ioctl() could lead to atomicity violations. mon_bin_vma_fault() accesses rp->b_size and rp->b_vec without locking, risking array out-of-bounds access or use-after-free bugs due to possible modifications in mon_bin_ioctl(). This possible bug is found by an experimental static analysis tool developed by our team, BassCheck[1]. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. The above possible bug is reported when our tool analyzes the source code of Linux 6.2. To address this issue, it is proposed to add a spin lock pair in mon_bin_vma_fault() to ensure atomicity. With this patch applied, our tool never reports the possible bug, with the kernel configuration allyesconfig for x86_64. Due to the lack of associated hardware, we cannot test the patch in runtime testing, and just verify it according to the code logic. [1] https://sites.google.com/view/basscheck/ Fixes: 19e6317 ("usb: mon: Fix a deadlock in usbmon between ...") Cc: <stable@vger.kernel.org> Signed-off-by: Gui-Dong Han <2045gemini@gmail.com> Link: https://lore.kernel.org/r/20240105052412.9377-1-2045gemini@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 9866dc4 commit 2dd23cc

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

drivers/usb/mon/mon_bin.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1250,14 +1250,19 @@ static vm_fault_t mon_bin_vma_fault(struct vm_fault *vmf)
12501250
struct mon_reader_bin *rp = vmf->vma->vm_private_data;
12511251
unsigned long offset, chunk_idx;
12521252
struct page *pageptr;
1253+
unsigned long flags;
12531254

1255+
spin_lock_irqsave(&rp->b_lock, flags);
12541256
offset = vmf->pgoff << PAGE_SHIFT;
1255-
if (offset >= rp->b_size)
1257+
if (offset >= rp->b_size) {
1258+
spin_unlock_irqrestore(&rp->b_lock, flags);
12561259
return VM_FAULT_SIGBUS;
1260+
}
12571261
chunk_idx = offset / CHUNK_SIZE;
12581262
pageptr = rp->b_vec[chunk_idx].pg;
12591263
get_page(pageptr);
12601264
vmf->page = pageptr;
1265+
spin_unlock_irqrestore(&rp->b_lock, flags);
12611266
return 0;
12621267
}
12631268

0 commit comments

Comments
 (0)