netdev - Re: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver modules' firmware notifications ability

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: <20240522072212.7a21c84b@kernel.org>
Date: Wed, 22 May 2024 07:22:12 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Danielle Ratson <danieller@...dia.com>
Cc: Ido Schimmel <idosch@...dia.com>, "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com"
 <pabeni@...hat.com>, "corbet@....net" <corbet@....net>,
 "linux@...linux.org.uk" <linux@...linux.org.uk>, "sdf@...gle.com"
 <sdf@...gle.com>, "kory.maincent@...tlin.com" <kory.maincent@...tlin.com>,
 "maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
 "vladimir.oltean@....com" <vladimir.oltean@....com>,
 "przemyslaw.kitszel@...el.com" <przemyslaw.kitszel@...el.com>,
 "ahmed.zaki@...el.com" <ahmed.zaki@...el.com>, "richardcochran@...il.com"
 <richardcochran@...il.com>, "shayagr@...zon.com" <shayagr@...zon.com>,
 "paul.greenwalt@...el.com" <paul.greenwalt@...el.com>, "jiri@...nulli.us"
 <jiri@...nulli.us>, "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, mlxsw <mlxsw@...dia.com>, Petr Machata
 <petrm@...dia.com>
Subject: Re: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver
 modules' firmware notifications ability

On Wed, 22 May 2024 13:56:11 +0000 Danielle Ratson wrote:
> > > 4. Add a new netlink notifier that when the relevant event takes place,  
> > deletes the node from the list, wait until the end of the work item, with
> > cancel_work_sync() and free allocations.
> > 
> > What's the "relevant event" in this case? Closing of the socket that user had
> > issued the command on?  
> 
> The event should match the below:
> event == NETLINK_URELEASE && notify->protocol == NETLINK_GENERIC
> 
> Then iterate over the list to look for work that matches the dev and portid.
> The socket doesn’t close until the work is done in that case. 

Okay, good, yes. I think you can use one of the callbacks I mentioned
below to achieve the same thing with less complexity than the notifier.

> > Easiest way to "notice" the socket got closed would probably be to add some
> > info to genl_sk_priv_*(). ->sock_priv_destroy() will get called. But you can also
> > get a close notification in the family  
> > ->unbind callback.  
> > 
> > I'm on the fence whether we should cancel the work. We could just mark the
> > command as 'no socket present' and stop sending notifications.
> > Not sure which is better..  
> 
> Is there a scenario that we hit this event and won't intend to cancel the work? 

I think it's up to us. I don't see any legit reason for user space to
intentionally cancel the flashing. So the only option is that user space
is either buggy or has crashed, and the socket got closed before
flashing finished. Right?