netdev - Re: [PATCH net-next 5/7] netconsole: add task name to extra data fields

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250225-doberman-of-scientific-champagne-640c69@leitao>
Date: Tue, 25 Feb 2025 03:17:33 -0800
From: Breno Leitao <leitao@...ian.org>
To: Simon Horman <horms@...nel.org>
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>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
	kernel-team@...a.com
Subject: Re: [PATCH net-next 5/7] netconsole: add task name to extra data
 fields

Hello Simon,

On Tue, Feb 25, 2025 at 10:19:10AM +0000, Simon Horman wrote:
> On Fri, Feb 21, 2025 at 05:52:10AM -0800, Breno Leitao wrote:
> > This is the core patch for this whole patchset. Add support for
> > including the current task's name in netconsole's extra data output.
> > This adds a new append_taskname() function that writes the task name
> > (from current->comm) into the target's extradata buffer, similar to how
> > CPU numbers are handled.
> > 
> > The task name is included when the SYSDATA_TASKNAME field is set,
> > appearing in the format "taskname=<name>" in the output. This additional
> > context can help with debugging by showing which task generated each
> > console message.
> > 
> > Signed-off-by: Breno Leitao <leitao@...ian.org>
> > ---
> >  drivers/net/netconsole.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index 5a29144ae37ee7b487b1a252b0f2ce8574f9cefa..625f4c0be11d8deb454139b1c526abc842697219 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -1179,12 +1179,22 @@ static int append_cpu_nr(struct netconsole_target *nt, int offset)
> >  			 raw_smp_processor_id());
> >  }
> >  
> > +static int append_taskname(struct netconsole_target *nt, int offset)
> > +{
> > +	if (WARN_ON_ONCE(!current))
> > +		return 0;
> 
> Hi Breno,
> 
> I gather that theoretically this could occur, but it isn't expected
> to happen in practice. Is that right?

That's correct. `current` isn't expected to be NULL in practice.
I've been running this code on several servers for days and have never
encountered this warning. 

While the taskname feature isn't enabled during early boot, netconsole
might be active at that time, which is why I exercised extra caution
here.

If you also think this is safe, I am more than happy to remove this
check.

Thanks for the review,
--breno