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: <20090112185546.GA15494@elte.hu>
Date:	Mon, 12 Jan 2009 19:55:46 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Mike Travis <travis@....com>
Cc:	Dieter Ries <clip2@....de>, rusty@...tcorp.com.au,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: 2.6.29-rc1 does not boot


* Mike Travis <travis@....com> wrote:

> Ingo Molnar wrote:
> > * Mike Travis <travis@....com> wrote:
> ...
> >> Rusty - any ideas on how to avoid these clashes with the 
> >> get_online_cpus() call in work_on_cpu()?  Or something else to indicate 
> >> to lockdep that the circular lock dependency is ok (as you mentioned 
> >> before)?
> > 
> > I've queued up the revert below, please check the commit message whether 
> > you agree with the analysis.
> > 
> > Mike, could you also check any other patches where you add work_on_cpu() 
> > usage to make sure we dont have similar mishaps? work_on_cpu() seems 
> > completely unsuited for any sort of set_cpus_allowed() replacement ...
> > 
> > 	Ingo
> 
> Yes, I'll do that now.  With the resume feature also calling these functions,
> I'm even less comfortable with it.
> 
> Shall I resurrect the 2nd cpumask in the task struct from my original patches,
> (and one that akpm also suggested more than a year ago)?
> 
> Basically, it looks like this:
> 
> --- linux-2.6-for-ingo.orig/include/linux/sched.h       2009-01-11 10:43:19.000000000 -0800
> +++ linux-2.6-for-ingo/include/linux/sched.h    2009-01-12 09:45:02.871247038 -0800
> @@ -1132,6 +1132,7 @@ struct task_struct {
> 
>         unsigned int policy;
>         cpumask_t cpus_allowed;
> +       cpumask_t save_cpus_allowed;
> 
> --- linux-2.6-for-ingo.orig/arch/x86/kernel/microcode_core.c    2009-01-12 09:05:36.000000000 -0800
> +++ linux-2.6-for-ingo/arch/x86/kernel/microcode_core.c 2009-01-12 09:49:19.315276144 -0800
> @@ -110,11 +110,10 @@ EXPORT_SYMBOL_GPL(ucode_cpu_info);
>  #ifdef CONFIG_MICROCODE_OLD_INTERFACE
>  static int do_microcode_update(const void __user *buf, size_t size)
>  {
> -       cpumask_t old;
>         int error = 0;
>         int cpu;
> 
> -       old = current->cpus_allowed;
> +       cpumask_copy(&current->save_cpus_allowed, &current->cpus_allowed);
> 
>         for_each_online_cpu(cpu) {
>                 struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
> @@ -122,7 +121,7 @@ static int do_microcode_update(const voi
>                 if (!uci->valid)
>                         continue;
> 
> -               set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu));
> +               set_cpus_allowed_ptr(current, cpumask_of(cpu));
>                 error = microcode_ops->request_microcode_user(cpu, buf, size);
>                 if (error < 0)
>                         goto out;
> @@ -130,7 +129,7 @@ static int do_microcode_update(const voi
>                         microcode_ops->apply_microcode(cpu);
>         }
>  out:
> -       set_cpus_allowed_ptr(current, &old);
> +       set_cpus_allowed_ptr(current, &current->save_cpus_allowed);
>         return error;
>  }
> 
> The primary concern is that there is only one temp, so I had also put in 
> a warning if it was already in use.  But the scope of where it's used is 
> very short-lived, so I don't know if a preempt_disable() is required, 
> but it seems the safe thing to do.

that's rather fragile. Fix work_on_cpu() instead? Why does it need to take 
the CPU-hotplug lock?

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ