linux-kernel - 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-bright-jasmine-butterfly-aa1bb0@leitao>
Date: Tue, 25 Feb 2025 05:11:47 -0800
From: Breno Leitao <leitao@...ian.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Simon Horman <horms@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, 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 Paolo,

On Tue, Feb 25, 2025 at 12:53:49PM +0100, Paolo Abeni wrote:
> On 2/25/25 12:17 PM, Breno Leitao wrote:
> > 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.
> 
> So `current` can't be NULL here. I think it's better to drop such check,
> it's presence would be misleading. i.e. like adding checks for UDP stack
> being initialized before calling send_msg_fragmented()

Ack. I will remove the check then, and check if the UDP stack has been
initialized before calling netpoll helpers.

What is the best way to make sure taht the UDP stack has been
initialized?

Thanks for the review,
--breno