linux-kernel - Re: [block IO crash] Re: 2.6.39-rc5-git2 boot crashs

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.LFD.2.02.1105041417330.3005@ionos>
Date:	Wed, 4 May 2011 15:00:37 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Tejun Heo <tj@...nel.org>
cc:	Pekka Enberg <penberg@...nel.org>, Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jens Axboe <axboe@...nel.dk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	werner <w.landgraf@...ru>, "H. Peter Anvin" <hpa@...or.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Christoph Lameter <cl@...ux.com>
Subject: Re: [block IO crash] Re: 2.6.39-rc5-git2 boot crashs

On Wed, 4 May 2011, Tejun Heo wrote:
> > > And that code runs with preemption enabled. So when the task gets
> > > preempted _BEFORE_ it has actuallty written back the data, then the
> > > race window is wide open.
> 
> Hmmm... if it's a race caused by preemtion enabled where it shouldn't
> be, it's most likely the wrong type of this_cpu_cmpxchg_double() being
> used in SLUB?  ie. __this_cpu_cmpxchg_double() where it should have
> been this_cpu_cmpxchg_double()?  Christoph?

No, the problem is that ELAN prevents the cmpxchg8b, but keeps
CONFIG_CMPXCHG_LOCAL=y which then results in the unprotected code for
the following reason:

this_cpu_cmpxchg_double() 

-> __pcpu_double_call_return_bool 

-> this_cpu_cmpxchg_double_4

Which on x86 expands to 

-> percpu_cmpxchg8b_double() when CONFIG_X86_CMPXCHG64=y

With CONFIG_X86_CMPXCHG64=n it expands to the default:

_this_cpu_generic_cmpxchg_double() in linux/percpu.h

#define _this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)        \
({                                                                      \
        int ret__;                                                      \
        preempt_disable();                                              \
        ret__ = __this_cpu_generic_cmpxchg_double(pcp1, pcp2,           \
                        oval1, oval2, nval1, nval2);                    \
        preempt_enable();                                               \
        ret__;                                                          \
})

And:

#define __this_cpu_generic_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2)       \
({                                                                      \
        int __ret = 0;                                                  \
        if (__this_cpu_read(pcp1) == (oval1) &&                         \
                         __this_cpu_read(pcp2)  == (oval2)) {           \
                __this_cpu_write(pcp1, (nval1));                        \
                __this_cpu_write(pcp2, (nval2));                        \
                __ret = 1;                                              \
        }                                                               \
        (__ret);                                                        \
})

So now that failing config has CONFIG_PREEMPT=n which makes
preempt_disable / enable a nop. 

So preemption is not the problem, but what about interrupts and
softirqs ?

So the question is whether CMPXCHG_LOCAL for x86 wants to depend on
X86_CMPXCHG64.

The other solution is to use irqsafe_cpu_cmpxchg_double() instead of
this_cpu_cmpxchg_double() in slub.c. 

This will not hurt the X86_CMPXCHG64=y case, but keep the expansion to
the above __this_cpu_generic_cmpxchg_double working. 

Which makes me even wonder some more whether we need that whole
CMPXCHG_LOCAL #ifdeffery in slub.c at all.

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/