linux-kernel - Re: [PATCH net-next 2/4] netconsole: Add option to auto-populate CPU number in userdata

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: <20241204-smooth-fascinating-millipede-cc67b0@leitao>
Date: Wed, 4 Dec 2024 08:52:25 -0800
From: Breno Leitao <leitao@...ian.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...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, max@...sevol.com,
	thepacketgeek@...il.com, vlad.wing@...il.com,
	davej@...emonkey.org.uk
Subject: Re: [PATCH net-next 2/4] netconsole: Add option to auto-populate CPU
 number in userdata

On Tue, Nov 19, 2024 at 09:07:45AM -0800, Breno Leitao wrote:
> > >  #endif
> > >  	bool			enabled;
> > >  	bool			extended;
> > 
> > > +	/* Check if CPU NR should be populated, and append it to the user
> > > +	 * dictionary.
> > > +	 */
> > > +	if (child_count < MAX_USERDATA_ITEMS && nt->userdata_auto & AUTO_CPU_NR)
> > > +		scnprintf(&nt->userdata_complete[complete_idx],
> > > +			  MAX_USERDATA_ENTRY_LENGTH, " cpu=%u\n",
> > > +			  raw_smp_processor_id());
> > 
> > I guess it may be tricky for backward compat, but shouldn't we return
> > an error rather than silently skip?
> 
> yes, this should be easy to do, in fact. Probably return -E2BIG to
> userspace when trying to update the entry. I thought about something as
> the following patch, and piggy-back into it.

Back to this topic, in fact, this is not needed at all. 

The configfs make item helper (userdatum_make_item()) checks for
exceeding entries, and fails if an additional entry is created.


	static struct config_item *userdatum_make_item(struct config_group *group,
							const char *name)
	{
		....
		child_count = list_count_nodes(&nt->userdata_group.cg_children);
		if (child_count >= MAX_USERDATA_ITEMS)
			return ERR_PTR(-ENOSPC);


I've sent an additional test for this mechanism, and make the check in
update_userdata() a warning instead of just silently dropping the entry.

https://lore.kernel.org/all/20241204-netcons_overflow_test-v1-0-a85a8d0ace21@debian.org/