linux-kernel - Re: [PATCH] Fixed division by zero bug in kernel/padata.c

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: <20100702125628.GK10072@secunet.com>
Date:	Fri, 2 Jul 2010 14:56:28 +0200
From:	Steffen Klassert <steffen.klassert@...unet.com>
To:	Dan Kruchinin <dkruchinin@....org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [PATCH] Fixed division by zero bug in kernel/padata.c

On Fri, Jul 02, 2010 at 03:59:54PM +0400, Dan Kruchinin wrote:
>  When boot CPU(typically CPU #0) is excluded from padata cpumask and
>  user enters halt command from console, kernel faults on division by zero;
>  This occurs because during the halt kernel shuts down each non-boot CPU one
>  by one and after it shuts down the last CPU that is set in the padata cpumask,
>  the only working CPU in the system is a boot CPU(#0) and it's the only CPU that
>  is set in the cpu_active_mask. Hence when padata_cpu_callback calls
>  __padata_remove_cpu(which calls padata_alloc_pd) it appears that
> padata cpumask and
>  cpu_active_mask aren't intersect. Hence the following code in
> padata_alloc_pd causes
>  a DZ error exception:
>   cpumask_and(pd->cpumask, cpumask, cpu_active_mask); // pd->cpumask
> will be empty
>   ...
>   num_cpus = cpumask_weight(pd->cpumask); // num_cpus = 0
>   pd->max_seq_nr = (MAX_SEQ_NR / num_cpus) * num_cpus - 1; // DZ!
> 

Good catch!

> 
> Signed-off-by: Dan Kruchinin <dkruchinin@....org>
> ---
>  kernel/padata.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/padata.c b/kernel/padata.c
> index fdd8ae6..dbe6d26 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -434,7 +434,7 @@ static struct parallel_data
> *padata_alloc_pd(struct padata_instance *pinst,
>  		atomic_set(&queue->num_obj, 0);
>  	}
> 
> -	num_cpus = cpumask_weight(pd->cpumask);
> +	num_cpus = cpumask_weight(pd->cpumask) + 1;
>  	pd->max_seq_nr = (MAX_SEQ_NR / num_cpus) * num_cpus - 1;
> 

num_cpus should stay the number of cpus in this cpumask, this is required
to handle a smooth overrun of the sequence numbers. 
I think it's better to return with an error and to stop the instance
if somebody takes away the last cpu in our cpumask. We can't run with an 
empty cpumask anyway.

Let us look again at this on monday.

Thanks again for catching this,

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