linux-kernel - Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <68f9b7d000ec433adef02294552280902590f762.camel@suse.com>
Date: Thu, 02 Oct 2025 16:03:07 -0300
From: Marcos Paulo de Souza <mpdesouza@...e.com>
To: John Ogness <john.ogness@...utronix.de>, Greg Kroah-Hartman	
 <gregkh@...uxfoundation.org>, Petr Mladek <pmladek@...e.com>, Steven
 Rostedt	 <rostedt@...dmis.org>, Sergey Senozhatsky
 <senozhatsky@...omium.org>, Jason Wessel <jason.wessel@...driver.com>,
 Daniel Thompson <danielt@...nel.org>, Douglas Anderson	
 <dianders@...omium.org>
Cc: linux-kernel@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net
Subject: Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

On Wed, 2025-10-01 at 17:02 +0206, John Ogness wrote:
> On 2025-09-30, Marcos Paulo de Souza <mpdesouza@...e.com> wrote:
> > diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> > index
> > 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc420
> > 57fadbb44a43446 100644
> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -1855,3 +1855,69 @@ void nbcon_device_release(struct console
> > *con)
> >  	console_srcu_read_unlock(cookie);
> >  }
> >  EXPORT_SYMBOL_GPL(nbcon_device_release);
> > +
> > +/**
> > + * nbcon_kdb_try_acquire - Try to acquire nbcon console, enter
> > unsafe
> > + *				section, and initialized nbcon
> > write context
> 
>                                initialize ---^^^^^^^^^^^
> 
> And technically it is not initializing the write context, just the
> console ownership context. I'm not sure it is really necessary to
> mention that.
> 
> > + * @con:	The nbcon console to acquire
> > + * @wctxt:	The nbcon write context to be used on success
> > + *
> > + * Context:	Under console_srcu_read_lock() for emiting a
> > single kdb message
> 
>                                        emitting ---^^^^^^^
> 
> "./scripts/checkpatch.pl --codespell" is your friend. ;-)
> 
> > + *		using the given con->write_atomic() callback. Can
> > be called
> > + *		only when the console is usable at the moment.
> > + *
> > + * Return:	True if the console was acquired. False otherwise.
> > + *
> > + * kdb emits messages on consoles registered for printk() without
> > + * storing them into the ring buffer. It has to acquire the
> > console
> > + * ownerhip so that it could call con->write_atomic() callback a
> > safe way.
> > + *
> > + * This function acquires the nbcon console using priority
> > NBCON_PRIO_EMERGENCY
> > + * and marks it unsafe for handover/takeover.
> > + */
> > +bool nbcon_kdb_try_acquire(struct console *con,
> > +			   struct nbcon_write_context *wctxt)
> > +{
> > +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > +	memset(ctxt, 0, sizeof(*ctxt));
> > +	ctxt->console = con;
> > +	ctxt->prio    = NBCON_PRIO_EMERGENCY;
> > +
> > +	if (!nbcon_context_try_acquire(ctxt, false))
> > +		return false;
> > +
> > +	if (!nbcon_context_enter_unsafe(ctxt))
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * nbcon_kdb_release - Exit unsafe section and release the nbcon
> > console
> > + *
> > + * @wctxt:	The nbcon write context initialized by a
> > successful
> > + *		nbcon_kdb_try_acquire()
> > + *
> > + * Context:	Under console_srcu_read_lock() for emiting a
> > single kdb message
> 
>                                        emitting ---^^^^^^^
> 
> > + *		using the given con->write_atomic() callback. Can
> > be called
> > + *		only when the console is usable at the moment.
> 
> I do not think the "Context" is relevant. It must be called if
> a previous call to nbcon_kdb_try_acquire() was successful.
> 
> > + */
> > +void nbcon_kdb_release(struct nbcon_write_context *wctxt)
> > +{
> > +	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > +	if (!nbcon_context_exit_unsafe(ctxt))
> > +		return;
> > +
> > +	nbcon_context_release(ctxt);
> > +
> > +	/*
> > +	 * Flush any new printk() messages added when the console
> > was blocked.
> > +	 * Only the console used by the given write context
> > was	blocked.
> > +	 * The console was locked only when the write_atomic()
> > callback
> > +	 * was usable.
> > +	 */
> > +	__nbcon_atomic_flush_pending_con(ctxt->console,
> > +					
> > prb_next_reserve_seq(prb), false);
> 
> This can all be one line. 100 characters is the official limit for
> code.

I fixed all your suggestions and published my branch here[1]. The only
change that I didn't fixed was about the "Context:" change that you
suggested, since I'll let Petr to thing about it, since he usually has
comments about it.

[1]: https://github.com/marcosps/linux/tree/nbcon-kgdboc-v5

> 
> > +}