netdev - Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when required

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: <ZcDN7ovejiXe754i@linux.intel.com>
Date: Mon, 5 Feb 2024 13:00:46 +0100
From: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: linux-pm@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when
 required

On Sat, Feb 03, 2024 at 02:15:19PM +0100, Jiri Pirko wrote:
> Fri, Feb 02, 2024 at 02:00:46PM CET, stanislaw.gruszka@...ux.intel.com wrote:
> >On Fri, Feb 02, 2024 at 01:36:09PM +0100, Jiri Pirko wrote:
> >> Wed, Jan 31, 2024 at 01:05:35PM CET, stanislaw.gruszka@...ux.intel.com wrote:
> >> 
> >> [...]
> >> 
> >> 
> >> >+static int hfi_netlink_notify(struct notifier_block *nb, unsigned long state,
> >> >+			      void *_notify)
> >> >+{
> >> >+	struct netlink_notify *notify = _notify;
> >> >+	struct hfi_instance *hfi_instance;
> >> >+	smp_call_func_t func;
> >> >+	unsigned int cpu;
> >> >+	int i;
> >> >+
> >> >+	if (notify->protocol != NETLINK_GENERIC)
> >> >+		return NOTIFY_DONE;
> >> >+
> >> >+	switch (state) {
> >> >+	case NETLINK_CHANGE:
> >> >+	case NETLINK_URELEASE:
> >> >+		mutex_lock(&hfi_instance_lock);
> >> >+
> >> 
> >> What's stopping other thread from mangling the listeners here?
> >
> >Nothing. But if the listeners will be changed, we will get next notify.
> >Serialization by the mutex is needed to assure that the last setting will win,
> >so we do not end with HFI disabled when there are listeners or vice versa.
> 
> Okay. Care to put a note somewhere?

I would if the flow would stay the same. But it was requested by Jakub to use
netlink bind/unbind, and this will not work the way described above any longer,
since bind() is before listeners change and unbind() after:

                if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) {
                        err = nlk->netlink_bind(sock_net(sk), val);
                        if (err)
                                return err;
                }
                netlink_table_grab();
                netlink_update_socket_mc(nlk, val,
                                         optname == NETLINK_ADD_MEMBERSHIP);
                netlink_table_ungrab();
                if (optname == NETLINK_DROP_MEMBERSHIP && nlk->netlink_unbind)
                        nlk->netlink_unbind(sock_net(sk), val)

To avoid convoluted logic or new global lock, I'll use properly protected
local counter increased on bind and decreased on unbind.

Regards
Stanislaw