netdev - Re: [PATCH net-next 4/5] devlink: embed driver's priv data callback param into devlink

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: <20240808194150.1ac32478@kernel.org>
Date: Thu, 8 Aug 2024 19:41:50 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Przemek Kitszel <przemyslaw.kitszel@...el.com>, netdev@...r.kernel.org,
 Ido Schimmel <idosch@...dia.com>, Petr Machata <petrm@...dia.com>, Andrew
 Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Vladimir
 Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Saeed
 Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>, Tariq
 Toukan <tariqt@...dia.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
 nex.sw.ncis.osdt.itp.upstreaming@...el.com, Wojciech Drewek
 <wojciech.drewek@...el.com>
Subject: Re: [PATCH net-next 4/5] devlink: embed driver's priv data callback
 param into devlink_resource

On Wed, 7 Aug 2024 08:49:57 +0200 Jiri Pirko wrote:
> > 	lockdep_assert_held(&devlink->lock);
> > 
> > 	resource = devlink_resource_find(devlink, NULL, resource_id);
> >-	if (WARN_ON(!resource))
> >+	if (WARN_ON(!resource || occ_priv_size > resource->priv_size))  
> 
> Very odd. You allocate a mem in devl_resource_register() and here you
> copy data to it. Why the void pointer is not enough for you? You can
> easily alloc struct in the driver and pass a pointer to it.
> 
> This is quite weird. Please don't.

The patch is a bit of a half measure, true.

Could you shed more light on the design choices for the resource API,
tho? Why the tying of objects by driver-defined IDs? It looks like 
the callback for getting resources occupancy is "added" later once 
the resource is registered? Is this some legacy of the old locking
scheme? It's quite unusual.