Received-SPF: pass (google.com: domain of linux-kernel+bounces-70499-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH printk v2 06/26] printk: nbcon: Ensure ownership release on failed emit
Date: Sun, 18 Feb 2024 20:03:06 +0106
Message-Id: <20240218185726.1994771-7-john.ogness@linutronix.de>
In-Reply-To: <20240218185726.1994771-1-john.ogness@linutronix.de>
References: <20240218185726.1994771-1-john.ogness@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Until now it was assumed that ownership has been lost when the
write_atomic() callback fails. And nbcon_emit_next_record()
directly returned false. However, if nbcon_emit_next_record()
returns false, the context must no longer have ownership.

The semantics for the callbacks could be specified such that
if they return false, they must have released ownership. But
in practice those semantics seem odd since the ownership was
acquired outside of the callback.

Ensure ownership has been released before reporting failure by
explicitly attempting a release. If the current context is not
the owner, the release has no effect.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/nbcon.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index c8093bcc01fe..8ecd76aa22e6 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -852,7 +852,7 @@ static bool nbcon_emit_next_record(struct nbcon_write_context *wctxt)
 	unsigned long con_dropped;
 	struct nbcon_state cur;
 	unsigned long dropped;
-	bool done;
+	bool done = false;
 
 	/*
 	 * The printk buffers are filled within an unsafe section. This
@@ -891,17 +891,18 @@ static bool nbcon_emit_next_record(struct nbcon_write_context *wctxt)
 	nbcon_state_read(con, &cur);
 	wctxt->unsafe_takeover = cur.unsafe_takeover;
 
-	if (con->write_atomic) {
+	if (con->write_atomic)
 		done = con->write_atomic(con, wctxt);
-	} else {
-		nbcon_context_release(ctxt);
-		WARN_ON_ONCE(1);
-		done = false;
-	}
 
-	/* If not done, the emit was aborted. */
-	if (!done)
+	if (!done) {
+		/*
+		 * The emit was aborted, probably due to a loss of ownership.
+		 * Ensure ownership was lost or released before reporting the
+		 * loss.
+		 */
+		nbcon_context_release(ctxt);
 		return false;
+	}
 
 	/*
 	 * Since any dropped message was successfully output, reset the
-- 
2.39.2