linux-kernel - Re: [PATCH v5 4/8] smp: Add func to IPI cpus based on parameter func

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: <20120103143417.7cbea589.akpm@linux-foundation.org>
Date:	Tue, 3 Jan 2012 14:34:17 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Gilad Ben-Yossef <gilad@...yossef.com>
Cc:	linux-kernel@...r.kernel.org, Chris Metcalf <cmetcalf@...era.com>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Russell King <linux@....linux.org.uk>, linux-mm@...ck.org,
	Pekka Enberg <penberg@...nel.org>,
	Matt Mackall <mpm@...enic.com>,
	Sasha Levin <levinsasha928@...il.com>,
	Rik van Riel <riel@...hat.com>,
	Andi Kleen <andi@...stfloor.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, Avi Kivity <avi@...hat.com>
Subject: Re: [PATCH v5 4/8] smp: Add func to IPI cpus based on parameter
 func

On Mon,  2 Jan 2012 12:24:15 +0200
Gilad Ben-Yossef <gilad@...yossef.com> wrote:

> Add the on_each_cpu_required() function that wraps on_each_cpu_mask()
> and calculates the cpumask of cpus to IPI by calling a function supplied
> as a parameter in order to determine whether to IPI each specific cpu.

The name is actually "on_each_cpu_cond".

> The function deals with allocation failure of cpumask variable in
> CONFIG_CPUMASK_OFFSTACK=y by sending IPI to all cpus via on_each_cpu()
> instead.

This seems rather dangerous.  Poeple will test and ship code which has
always called only the targetted CPUs.  Later, real live users will get
the occasional memory exhaustion and will end up calling the callback
function on CPUs which aren't supposed to be used.  So users end up
running untested code.  And it's code which could quite easily explode,
because inattentive programmers could fall into assuming that the
function is not called on incorrect CPUs.  I think this is easy to fix
(see below).

> The function is useful since it allows to seperate the specific
> code that decided in each case whether to IPI a specific cpu for
> a specific request from the common boilerplate code of handling
> creating the mask, handling failures etc.
> 
>
> ...
>
> @@ -147,6 +155,14 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
>  			local_irq_enable();		\
>  		}					\
>  	} while (0)
> +#define on_each_cpu_cond(cond_func, func, info, wait) \
> +	do {						\
> +		if (cond_func(0, info)) {		\

I suppose this is reasonable.  It's likely that on UP, cond_func() will
always return true but perhaps for some reason it won't.  hmmm...

> +			local_irq_disable();		\
> +			(func)(info);			\
> +			local_irq_enable();		\
> +		}					\
> +	} while (0)
>  
>  static inline void smp_send_reschedule(int cpu) { }
>  #define num_booting_cpus()			1
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 7c0cbd7..5f7b24e 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -721,3 +721,30 @@ void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
>  	put_cpu();
>  }
>  EXPORT_SYMBOL(on_each_cpu_mask);
> +
> +/*
> + * Call a function on each processor for which the supplied function
> + * cond_func returns a positive value. This may include the local
> + * processor, optionally waiting for all the required CPUs to finish.
> + * The function may be called on all online CPUs without running the
> + * cond_func function in extreme circumstance (memory allocation
> + * failure condition when CONFIG_CPUMASK_OFFSTACK=y)
> + * All the limitations specified in smp_call_function_many apply.
> + */
> +void on_each_cpu_cond(int (*cond_func) (int cpu, void *info),
> +			void (*func)(void *), void *info, bool wait)
> +{
> +	cpumask_var_t cpus;
> +	int cpu;
> +
> +	if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
> +		for_each_online_cpu(cpu)
> +			if (cond_func(cpu, info))
> +				cpumask_set_cpu(cpu, cpus);
> +		on_each_cpu_mask(cpus, func, info, wait);
> +		free_cpumask_var(cpus);
> +	} else
> +		on_each_cpu(func, info, wait);
> +}
> +EXPORT_SYMBOL(on_each_cpu_cond);

If zalloc_cpumask_var() fails, can we not fall back to

		for_each_online_cpu(cpu)
			if (cond_func(cpu, info))
				smp_call_function_single(...);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/