mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Qiu-ji Chen <chenqiuji666@gmail.com>
To: amit@kernel.org, arnd@arndb.de, gregkh@linuxfoundation.org
Cc: virtualization@lists.linux.dev, linux-kernel@vger.kernel.org,
	baijiaju1990@gmail.com, Qiu-ji Chen <chenqiuji666@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH] virtio: console: Fix atomicity violation in fill_readbuf()
Date: Fri, 27 Sep 2024 16:40:56 +0800	[thread overview]
Message-ID: <20240927084056.7193-1-chenqiuji666@gmail.com> (raw)

The atomicity violation issue is due to the invalidation of the function 
port_has_data()'s check caused by concurrency. Imagine a scenario where a 
port that contains data passes the validity check but is simultaneously 
assigned a value with no data. This could result in an empty port passing 
the validity check, potentially leading to a null pointer dereference 
error later in the program, which is inconsistent.

To address this issue, we believe there is a problem with the original 
logic. Since the validity check and the read operation were separated, it 
could lead to inconsistencies between the data that passes the check and 
the data being read. Therefore, we moved the main logic of the 
port_has_data function into this function and placed the read operation 
within a lock, ensuring that the validity check and read operation are 
not separated, thus resolving the problem.

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.

Fixes: 203baab8ba31 ("virtio: console: Introduce function to hand off data from host to readers")
Cc: stable@vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666@gmail.com>
---
 drivers/char/virtio_console.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index de7d720d99fa..5aaf07f71a4c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -656,10 +656,14 @@ static ssize_t fill_readbuf(struct port *port, u8 __user *out_buf,
 	struct port_buffer *buf;
 	unsigned long flags;
 
-	if (!out_count || !port_has_data(port))
+	if (!out_count)
 		return 0;
 
-	buf = port->inbuf;
+	spin_lock_irqsave(&port->inbuf_lock, flags);
+	buf = port->inbuf = get_inbuf(port);
+	spin_unlock_irqrestore(&port->inbuf_lock, flags);
+	if (!buf)
+		return 0;
 	out_count = min(out_count, buf->len - buf->offset);
 
 	if (to_user) {
-- 
2.34.1

                 reply	other threads:[~2024-09-27  8:41 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20240927084056.7193-1-chenqiuji666@gmail.com \
    --to=chenqiuji666@gmail.com \
    --cc=amit@kernel.org \
    --cc=arnd@arndb.de \
    --cc=baijiaju1990@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=virtualization@lists.linux.dev \
    /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

all inboxes | Powered by JetHome®