linux-kernel - Re: [PATCH net-next v4 1/5] printk: Add execution context (task name/CPU) to printk

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: <aXnp0uRXlrSMUlcH@gmail.com>
Date: Wed, 28 Jan 2026 02:52:21 -0800
From: Breno Leitao <leitao@...ian.org>
To: John Ogness <john.ogness@...utronix.de>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, pmladek@...e.com, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Steven Rostedt <rostedt@...dmis.org>, 
	Sergey Senozhatsky <senozhatsky@...omium.org>, Andrew Morton <akpm@...ux-foundation.org>, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, asantostc@...il.com, efault@....de, 
	gustavold@...il.com, calvin@...nvd.org, jv@...sburgh.net, mpdesouza@...e.com, 
	kernel-team@...a.com
Subject: Re: [PATCH net-next v4 1/5] printk: Add execution context (task
 name/CPU) to printk_info

Hello John,

On Tue, Jan 27, 2026 at 09:49:25PM +0106, John Ogness wrote:
> On 2026-01-23, Breno Leitao <leitao@...ian.org> wrote:
> > diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> > index 32fc12e536752..391a58be0c5b3 100644
> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -946,6 +946,19 @@ void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt)
> >  }
> >  EXPORT_SYMBOL_GPL(nbcon_reacquire_nobuf);
> >  
> > +#ifdef CONFIG_PRINTK_EXECUTION_CTX
> > +static void wctxt_load_execution_ctx(struct nbcon_write_context *wctxt,
> > +				     struct printk_message *pmsg)
> > +{
> > +	wctxt->cpu = pmsg->cpu;
> > +	wctxt->pid = pmsg->pid;
> > +	memcpy(wctxt->comm, pmsg->comm, TASK_COMM_LEN);
> 
> Perhaps using sizeof() instead?
> 
> 	memcpy(wctxt->comm, pmsg->comm, sizeof(wctxt->comm));
> 
> And adding a static assert that the sizes match?
> 
> 	static_assert(sizeof(wctxt->comm) == sizeof(pmsg->comm));

Sure, I move the size from TASK_COMM_LEN to sizeof(wctxt->comm) and also
adding the build time assert.

> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 1d765ad242b82..7daaa27705339 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2131,12 +2131,37 @@ static inline void printk_delay(int level)
> >  	}
> >  }
> >  
> > +#define CALLER_ID_MASK 0x80000000
> > +
> >  static inline u32 printk_caller_id(void)
> >  {
> >  	return in_task() ? task_pid_nr(current) :
> > -		0x80000000 + smp_processor_id();
> > +		CALLER_ID_MASK + smp_processor_id();
> > +}
> > +
> > +#ifdef CONFIG_PRINTK_EXECUTION_CTX
> > +/* Store the opposite info than caller_id. */
> > +static u32 printk_caller_id2(void)
> > +{
> > +	return !in_task() ? task_pid_nr(current) :
> > +		CALLER_ID_MASK + smp_processor_id();
> >  }
> >  
> > +static pid_t printk_info_get_pid(const struct printk_info *info)
> > +{
> > +	u32 caller_id = info->caller_id;
> > +	u32 caller_id2 = info->caller_id2;
> > +
> > +	return caller_id & CALLER_ID_MASK ? caller_id2 : caller_id;
> > +}
> > +
> > +static int printk_info_get_cpu(const struct printk_info *info)
> > +{
> > +	return ((info->caller_id & CALLER_ID_MASK ?
> > +		 info->caller_id : info->caller_id2) & ~CALLER_ID_MASK);
> > +}
> 
> It is a bit odd that printk_info_get_pid() uses local variables and
> printk_info_get_cpu() does not. I could understand if things evolve that
> way over time, but it is odd to use the two different styles from the
> beginning.
>
> I would prefer the local variable variant. But mostly I would prefer
> that they are the same style.

They evolve different, I will get both of them using local variables and
in the same style.

> > +
> > +static void pmsg_load_execution_ctx(struct printk_message *pmsg,
> > +				    const struct printk_info *info)
> > +{
> > +	pmsg->cpu = printk_info_get_cpu(info);
> > +	pmsg->pid = printk_info_get_pid(info);
> > +	memcpy(pmsg->comm, info->comm, TASK_COMM_LEN);
> 
> Here I also suggest using sizeof() and static_assert():
> 
> 	memcpy(pmsg->comm, info->comm, sizeof(pmsg->comm));
> 	static_assert(sizeof(pmsg->comm) == sizeof(info->comm));

Ack! I will respin it.

Thanks for the review,
--breno