linux-kernel - Re: [PATCH] mm: Fix boot crash in mm

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: <BANLkTikHejgEyz9LfJ962Bu89vn1cBP+WQ@mail.gmail.com>
Date:	Sun, 29 May 2011 09:22:49 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>, linux-mm@...ck.org
Subject: Re: [PATCH] mm: Fix boot crash in mm_alloc()

On Sun, May 29, 2011 at 12:22 AM, Ingo Molnar <mingo@...e.hu> wrote:
>
> Would be nice to get the fix below into -rc1 as well, it triggers
> rather easily on bootup when CONFIG_CPUMASK_OFFSTACK is turned on.

Looking at that commit de03c72cfce5, it looks odd in other ways too.

For example, it looks like mm_cpumask is always initialized to zero.
That's a bit odd, isn't it, since it *used* to be initialized
statically with this:

-       .cpu_vm_mask    = CPU_MASK_ALL,

which is rather different from zero.

Now, I'm sure the init mm_cpumask doesn't really matter, but I'd have
expected a commentary about it.

I also wonder if that whole conversion to cpumask_var_t was worth it,
since clearly it wasn't very well tested. It results in an extra
allocation at fork() time for the many-cpu case, and I do get the
feeling that we would have been better off keeping the cpumask inside
the mm_struct. Moving it to the end of mm_struct makes sense for the
many-cpu case, but at the same time I end up wondering what it does to
the switch_mm() cache behavior. (And perhaps the TLB flush IPI cache
activity).

Ho humm. I have this suspicion that that whole patch wasn't fully
thought out, and that I should revert it rather than fix the oops.

Or, in fact, we could just do something like the attached (UNTESTED!)
which does the whole "move big allocation to end, but keep the
cpumask_var_t at the beginning, and don't do any extra allocations"
thing.

NOTE NOTE NOTE! Not only is the attached patch untested, but please
see the added FIXME comment about the whole mm_struct
kmem_cache_create(). Right now we always allocate the whole
maximum-sized bitmap.

Comments?

                          Linus

View attachment "patch.diff" of type "text/x-patch" (4695 bytes)