linux-kernel - Re: [PATCH] workqueue: Ensure that cpumask set for pools created after boot

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: <20170612173225.GD19206@htj.duckdns.org>
Date:   Mon, 12 Jun 2017 13:32:25 -0400
From:   Tejun Heo <tj@...nel.org>
To:     Michael Bringmann <mwb@...ux.vnet.ibm.com>
Cc:     Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org,
        Nathan Fontenot <nfont@...ux.vnet.ibm.com>
Subject: Re: [PATCH] workqueue: Ensure that cpumask set for pools created
 after boot

Hello,

On Mon, Jun 12, 2017 at 12:10:49PM -0500, Michael Bringmann wrote:
> > The reason why we're ending up with empty masks is because
> > wq_calc_node_cpumask() is assuming that the possible node cpumask is
> > always a superset of online (as it should).  We can trigger a fat
> > warning there if that isn't so and just return false from that
> > function.
> 
> What would that look like?  I should be able to test it on top of the
> other changes / corrections.

So, the function looks like the following now.

  static bool wq_calc_node_cpumask(const struct workqueue_attrs *attrs, int node,
				   int cpu_going_down, cpumask_t *cpumask)
  {
	  if (!wq_numa_enabled || attrs->no_numa)
		  goto use_dfl;

	  /* does @node have any online CPUs @attrs wants? */
A:	cpumask_and(cpumask, cpumask_of_node(node), attrs->cpumask);
	  if (cpu_going_down >= 0)
		  cpumask_clear_cpu(cpu_going_down, cpumask);

B:	if (cpumask_empty(cpumask))
		  goto use_dfl;

	  /* yeap, return possible CPUs in @node that @attrs wants */
C:	cpumask_and(cpumask, attrs->cpumask, wq_numa_possible_cpumask[node]);
	  return !cpumask_equal(cpumask, attrs->cpumask);

  use_dfl:
	  cpumask_copy(cpumask, attrs->cpumask);
	  return false;
  }

A is calculating the target cpumask to use using the online map.  B
falls back to dfl mask if the intersection is empty.  C calculates the
eventual mask to use from the intersection of possible mask and what's
requested.  The assumption is that because possible is a superset of
online, C's result can't be smaller than A.

So, what we can do is if to calculate the possible intersection,
compare it against the online intersection, and if the latter is
bigger, trigger a big fat warning and return false there.

> > I have no idea about the specifics of ppc but at least the code base
> > we have currently expect all possible cpus and nodes and their
> > mappings to be established on boot.
> 
> Hopefully, the new properties will fix the holes in the current implementation
> with regard to hot-add.

Yeah, that's the only proper fix here.

Thanks.

-- 
tejun