January 2017 - Wine-devel - list.winehq.org

[v3 PATCH 00/10] x86: Enable User-Mode Instruction Prevention
by Ricardo Neri 01 Feb '17

01 Feb '17

This is v3 of this series. Again, it took me a while to complete the updates as support for virtual-8086 mode required extra rework. The two previous submissions can be found here [1] and here [2]. User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. There is a caveat, however. Certain applications rely on some of these instructions to function. An example of this are applications that use WineHQ[3]. For instance, these applications rely on sidt returning a non- accesible memory location[4]. During the discussions, it was proposed that the fault could be relied to the user-space and perform the emulation in user-mode. However, this would break existing applications until, for instance, they update to a new WineHQ version. However, this approach would require UMIP to be disabled by default. The concensus in this forum is to always enable it. This patchset initially treated tasks running in virtual-8086 mode as a special case. However, I received clarification that DOSEMU[5] does not support applications that use these instructions. It relies on WineHQ for this[6]. Furthermore, the applications for which the concern was raised run in protected mode [4]. This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values are located within a memory map hole in x86_64. For x86_32, the same values are used but truncated to 4 bytes. This is also the case for virtual- 8086 mode tasks, in which the base is truncated to 3 bytes. In all cases, the limit of the table is set to 0. * STR and SLDT return 0 as the segment selector. This looks appropriate since we are providing a dummy value as the base address of the global descriptor table. * 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. The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. While here, I fixed two small bugs that I found in the MPX implementation. This new library was also extended to handle 16-bit address encodings as those found in virtual-8086 mode tasks. A set of representative selftests for virtual-8086 mode tasks are included as part of this series. The tested uses use displacements, registers to indicate memory addresses as well as the use as registers as operands. Extensive test cases were performed to test the page fault that is emulated when memory to write the instructions results is not accesible[7], to tests almost all combinations of operands (ModRM, SiB, REX prefix and displacements) in protected mode[8] and to test almost all the combinations of operands in virtual-8086 mode[9]. If there is interest, this could could also be submitted for the kernel selftests. [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://www.winehq.org/ [4]. https://www.winehq.org/pipermail/wine-devel/2016-November/115320.html [5]. http://www.dosemu.org/ [6]. http://marc.info/?l=linux-kernel&m=147876798717927&w=2 [7]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-core/um… [8]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-core/um… [9]. https://github.com/01org/luv-yocto/blob/rneri/umip/meta-luv/recipes-kernel/… Thanks and BR, Ricardo Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added selftests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Cc: Andy Lutomirski <luto(a)kernel.org> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Brian Gerst <brgerst(a)gmail.com> Cc: Chen Yucong <slaoub(a)gmail.com> Cc: Chris Metcalf <cmetcalf(a)mellanox.com> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Fenghua Yu <fenghua.yu(a)intel.com> Cc: Huang Rui <ray.huang(a)amd.com> Cc: Jiri Slaby <jslaby(a)suse.cz> Cc: Jonathan Corbet <corbet(a)lwn.net> Cc: Michael S. Tsirkin <mst(a)redhat.com> Cc: Paul Gortmaker <paul.gortmaker(a)windriver.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: Vlastimil Babka <vbabka(a)suse.cz> Cc: Shuah Khan <shuah(a)kernel.org> Cc: Paolo Bonzini <pbonzini(a)redhat.com> Cc: Liang Z Li <liang.z.li(a)intel.com> Cc: x86(a)kernel.org Cc: linux-msdos(a)vger.kernel.org Ricardo Neri (10): x86/mpx: Do not use SIB index if index points to R/ESP x86/mpx: Fail decoding when SIB baseR/EBP is and no displacement is used x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel x86/insn-kernel: Add a function to obtain register offset in ModRM x86/insn-kernel: Add support to resolve 16-bit addressing encodings x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-kernel.h | 17 ++ arch/x86/include/asm/umip.h | 15 ++ arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 251 +++++++++++++++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-kernel.c | 344 ++++++++++++++++++++++++++ arch/x86/mm/mpx.c | 120 +-------- tools/testing/selftests/x86/entry_from_vm86.c | 39 ++- 14 files changed, 708 insertions(+), 122 deletions(-) create mode 100644 arch/x86/include/asm/insn-kernel.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-kernel.c -- 2.9.3

4 24

Re: wbemprox: Add a partial implementation of Win32_IP4RouteTable.
by Alexandre Julliard 31 Jan '17

31 Jan '17

Hans Leidekker <hans(a)codeweavers.com> writes: > Signed-off-by: Hans Leidekker <hans(a)codeweavers.com> > --- > dlls/wbemprox/builtin.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ > dlls/wbemprox/tests/query.c | 63 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 138 insertions(+) This doesn't build on Mingw: i686-w64-mingw32-gcc -c -o builtin.o ../../../wine/dlls/wbemprox/builtin.c -I. -I../../../wine/dlls/wbemprox \ -I../../include -I../../../wine/include -D__WINESRC__ -DUSE_WS_PREFIX -D_REENTRANT -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla \ -Wwrite-strings -Wpointer-arith -Wlogical-op -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer \ -g -O2 -fno-diagnostics-show-caret -D_WIN32 In file included from ../../../wine/dlls/wbemprox/builtin.c:38:0: ../../../wine/include/ws2tcpip.h:176:104: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration ../../../wine/include/ws2tcpip.h:178:107: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration ../../../wine/dlls/wbemprox/builtin.c: In function ‘fill_ip4routetable’: ../../../wine/dlls/wbemprox/builtin.c:2074:47: warning: implicit declaration of function ‘ntohl’ [-Wimplicit-function-declaration] ../.././../obj-elf64/tools/winegcc/winegcc -o wbemprox.dll -B../.././../obj-elf64/tools/winebuild \ --sysroot=../.. -b i686-w64-mingw32 -fasynchronous-unwind-tables -shared ../../../wine/dlls/wbemprox/wbemprox.spec \ builtin.o class.o main.o process.o qualifier.o query.o reg.o security.o service.o services.o \ table.o wbemlocator.o wql.tab.o wbemprox_r.res ../../dlls/winspool.drv/libwinspool.a \ ../../dlls/version/libversion.a ../../dlls/iphlpapi/libiphlpapi.a ../../dlls/dxgi/libdxgi.a \ ../../dlls/oleaut32/liboleaut32.a ../../dlls/ole32/libole32.a ../../dlls/advapi32/libadvapi32.a \ ../../dlls/user32/libuser32.a ../../dlls/gdi32/libgdi32.a ../../dlls/ws2_32/libws2_32.a \ ../../dlls/rpcrt4/librpcrt4.a ../../libs/port/libwine_port.a -lwine builtin.o: In function `fill_ip4routetable': /home/julliard/wine/build/obj-pe32/dlls/wbemprox/../../../wine/dlls/wbemprox/builtin.c:2074: undefined reference to `ntohl' /home/julliard/wine/build/obj-pe32/dlls/wbemprox/../../../wine/dlls/wbemprox/builtin.c:2076: undefined reference to `ntohl' collect2: error: ld returned 1 exit status winegcc: i686-w64-mingw32-gcc failed Makefile:638: recipe for target 'wbemprox.dll' failed -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] user32:add SIZEALL_BIG cursor, reusing SIZEALL resource Fixes bug#42303
by Alexandre Julliard 31 Jan '17

31 Jan '17

Anton Romanov <theli.ua(a)gmail.com> writes: > @@ -2227,6 +2227,7 @@ typedef struct tagCBTACTIVATESTRUCT > #define IDC_HAND MAKEINTRESOURCE(32649) > #define IDC_APPSTARTING MAKEINTRESOURCE(32650) > #define IDC_HELP MAKEINTRESOURCE(32651) > +#define IDC_SIZEALL_BIG MAKEINTRESOURCE(32654) There's no such definition in Windows. > @@ -1011,6 +1011,7 @@ > #define OCR_NO 32648 > #define OCR_HAND 32649 > #define OCR_APPSTARTING 32650 > +#define OCR_SIZEALL_BIG 32654 This is already defined as OCR_RDR2DIM. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] wined3d: Implement buffer texture range.
by Guillaume Charifi 31 Jan '17

31 Jan '17

Thank you for your detailed answer :) Le 31/01/2017 à 00:19, Józef Kucia a écrit : > I'll explain in more details to encourage you ;-) > > On Sun, Jan 29, 2017 at 1:12 PM, Józef Kucia <joseph.kucia(a)gmail.com> wrote: >> Please add a test. > Adding a test should be very easy. In test_buffer_srv() everything is > ready to add a test with an offset. You can use "rgba16_buffer" as > data, set size to {2.0f, 2.0f} or {1.0f, 1.0f}, and test with > different values of "srv_first_element" and "srv_element_count". Try > to come up with interesting test cases and test edge cases. Looks easier than I thought, indeed. I'll be doing this. > >> On Sun, Jan 29, 2017 at 12:25 AM, Guillaume Charifi >> <guillaume.charifi(a)sfr.fr> wrote: >>> + GL_EXTCALL(glTexBufferRange(GL_TEXTURE_BUFFER, view_format->glInternal, buffer->buffer_object, >>> + desc->u.buffer.start_idx, desc->u.buffer.count)); >> This is wrong. > glTexBufferRange() accepts offsets and sizes in bytes, but in D3D11 > offsets and sizes for typed buffers are expressed as the number of > elements. You have to multiply "start_idx" and "count" by > "view_format->byte_count". A test would also help you to find this > mistake. Oh, I see, I'm fixing it. > > Another advice: start small. It's better to send a few well tested > patches and get them accepted first. > > I'll hope to see some patches from you committed soon. > > Cheers, > Józef >

1 0

Re: [PATCH 2/2] winspool/tests: Test for LastError also on success of AddMonitor
by Huw Davies 31 Jan '17

31 Jan '17

On Mon, Jan 30, 2017 at 07:43:35PM +0100, Detlef Riekenberg wrote: > dlls/winspool.drv/tests/info.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/winspool.drv/tests/info.c b/dlls/winspool.drv/tests/info.c > index f6527c5..602e935 100644 > --- a/dlls/winspool.drv/tests/info.c > +++ b/dlls/winspool.drv/tests/info.c > @@ -435,6 +435,12 @@ static void test_AddMonitor(void) > SetLastError(MAGIC_DEAD); > res = AddMonitorA(NULL, 2, (LPBYTE) &mi2a); > ok(res, "returned %d with %d (expected '!= 0')\n", res, GetLastError()); > + if (res) > + { > + /* Some apps depend on the result of GetLastError() */ > + res = GetLastError(); > + ok(res == ERROR_SUCCESS, "got 0x%x/%u (expected ERROR_SUCCESS)\n", res, res); > + } There's no need to have this test conditional on res. Also, don't reassign res to the error, just use GetLastError() directly. And just print the error in decimal. Huw.

1 0

Re: [PATCH 1/2] localspl: Clear LastError on success in AddMonitor
by Huw Davies 31 Jan '17

31 Jan '17

On Mon, Jan 30, 2017 at 07:43:34PM +0100, Detlef Riekenberg wrote: > dlls/localspl/provider.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/dlls/localspl/provider.c b/dlls/localspl/provider.c > index f359b73..f370b9f 100644 > --- a/dlls/localspl/provider.c > +++ b/dlls/localspl/provider.c > @@ -1541,6 +1541,9 @@ static BOOL WINAPI fpAddMonitor(LPWSTR pName, DWORD Level, LPBYTE pMonitors) > RegCloseKey(hentry); > } > > + if (res) > + SetLastError(ERROR_SUCCESS); /* Monitor installer depend on this */ > + > RegCloseKey(hroot); > return (res); > } I think unconditionally resetting the error after monitor_unload() would be cleaner. Huw.

1 0

Re: comctl32: Fix some more spec file entries
by Huw Davies 31 Jan '17

31 Jan '17

On Sun, Jan 29, 2017 at 09:58:36PM +0100, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/comctl32/comctl32.spec | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/comctl32/comctl32.spec b/dlls/comctl32/comctl32.spec > index c3e9167..dbfa123 100644 > --- a/dlls/comctl32/comctl32.spec > +++ b/dlls/comctl32/comctl32.spec > @@ -93,7 +93,7 @@ > 382 stdcall -noname SmoothScrollWindow(ptr) > 383 stub -noname DoReaderMode > 384 stdcall -noname SetPathWordBreakProc(ptr long) > -385 stdcall -ordinal DPA_EnumCallback(long long ptr) > +385 stdcall -ordinal DPA_EnumCallback(long ptr ptr) Should be: 385 stdcall -ordinal DPA_EnumCallback(ptr ptr ptr) Huw.

4 3

Re: [PATCH] winealsa.drv: Fix handling of system real time MIDI messages
by Austin English 31 Jan '17

31 Jan '17

On Tue, Jan 31, 2017 at 12:16 AM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > MIDI spec at http://www.somascape.org/midi/tech/spec.html state that system real time messages "... are each single byte messages, with no following data bytes...", currently wine is prepending SYSEX header generating invalid messages. > > A comparison before and after patch using gmidimonitor as ouput MIDI port: > http://alexa.pro.br/~bruno/wine/midi.png > > I believe the patch to be correct, I don't own any MIDI hardware to test but gmidimonitor seems good enough. Do wineoss / winepulse need similar fixes? -- -Austin GPG: 14FB D7EA A041 937B

2 1

Re: [PATCH] regedit: When processing 3.1 ignore any lines that don't begin with HKCR.
by Nikolay Sivov 31 Jan '17

31 Jan '17

On 31.01.2017 5:51, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > programs/regedit/regproc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/programs/regedit/regproc.c b/programs/regedit/regproc.c > index c055c10..b6de728 100644 > --- a/programs/regedit/regproc.c > +++ b/programs/regedit/regproc.c > @@ -616,8 +616,9 @@ static void processRegEntry31(WCHAR *line) > int res; > > static WCHAR empty[] = {0}; > + static WCHAR hkcr[] = {'H','K','E','Y','_','C','L','A','S','S','E','S','_','R','O','O','T',0}; > > - if (line[0] == '\0') return; > + if (memcmp(line, hkcr, sizeof(hkcr) - sizeof(WCHAR))) return; > > /* get key name */ > while (line[key_end] && !isspaceW(line[key_end])) key_end++; > Is it guaranteed 'line' is a least that large? Also you can remove terminating 0 from hkcr to avoid sizeof(WCHAR) subtraction.

2 1

Re: [PATCH 05/10] wined3d: Add compute shaders cb/samplers state updates.
by Guillaume Charifi 30 Jan '17

30 Jan '17

Thank you for your feedback. WineD3D/D3D11/OpenGL are complex APIs with lots of tricky details. I know my implementation is far from perfect, but it seems that you learn better by working, trying, so I do. I have a basic but (likely) working tessellation implementation and I'll submit it soon. But it's quite dirty, so I expect feedbacks to improve it (if you haven't done it locally yet). Btw, I think you should write down somewhere who is currently working on what at Crossover, to avoid useless duplicated work. <div>-------- Message d'origine --------</div><div>De : Józef Kucia <joseph.kucia(a)gmail.com> </div><div>Date :30/01/2017 14:13 (GMT+01:00) </div><div>À : wine-devel <wine-devel(a)winehq.org> </div><div>Cc : Guillaume Charifi <guillaume.charifi(a)sfr.fr> </div><div>Objet : Re: [PATCH 05/10] wined3d: Add compute shaders cb/samplers state updates. </div><div> </div>FWIW, I have a patch series adding basic support for compute shaders in my patch queue. I should start submitting the series when my SM5 shader instructions series from wine-patches is committed. I've skimmed over your patches though. I'll share some high-level comments. On Sun, Jan 29, 2017 at 2:43 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/context.c | 1 + > dlls/wined3d/state.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c > index ca73bcc..eaae0b8 100644 > --- a/dlls/wined3d/context.c > +++ b/dlls/wined3d/context.c > @@ -3382,6 +3382,7 @@ static void context_bind_shader_resources(struct wined3d_context *context, const > { > {WINED3D_SHADER_TYPE_PIXEL, 0, MAX_FRAGMENT_SAMPLERS}, > {WINED3D_SHADER_TYPE_VERTEX, MAX_FRAGMENT_SAMPLERS, MAX_VERTEX_SAMPLERS}, > + {WINED3D_SHADER_TYPE_COMPUTE, 0, MAX_COMPUTE_SAMPLERS}, > }; I don't think that adding a proper support for binding compute shader resources is that simple.

2 1