| 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
| ||
|
Message-ID: <AANLkTi=NqF=ue3-EghrNVrTn6Cnd6_6i4=WXOJOrm7jC@mail.gmail.com>
Date: Wed, 8 Sep 2010 12:55:45 +0100
From: Nick Lowe <nick.lowe@...il.com>
To: Hans-Peter Jansen <hpj@...la.net>, linux-kernel@...r.kernel.org
Subject: Re: AMD Geode NOPL emulation for kernel 2.6.36-rc2
I also really don't think that you can get away with emulating it.
You're changing the contract of NOPL to something non-deterministic.
How is that acceptable?
What about the situation where these NOPs have been used to
synchronise? With the change in contract, you subtly break execution
there.
That's far worse than it not running in the first place, surely?
Nick
On Wed, Sep 8, 2010 at 12:34 PM, Nick Lowe <nick.lowe@...il.com> wrote:
> Hi,
>
> If a process crashes because of NOPL usage, you get the feedback loop to fix it.
>
> The beauty of open source is that we can recompile to correct the bad
> assumption; the latest version of binutils (GAS) finally corrects the
> bad semantic that generic i686 includes NOPL.
>
> http://sourceware.org/ml/binutils-cvs/2010-08/msg00057.html
> http://gcc.gnu.org/ml/gcc/2010-08/msg00194.html
>
> That fixes the vast vast majority of cases, so I really don't see the
> need for this.
>
> I've posted a RFC on "Promoting Crusoe and Geode Processors to i686
> Status" if you have any thoughts on that?
>
> Cheers,
>
> Nick
>
> On Wed, Sep 8, 2010 at 10:15 AM, Hans-Peter Jansen <hpj@...la.net> wrote:
>> On Tuesday 07 September 2010, 17:57:17 Nick Lowe wrote:
>>> In my opinion, this patch shouldn't be seen as a solution in the
>>> slightest. To me, it's a ugly hack and a kludge at best!
>>>
>>> Abstractly a NOP is meant to be exactly as it says on the tin, a No
>>> Operation.
>>>
>>> It's meant to do nothing at all - for a predefined number of clock
>>> cycles. A NOP is commonly used for timing purposes, this completely
>>> breaks that contract surely?
>>>
>>> NOPL is not standard i686, it was undocumented and has just been de facto
>>> supported since the Pentium Pro.
>>>
>>> The solution is surely to ensure that when something is compiled for
>>> generic i686, NOPL is nowhere to be seen... Any compiler that does
>>> otherwise is broken.
>>
>> From a user POV, this patch done right is the better solution, because you
>> cannot ensure, that all code _generally_ comply to your *policy*. This is
>> not a kernel only issue! Keep processes crashing is worse IMHO.
>>
>> With properly done, I mean:
>> - runtime check, if iopl opcode exists
>> - emulate, if not
>>
>> Hence, don't bind this to nor label it after a certain cpu model.
>>
>> Remember: we do not live in a perfect world.
>>
>> Pete
>>
>>> For example, until recently, the basic semantics for choosing the NOP
>>> sequence were completely wrong in binutils. This has, finally, been
>>> fixed!
>>>
>>> http://sourceware.org/ml/binutils-cvs/2010-08/msg00057.html
>>> http://gcc.gnu.org/ml/gcc/2010-08/msg00194.html
>>>
>>> On 27 Aug 2010, at 19:10, Matteo Croce wrote:
>>> > Hi,
>>> > I have received many mails asking to refresh the patch so I decided to
>>> > post them on the public mailing list
>>> > In this version such feature is configurable via a kernel config option
>>> >
>>> > Signed-off-by: Matteo Croce <matteo@...nwrt.org>
>>> >
>>> > --- a/arch/x86/kernel/Makefile 2010-08-27 00:27:50.101261000 +0200
>>> > +++ b/arch/x86/kernel/Makefile 2010-08-27 00:31:11.391261002 +0200
>>> > @@ -88,6 +88,8 @@
>>> > obj-$(CONFIG_APB_TIMER) += apb_timer.o
>>> >
>>> > obj-$(CONFIG_K8_NB) += k8.o
>>> > +obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
>>> > +obj-$(CONFIG_GEODE_NOPL) += nopl_emu.o
>>> > obj-$(CONFIG_DEBUG_RODATA_TEST) += test_rodata.o
>>> > obj-$(CONFIG_DEBUG_NX_TEST) += test_nx.o
>>> >
>>> > --- a/arch/x86/kernel/cpu/amd.c 2010-08-27 00:27:50.161261000 +0200
>>> > +++ b/arch/x86/kernel/cpu/amd.c 2010-08-27 00:34:13.371261000 +0200
>>> > @@ -137,11 +137,15 @@
>>> > return;
>>> > }
>>> >
>>> > +#ifdef CONFIG_GEODE_NOPL
>>> > if (c->x86_model == 10) {
>>> > - /* AMD Geode LX is model 10 */
>>> > - /* placeholder for any needed mods */
>>> > + /* Geode only lacks the NOPL instruction to be i686,
>>> > + but we can promote it to a i686 class cpu
>>> > + and emulate NOPLs in the exception handler*/
>>> > + boot_cpu_data.x86 = 6;
>>> > return;
>>> > }
>>> > +#endif
>>> > }
>>> >
>>> > static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c)
>>> > --- a/arch/x86/kernel/entry_32.S 2010-08-27 00:27:50.051261000 +0200
>>> > +++ b/arch/x86/kernel/entry_32.S 2010-08-27 00:57:35.531261000 +0200
>>> > @@ -978,7 +978,11 @@
>>> > RING0_INT_FRAME
>>> > pushl $0
>>> > CFI_ADJUST_CFA_OFFSET 4
>>> > +#ifdef CONFIG_GEODE_NOPL
>>> > + pushl $do_nopl_emu
>>> > +#else
>>> > pushl $do_invalid_op
>>> > +#endif
>>> > CFI_ADJUST_CFA_OFFSET 4
>>> > jmp error_code
>>> > CFI_ENDPROC
>>> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
>>> > +++ b/arch/x86/kernel/nopl_emu.c 2010-08-27 00:27:57.881261002 +0200
>>> > @@ -0,0 +1,110 @@
>>> > +/*
>>> > + * linux/arch/x86/kernel/nopl_emu.c
>>> > + *
>>> > + * Copyright (C) 2002 Willy Tarreau
>>> > + * Copyright (C) 2010 Matteo Croce
>>> > + */
>>> > +
>>> > +#include <asm/processor.h>
>>> > +#include <asm/traps.h>
>>> > +
>>> > +/* This code can be used to allow the AMD Geode to hopefully correctly
>>> > execute + * some code which was originally compiled for an i686, by
>>> > emulating NOPL, + * the only missing i686 instruction in the CPU
>>> > + *
>>> > + * Willy Tarreau <willy@...a-x.org>
>>> > + * Matteo Croce <technoboy85@...il.co>
>>> > + */
>>> > +
>>> > +static inline int do_1f(u8 *ip)
>>> > +{
>>> > + int length = 3;
>>> > + switch (*ip) {
>>> > + case 0x84:
>>> > + if (!ip[5])
>>> > + length++;
>>> > + else
>>> > + return 0;
>>> > + case 0x80:
>>> > + if (!ip[4] && !ip[3])
>>> > + length += 2;
>>> > + else
>>> > + return 0;
>>> > + case 0x44:
>>> > + if (!ip[2])
>>> > + length++;
>>> > + else
>>> > + return 0;
>>> > + case 0x40:
>>> > + if (!ip[1])
>>> > + length++;
>>> > + else
>>> > + return 0;
>>> > + case 0x00:
>>> > + return length;
>>> > + }
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static inline int do_0f(u8 *ip)
>>> > +{
>>> > + if (*ip == 0x1f)
>>> > + return do_1f(ip + 1);
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static inline int do_66(u8 *ip)
>>> > +{
>>> > + if (*ip == 0x90)
>>> > + return 2;
>>> > + if (*ip == 0x0f) {
>>> > + int res = do_0f(ip + 1);
>>> > + if (res)
>>> > + return res + 1;
>>> > + else
>>> > + return 0;
>>> > + }
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static inline int do_start(u8 *ip)
>>> > +{
>>> > + if (*ip == 0x0f)
>>> > + return do_0f(ip + 1);
>>> > + if (*ip == 0x66)
>>> > + return do_66(ip + 1);
>>> > + return 0;
>>> > +}
>>> > +
>>> > +/* [do_nopl_emu] is called by exception 6 after an invalid opcode has
>>> > been + * encountered. It will try to emulate it by doing nothing,
>>> > + * and will send a SIGILL or SIGSEGV to the process if not possible.
>>> > + * the NOPL can have variable length opcodes:
>>> > +
>>> > +bytes number opcode
>>> > + 2 66 90
>>> > + 3 0f 1f 00
>>> > + 4 0f 1f 40 00
>>> > + 5 0f 1f 44 00 00
>>> > + 6 66 0f 1f 44 00 00
>>> > + 7 0f 1f 80 00 00 00 00
>>> > + 8 0f 1f 84 00 00 00 00 00
>>> > + 9 66 0f 1f 84 00 00 00 00 00
>>> > +*/
>>> > +void do_nopl_emu(struct pt_regs *regs, long error_code)
>>> > +{
>>> > + u8 *ip = (u8 *)instruction_pointer(regs);
>>> > + int res = do_start(ip);
>>> > +
>>> > + if (res) {
>>> > + int i = 0;
>>> > + do {
>>> > + ip += res;
>>> > + i++;
>>> > + res = do_start(ip);
>>> > + } while(res);
>>> > + printk(KERN_DEBUG "geode_nopl: emulated %d instructions\n", i);
>>> > + regs->ip = (typeof(regs->ip))ip;
>>> > + } else
>>> > + do_invalid_op(regs, error_code);
>>> > +}
>>> >
>>> >
>>> > --
>>> > Matteo Croce
>>> > OpenWrt developer
>>> > Â _______Â Â Â Â Â Â Â Â Â Â Â ________Â Â Â Â __
>>> > Â |Â Â Â Â |.-----.-----.-----.|Â |Â |Â |.----.|Â |_
>>> > Â |Â Â -Â Â ||Â _Â |Â -__|Â Â Â ||Â |Â |Â ||Â Â _||Â Â _|
>>> > Â |_______||Â Â __|_____|__|__||________||__|Â |____|
>>> > Â Â Â Â Â |__| W I R E L E S SÂ Â F R E E D O M
>>> > Â KAMIKAZE (bleeding edge) ------------------
>>> >  * 10 oz Vodka    Shake well with ice and strain
>>> >  * 10 oz Triple sec mixture into 10 shot glasses.
>>> >  * 10 oz lime juice Salute!
>>> > Â ---------------------------------------------------
>>> > --
>>> > 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/
>>>
>>> --
>>> 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/
>>
>>
>>
>
--
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