linux-kernel - Re: [Bugfix v2 2/5] x86/irq: Enhance __assign_irq

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: <alpine.DEB.2.11.1512301947370.28591@nanos>
Date:	Wed, 30 Dec 2015 19:52:20 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jiang Liu <jiang.liu@...ux.intel.com>
cc:	Joe Lawrence <joe.lawrence@...atus.com>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Jeremiah Mahler <jmmahler@...il.com>,
	Borislav Petkov <bp@...en8.de>, andy.shevchenko@...il.com,
	Guenter Roeck <linux@...ck-us.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [Bugfix v2 2/5] x86/irq: Enhance __assign_irq_vector() to rollback
 in case of failure

On Wed, 23 Dec 2015, Jiang Liu wrote:
> @@ -167,11 +170,13 @@ next:
>  
>  		if (test_bit(vector, used_vectors))
>  			goto next;
> -
>  		for_each_cpu_and(new_cpu, vector_cpumask, cpu_online_mask) {
>  			if (!IS_ERR_OR_NULL(per_cpu(vector_irq, new_cpu)[vector]))
>  				goto next;
>  		}
> +		if (apic->cpu_mask_to_apicid_and(mask, vector_cpumask, &dest))
> +			goto next;

This is silly. If the checks in cpu_mask_to_apicid_and() fail, then there is
no point to try the next vector number simply because the checks will fail
with the next vector again. You need a new vector_cpumask() in order to make
progress.

> +
>  		/* Found one! */
>  		current_vector = vector;
>  		current_offset = offset;
> @@ -190,8 +195,7 @@ next:
>  
>  	if (!err) {
>  		/* cache destination APIC IDs into cfg->dest_apicid */
> -		err = apic->cpu_mask_to_apicid_and(mask, d->domain,
> -						   &d->cfg.dest_apicid);

I have serious doubts that this is the proper solution.

d->domain is @vector_cpumask. @vector_cpumask gets assigned by the call to
apic->vector_allocation_domain()

So the only way this can fail is when:

   vector_cpumask & mask & cpu_online_mask == 0

So we can do that check way earlier, i.e. right after the call to
apic->vector_allocation_domain(). Once we have established that this check
does not fail, we know that the call to apic->cpu_mask_to_apicid_and() wont
fail either once we have a vector number to use.

Thanks,

	tglx
--
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/