Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention

2017-04-01 Thread H. Peter Anvin
,linux-msdos@vger.kernel.org,wine-de...@winehq.org
From: h...@zytor.com
Message-ID: <3fd12652-aa83-4d73-9914-bba089e58...@zytor.com>

On April 1, 2017 6:08:43 AM PDT, Stas Sergeev  wrote:
>30.03.2017 08:14, Ricardo Neri пишет:
>>> You know the wine's
>>> requirements now - they are very small. And
>>> dosemu doesn't need anything at all but smsw.
>>> And even smsw is very rare.
>> But emulation is still needed for SMSW, right?
> Likely so.
> If you want, I can enable the logging of this command
> and see if it is used by some of the DOS programs I have.
 It would be great if you could do that, if you don't mind.
>>> OK, scheduled to the week-end.
>>> I'll let you know.
>> Thanks!
>OK, done the testing.
>It appears smsw is used in v86 by windows-3.1 and dos4gw
>at the very least, and these are the "major" apps. So doing
>without a fixup in v86 will not go unnoticed. Unfortunately
>this also means that KVM-vm86 should be properly tested.
>I have also found a weird program that does SGDT under
>v86. This causes "ERROR: SGDT not implemented" under
>dosemu, but the prog still works fine as it obviously does
>not care about the results. This app can easily be broken
>of course, if that makes any sense (likely not).

Using SMSW to detect v86 mode is relatively common.  pushf hides the VM flag, 
but SMSW is available, providing the v86 virtualization hole.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 15/17] x86/traps: Fixup general protection faults caused by UMIP

2017-02-24 Thread H. Peter Anvin
Luck 
From: h...@zytor.com
Message-ID: 

On February 24, 2017 11:36:19 AM PST, Ricardo Neri 
 wrote:
>On Fri, 2017-02-24 at 11:11 -0800, Andy Lutomirski wrote:
>> > In a previous version Andy Lutomirsky suggested that
>> > if (user_mode(regs) && (fixup_umip_exception(regs) == 0))
>> >
>> > was easier to read :). Although at the time fixup_umip_exception
>> > returned a numeric value. Now it only returns true/false for
>> > successful/failed emulation. If with true/false not comparing to
>> true
>> > makes it easier to read, I will make the change.
>> 
>> I think == true is silly :)
>
>Then I'll make the change.
>
>Thanks and BR,
>Ricardo

It's worse than silly, it is potentially toxic.

true is a macro which it's defined as 1.  Thus

 foo == true

... doesn't actually mean what people *think* it does, which is roughly the 
same thing as

!!foo

However, if foo is not a boolean, this is *very* different; consider if foo is 
2.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings

2017-01-25 Thread H. Peter Anvin
Buchbinder ,Colin Ian King 
,Lorenzo Stoakes ,Qiaowei Ren 
,Arnaldo Carvalho de Melo ,Adrian 
Hunter ,Kees Cook ,Thomas 
Garnier ,Dmitry Vyukov 
From: h...@zytor.com
Message-ID: <18e8698f-6c60-4b98-ae73-c371184c5...@zytor.com>

On January 25, 2017 1:58:27 PM PST, Andy Lutomirski  wrote:
>On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri
> wrote:
>> Tasks running in virtual-8086 mode will use 16-bit addressing form
>> encodings as described in the Intel 64 and IA-32 Architecture
>Software
>> Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing
>encodings
>> differ in several ways from the 32-bit/64-bit addressing form
>encodings:
>> the r/m part of the ModRM byte points to different registers and, in
>some
>> cases, addresses can be indicated by the addition of the value of two
>> registers. Also, there is no support for SiB bytes. Thus, a separate
>> function is needed to parse this form of addressing.
>>
>> Furthermore, virtual-8086 mode tasks will use real-mode addressing.
>This
>> implies that the segment selectors do not point to a segment
>descriptor
>> but are used to compute logical addresses. Hence, there is a need to
>> add support to compute addresses using the segment selectors. If
>segment-
>> override prefixes are present in the instructions, they take
>precedence.
>>
>> Lastly, it is important to note that when a tasks is running in
>virtual-
>> 8086 mode and an interrupt/exception occurs, the CPU pushes to the
>stack
>> the segment selectors for ds, es, fs and gs. These are accesible via
>the
>> struct kernel_vm86_regs rather than pt_regs.
>>
>> Code for 16-bit addressing encodings is likely to be used only by
>virtual-
>> 8086 mode tasks. Thus, this code is wrapped to be built only if the
>> option CONFIG_VM86 is selected.
>
>That's not true.  It's used in 16-bit protected mode, too.  And there
>are (ugh!) six possibilities:
>
> - Normal 32-bit protected mode.  This should already work.
> - Normal 64-bit protected mode.  This should also already work.  (I
>forget whether a 16-bit SS is either illegal or has no effect in this
>case.)
> - Virtual 8086 mode
> - Normal 16-bit protected mode, used by DOSEMU and Wine.  (16-bit CS,
>16-bit address segment)
> - 16-bit CS, 32-bit address segment.  IIRC this might be used by some
>32-bit DOS programs to call BIOS.
>- 32-bit CS, 16-bit address segment.  I don't know whether anything
>uses this.
>
>I don't know if anything you're doing cares about SS's, DS's, etc.
>size, but I suspect you'll need to handle 16-bit CS.

Only the CS bitness matters for the purpose of addressing modes; the SS bitness 
(which has no effect in 64-bit mode) only matters for implicit stack references 
unless I'm completely out to sea.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [v3 PATCH 07/10] x86: Add emulation code for UMIP instructions

2017-01-25 Thread H. Peter Anvin
On 01/25/17 12:23, Ricardo Neri wrote:
> + case UMIP_SMSW:
> + dummy_value = CR0_STATE;

Unless the user space process is running in 64-bit mode this value
should be & 0x.  I'm not sure if we should even support fixing up
UMIP instructions in 64-bit mode.

Also, please put an explicit /* fall through */ here.

> + /*
> +  * These two instructions return a 16-bit value. We return
> +  * all zeros. This is equivalent to a null descriptor for
> +  * str and sldt.
> +  */
> + case UMIP_SLDT:
> + case UMIP_STR:
> + /* if operand is a register, it is zero-extended*/
> + if (X86_MODRM_MOD(insn->modrm.value) == 3) {
> + memset(data, 0, insn->opnd_bytes);
> + *data_size = insn->opnd_bytes;
> + /* if not, only the two least significant bytes are copied */
> + } else {
> + *data_size = 2;
> + }
> + memcpy(data, _value, sizeof(dummy_value));
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;


> +bool fixup_umip_exception(struct pt_regs *regs)
> +{
> + struct insn insn;
> + unsigned char buf[MAX_INSN_SIZE];
> + /* 10 bytes is the maximum size of the result of UMIP instructions */
> + unsigned char dummy_data[10] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
> +#ifdef CONFIG_X86_64
> + int x86_64 = user_64bit_mode(regs);
> +#else
> + int x86_64 = 0;
> +#endif

Again, could we simply do:

if (user_64bit_mode(regs))
return false;

or are there known users of these instructions *in 64-bit mode*?

-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [v3 PATCH 00/10] x86: Enable User-Mode Instruction Prevention

2017-01-25 Thread H. Peter Anvin
On 01/25/17 12:23, Ricardo Neri wrote:
>  * SMSW returns the value with which the CR0 register is programmed in
>head_32/64.S at boot time. This is, the following bits are enabed:
>CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for
>Extension Type, which will always be 1 in recent processors with UMIP;
>CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment
>Mask. Additionally, in x86_64, CR0.31 for Paging is set.

SMSW only returns CR0[15:0], so the reference here to CR0[31:16] seems odd.

-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html