public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Gui-Dong Han <2045gemini@gmail.com>
To: gregkh@linuxfoundation.org, ivan.orlov0322@gmail.com,
	surenb@google.com, 42.hyeyoo@gmail.com, Liam.Howlett@Oracle.com
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	baijiaju1990@outlook.com, Gui-Dong Han <2045gemini@gmail.com>,
	BassCheck <bass@buaa.edu.cn>
Subject: [PATCH] usb: mon: Fix atomicity violation in mon_bin_vma_fault
Date: Thu, 21 Dec 2023 18:40:34 +0800	[thread overview]
Message-ID: <20231221104034.4851-1-2045gemini@gmail.com> (raw)

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. 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.

Fixes: 6f23ee1fefdc1 ("USB: add binary API to usbmon")
Reported-by: BassCheck <bass@buaa.edu.cn>
Signed-off-by: Gui-Dong Han <2045gemini@gmail.com>
---
 drivers/usb/mon/mon_bin.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index 9ca9305243fe..509cd1b8ff13 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -1250,12 +1250,16 @@ static vm_fault_t mon_bin_vma_fault(struct vm_fault *vmf)
 	struct mon_reader_bin *rp = vmf->vma->vm_private_data;
 	unsigned long offset, chunk_idx;
 	struct page *pageptr;
-
+	unsigned long flags;
+	spin_lock_irqsave(&rp->b_lock, flags);
 	offset = vmf->pgoff << PAGE_SHIFT;
-	if (offset >= rp->b_size)
+	if (offset >= rp->b_size) {
+		spin_unlock_irqrestore(&rp->b_lock, flags);
 		return VM_FAULT_SIGBUS;
+	}
 	chunk_idx = offset / CHUNK_SIZE;
 	pageptr = rp->b_vec[chunk_idx].pg;
+	spin_unlock_irqrestore(&rp->b_lock, flags);
 	get_page(pageptr);
 	vmf->page = pageptr;
 	return 0;
-- 
2.34.1


             reply	other threads:[~2023-12-21 10:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 10:40 Gui-Dong Han [this message]
2023-12-21 10:49 ` [PATCH] usb: mon: Fix atomicity violation in mon_bin_vma_fault Greg KH
2023-12-21 10:50 ` Greg KH
2023-12-22  6:13   ` 20 39

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231221104034.4851-1-2045gemini@gmail.com \
    --to=2045gemini@gmail.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=Liam.Howlett@Oracle.com \
    --cc=baijiaju1990@outlook.com \
    --cc=bass@buaa.edu.cn \
    --cc=gregkh@linuxfoundation.org \
    --cc=ivan.orlov0322@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox