June 2017 - Wine-Devel - list.winehq.org

Re: [PATCH v7 20/26] x86/cpufeature: Add User-Mode Instruction Prevention definitions

by Borislav Petkov

On Fri, May 05, 2017 at 11:17:18AM -0700, Ricardo Neri wrote: > User-Mode Instruction Prevention is a security feature present in new > Intel processors that, when set, prevents the execution of a subset of > instructions if such instructions are executed in user mode (CPL > 0). > Attempting to execute such instructions causes a general protection > exception. > > The subset of instructions comprises: > > * SGDT - Store Global Descriptor Table > * SIDT - Store Interrupt Descriptor Table > * SLDT - Store Local Descriptor Table > * SMSW - Store Machine Status Word > * STR - Store Task Register > > This feature is also added to the list of disabled-features to allow > a cleaner handling of build-time configuration. > > Cc: Andy Lutomirski <luto(a)kernel.org> > Cc: Andrew Morton <akpm(a)linux-foundation.org> > Cc: H. Peter Anvin <hpa(a)zytor.com> > 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: Shuah Khan <shuah(a)kernel.org> > Cc: Vlastimil Babka <vbabka(a)suse.cz> > Cc: Tony Luck <tony.luck(a)intel.com> > Cc: Paolo Bonzini <pbonzini(a)redhat.com> > Cc: Liang Z. Li <liang.z.li(a)intel.com> > Cc: Alexandre Julliard <julliard(a)winehq.org> > Cc: Stas Sergeev <stsp(a)list.ru> > Cc: x86(a)kernel.org > Cc: linux-msdos(a)vger.kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > --- > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/disabled-features.h | 8 +++++++- > arch/x86/include/uapi/asm/processor-flags.h | 2 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) Reviewed-by: Borislav Petkov <bp(a)suse.de> -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 7, 2017

Re: [PATCH] kernel32: Don't limit physical memory size even if not large address aware.

by Zebediah Figura

On 06/07/2017 01:19 PM, Sebastian Lackner wrote: > On 07.06.2017 19:39, Zebediah Figura wrote: >> From testing on Windows 7, it appears Windows doesn't do this either. Also, >> the virtual size is already limited in ntdll, so that check is unnecessary >> as well. >> >> This fixes a bug with sndrec32.exe giving OOM errors when recording. >> >> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >> --- >> dlls/kernel32/heap.c | 10 ---------- >> 1 file changed, 10 deletions(-) > > Are you sure you tested it correctly (with a binary that does not have > IMAGE_FILE_LARGE_ADDRESS_AWARE set)? And what about Adobe Illustrator 8 > which depends on this, according to the comment in the source code? > > Regards, > Sebastian > I tested the same binary with IMAGE_FILE_LARGE_ADDRESS_AWARE set and unset; the only values that differed were the size of the virtual memory. The comment would seem to imply that Adobe Illustrator 8 requires only that the page size not be modified, unless I'm misreading it.

June 7, 2017

Re: [PATCH] kernel32: Don't limit physical memory size even if not large address aware.

by Sebastian Lackner

On 07.06.2017 19:39, Zebediah Figura wrote: > From testing on Windows 7, it appears Windows doesn't do this either. Also, > the virtual size is already limited in ntdll, so that check is unnecessary > as well. > > This fixes a bug with sndrec32.exe giving OOM errors when recording. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/kernel32/heap.c | 10 ---------- > 1 file changed, 10 deletions(-) Are you sure you tested it correctly (with a binary that does not have IMAGE_FILE_LARGE_ADDRESS_AWARE set)? And what about Adobe Illustrator 8 which depends on this, according to the comment in the source code? Regards, Sebastian

June 7, 2017

Re: [PATCH v7 18/26] x86/insn-eval: Add support to resolve 16-bit addressing encodings

by Borislav Petkov

On Fri, May 05, 2017 at 11:17:16AM -0700, Ricardo Neri wrote: > Tasks running in virtual-8086 mode or in protected mode with code > segment descriptors that specify 16-bit default address sizes via the > D bit 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: ModRM.rm points to different > registers and, in some cases, effective addresses are 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. > > A couple of functions are introduced. get_reg_offset_16() obtains the > offset from the base of pt_regs of the registers indicated by the ModRM > byte of the address encoding. get_addr_ref_16() computes the linear > address indicated by the instructions using the value of the registers > given by ModRM as well as the base address of the segment. > > Cc: Dave Hansen <dave.hansen(a)linux.intel.com> > Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> > Cc: Colin Ian King <colin.king(a)canonical.com> > Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> > Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> > Cc: Arnaldo Carvalho de Melo <acme(a)redhat.com> > Cc: Masami Hiramatsu <mhiramat(a)kernel.org> > Cc: Adrian Hunter <adrian.hunter(a)intel.com> > Cc: Kees Cook <keescook(a)chromium.org> > Cc: Thomas Garnier <thgarnie(a)google.com> > Cc: Peter Zijlstra <peterz(a)infradead.org> > Cc: Borislav Petkov <bp(a)suse.de> > Cc: Dmitry Vyukov <dvyukov(a)google.com> > Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> > Cc: x86(a)kernel.org > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > --- > arch/x86/lib/insn-eval.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 155 insertions(+) > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 9822061..928a662 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -431,6 +431,73 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, > } > > /** > + * get_reg_offset_16 - Obtain offset of register indicated by instruction Please end function names with parentheses. > + * @insn: Instruction structure containing ModRM and SiB bytes s/SiB/SIB/g > + * @regs: Structure with register values as seen when entering kernel mode > + * @offs1: Offset of the first operand register > + * @offs2: Offset of the second opeand register, if applicable. > + * > + * Obtain the offset, in pt_regs, of the registers indicated by the ModRM byte > + * within insn. This function is to be used with 16-bit address encodings. The > + * offs1 and offs2 will be written with the offset of the two registers > + * indicated by the instruction. In cases where any of the registers is not > + * referenced by the instruction, the value will be set to -EDOM. > + * > + * Return: 0 on success, -EINVAL on failure. > + */ > +static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs, > + int *offs1, int *offs2) > +{ > + /* 16-bit addressing can use one or two registers */ > + static const int regoff1[] = { > + offsetof(struct pt_regs, bx), > + offsetof(struct pt_regs, bx), > + offsetof(struct pt_regs, bp), > + offsetof(struct pt_regs, bp), > + offsetof(struct pt_regs, si), > + offsetof(struct pt_regs, di), > + offsetof(struct pt_regs, bp), > + offsetof(struct pt_regs, bx), > + }; > + > + static const int regoff2[] = { > + offsetof(struct pt_regs, si), > + offsetof(struct pt_regs, di), > + offsetof(struct pt_regs, si), > + offsetof(struct pt_regs, di), > + -EDOM, > + -EDOM, > + -EDOM, > + -EDOM, > + }; You mean "Table 2-1. 16-Bit Addressing Forms with the ModR/M Byte" in the SDM, right? Please add a comment pointing to it here because it is not trivial to map that code to the documentation. > + > + if (!offs1 || !offs2) > + return -EINVAL; > + > + /* operand is a register, use the generic function */ > + if (X86_MODRM_MOD(insn->modrm.value) == 3) { > + *offs1 = insn_get_modrm_rm_off(insn, regs); > + *offs2 = -EDOM; > + return 0; > + } > + > + *offs1 = regoff1[X86_MODRM_RM(insn->modrm.value)]; > + *offs2 = regoff2[X86_MODRM_RM(insn->modrm.value)]; > + > + /* > + * If no displacement is indicated in the mod part of the ModRM byte, s/"no "// > + * (mod part is 0) and the r/m part of the same byte is 6, no register > + * is used caculate the operand address. An r/m part of 6 means that > + * the second register offset is already invalid. > + */ > + if ((X86_MODRM_MOD(insn->modrm.value) == 0) && > + (X86_MODRM_RM(insn->modrm.value) == 6)) > + *offs1 = -EDOM; > + > + return 0; > +} > + > +/** > * get_desc() - Obtain address of segment descriptor > * @sel: Segment selector > * > @@ -689,6 +756,94 @@ int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs) > } > > /** > + * get_addr_ref_16() - Obtain the 16-bit address referred by instruction > + * @insn: Instruction structure containing ModRM byte and displacement > + * @regs: Structure with register values as seen when entering kernel mode > + * > + * This function is to be used with 16-bit address encodings. Obtain the memory > + * address referred by the instruction's ModRM bytes and displacement. Also, the > + * segment used as base is determined by either any segment override prefixes in > + * insn or the default segment of the registers involved in the address > + * computation. In protected mode, segment limits are enforced. > + * > + * Return: linear address referenced by instruction and registers on success. > + * -1L on failure. > + */ > +static void __user *get_addr_ref_16(struct insn *insn, struct pt_regs *regs) > +{ > + unsigned long linear_addr, seg_base_addr, seg_limit; > + short eff_addr, addr1 = 0, addr2 = 0; > + int addr_offset1, addr_offset2; > + int ret; > + > + insn_get_modrm(insn); > + insn_get_displacement(insn); > + > + /* > + * If operand is a register, the layout is the same as in > + * 32-bit and 64-bit addressing. > + */ > + if (X86_MODRM_MOD(insn->modrm.value) == 3) { > + addr_offset1 = get_reg_offset(insn, regs, REG_TYPE_RM); > + if (addr_offset1 < 0) > + goto out_err; <---- newline here. > + eff_addr = regs_get_register(regs, addr_offset1); > + seg_base_addr = insn_get_seg_base(regs, insn, addr_offset1); > + if (seg_base_addr == -1L) > + goto out_err; ditto. > + seg_limit = get_seg_limit(regs, insn, addr_offset1); > + } else { > + ret = get_reg_offset_16(insn, regs, &addr_offset1, > + &addr_offset2); > + if (ret < 0) > + goto out_err; ditto. > + /* > + * Don't fail on invalid offset values. They might be invalid > + * because they cannot be used for this particular value of > + * the ModRM. Instead, use them in the computation only if > + * they contain a valid value. > + */ > + if (addr_offset1 != -EDOM) > + addr1 = 0xffff & regs_get_register(regs, addr_offset1); > + if (addr_offset2 != -EDOM) > + addr2 = 0xffff & regs_get_register(regs, addr_offset2); > + eff_addr = addr1 + addr2; ditto. Space those codelines out, we want to be able to read that code again at some point :-))) > + /* > + * The first register is in the operand implies the SS or DS > + * segment selectors, the second register in the operand can > + * only imply DS. Thus, use the first register to obtain > + * the segment selector. > + */ > + seg_base_addr = insn_get_seg_base(regs, insn, addr_offset1); > + if (seg_base_addr == -1L) > + goto out_err; > + seg_limit = get_seg_limit(regs, insn, addr_offset1); > + > + eff_addr += (insn->displacement.value & 0xffff); > + } > + > + linear_addr = (unsigned long)(eff_addr & 0xffff); > + > + /* > + * Make sure the effective address is within the limits of the > + * segment. In long mode, the limit is -1L. Thus, the second part Long mode in a 16-bit handling function? > + * of the check always succeeds. > + */ > + if (linear_addr > seg_limit) > + goto out_err; > + > + linear_addr += seg_base_addr; > + > + /* Limit linear address to 20 bits */ > + if (v8086_mode(regs)) > + linear_addr &= 0xfffff; > + > + return (void __user *)linear_addr; > +out_err: > + return (void __user *)-1; > +} > + > +/** > * _to_signed_long() - Cast an unsigned long into signed long > * @val A 32-bit or 64-bit unsigned long > * @long_bytes The number of bytes used to represent a long number > -- > 2.9.3 > -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 7, 2017

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

by Borislav Petkov

On Fri, May 05, 2017 at 11:17:14AM -0700, Ricardo Neri wrote: > @@ -697,18 +753,21 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) > { > unsigned long linear_addr, seg_base_addr, seg_limit; > long eff_addr, base, indx; > - int addr_offset, base_offset, indx_offset; > + int addr_offset, base_offset, indx_offset, addr_bytes; > insn_byte_t sib; > > insn_get_modrm(insn); > insn_get_sib(insn); > sib = insn->sib.value; > + addr_bytes = insn->addr_bytes; > > if (X86_MODRM_MOD(insn->modrm.value) == 3) { > addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); > if (addr_offset < 0) > goto out_err; > - eff_addr = regs_get_register(regs, addr_offset); > + eff_addr = get_mem_offset(regs, addr_offset, addr_bytes); > + if (eff_addr == -1L) > + goto out_err; > seg_base_addr = insn_get_seg_base(regs, insn, addr_offset); > if (seg_base_addr == -1L) > goto out_err; This code here is too dense, it needs spacing for better readability. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 7, 2017

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

by Borislav Petkov

On Fri, May 05, 2017 at 11:17:14AM -0700, Ricardo Neri wrote: > The 32-bit and 64-bit address encodings are identical. This means that we > can use the same function in both cases. In order to reuse the function > for 32-bit address encodings, we must sign-extend our 32-bit signed > operands to 64-bit signed variables (only for 64-bit builds). To decide on > whether sign extension is needed, we rely on the address size as given by > the instruction structure. > > Once the effective address has been computed, a special verification is > needed for 32-bit processes. If running on a 64-bit kernel, such processes > can address up to 4GB of memory. Hence, for instance, an effective > address of 0xffff1234 would be misinterpreted as 0xffffffffffff1234 due to > the sign extension mentioned above. For this reason, the 4 must be Which 4? > truncated to obtain the true effective address. > > Lastly, before computing the linear address, we verify that the effective > address is within the limits of the segment. The check is kept for long > mode because in such a case the limit is set to -1L. This is the largest > unsigned number possible. This is equivalent to a limit-less segment. > > Cc: Dave Hansen <dave.hansen(a)linux.intel.com> > Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> > Cc: Colin Ian King <colin.king(a)canonical.com> > Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> > Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> > Cc: Arnaldo Carvalho de Melo <acme(a)redhat.com> > Cc: Masami Hiramatsu <mhiramat(a)kernel.org> > Cc: Adrian Hunter <adrian.hunter(a)intel.com> > Cc: Kees Cook <keescook(a)chromium.org> > Cc: Thomas Garnier <thgarnie(a)google.com> > Cc: Peter Zijlstra <peterz(a)infradead.org> > Cc: Borislav Petkov <bp(a)suse.de> > Cc: Dmitry Vyukov <dvyukov(a)google.com> > Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> > Cc: x86(a)kernel.org > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > --- > arch/x86/lib/insn-eval.c | 99 ++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 88 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 1a5f5a6..c7c1239 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -688,6 +688,62 @@ int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs) > return get_reg_offset(insn, regs, REG_TYPE_RM); > } > > +/** > + * _to_signed_long() - Cast an unsigned long into signed long > + * @val A 32-bit or 64-bit unsigned long > + * @long_bytes The number of bytes used to represent a long number > + * @out The casted signed long > + * > + * Return: A signed long of either 32 or 64 bits, as per the build configuration > + * of the kernel. > + */ > +static int _to_signed_long(unsigned long val, int long_bytes, long *out) > +{ > + if (!out) > + return -EINVAL; > + > +#ifdef CONFIG_X86_64 > + if (long_bytes == 4) { > + /* higher bytes should all be zero */ > + if (val & ~0xffffffff) > + return -EINVAL; > + > + /* sign-extend to a 64-bit long */ So this is a 32-bit userspace on a 64-bit kernel, right? If so, how can a memory offset be > 32-bits and we have to extend it to a 64-bit long?!? I *think* you want to say that you want to convert it to long so that you can do the calculation in longs. However! If you're a 64-bit kernel running a 32-bit userspace, you need to do the calculation in 32-bits only so that it overflows, as it would do on 32-bit hardware. IOW, the clamping to 32-bits at the end is not something you wanna do but actually let it wrap if it overflows. Or am I missing something? -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 7, 2017

Re: [PATCH 2/4] ntdll: Provide more accurate virtual memory counters.

by Akihiro Sagawa

On Tue, 6 Jun 2017 17:40:26 -0600, Alex Henrie wrote: > When I try to run that game, I get "Failed to load libGL: libGL.so.1: > failed to map segment from shared object" with or without your > patchset. > > I also tried adding 100, 500, and 1000 MB to pvmi->VirtualSize and > pvmi->PeakVirtualSize and that didn't help either. > > So, although this patchset is interesting, I'm not convinced that it's > the right solution. There must be something else going on. > > -Alex Thanks for testing and feedback. Indeed, it needs some improvements. I'll find out more about tihs issue. Akihiro Sagawa

June 7, 2017

Re: [PATCH 1/2] mshtml: Added IBindCallbackRedirect implementation.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=31467 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

June 7, 2017

Re: [v2 PATCH 3/3] d3drm: Implement AddNormal()

by Henri Verbeet

On 7 June 2017 at 12:38, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > @@ -1165,12 +1169,13 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData, > mesh_builder->nb_normals = *(DWORD*)ptr; > nb_faces_normals = *(DWORD*)(ptr + sizeof(DWORD) + mesh_builder->nb_normals * sizeof(D3DVECTOR)); > > - TRACE("MeshNormals: nb_normals = %d, nb_faces_normals = %d\n", mesh_builder->nb_normals, nb_faces_normals); > + TRACE("MeshNormals: nb_normals = %lu, nb_faces_normals = %d\n", mesh_builder->nb_normals, nb_faces_normals); > if (nb_faces_normals != mesh_builder->nb_faces) > - WARN("nb_face_normals (%d) != nb_faces (%d)\n", nb_faces_normals, mesh_builder->nb_normals); > + WARN("nb_face_normals (%d) != nb_faces (%d)\n", nb_faces_normals, mesh_builder->nb_faces); > > - mesh_builder->pNormals = HeapAlloc(GetProcessHeap(), 0, mesh_builder->nb_normals * sizeof(D3DVECTOR)); > - memcpy(mesh_builder->pNormals, ptr + sizeof(DWORD), mesh_builder->nb_normals * sizeof(D3DVECTOR)); > + d3drm_array_reserve((void **)&mesh_builder->normals, &mesh_builder->normals_size, > + mesh_builder->nb_normals + 1, sizeof(*mesh_builder->normals)); That's not quite equivalent, but probably harmless. > @@ -1379,8 +1384,8 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData, > if (!mesh_builder->nb_normals) > { > /* Allocate normals, one per vertex */ > - mesh_builder->pNormals = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, mesh_builder->nb_vertices * sizeof(D3DVECTOR)); > - if (!mesh_builder->pNormals) > + if (!d3drm_array_reserve((void **)&mesh_builder->normals, &mesh_builder->normals_size, > + mesh_builder->nb_normals + 1, sizeof(*mesh_builder->normals))) This one isn't quite equivalent either, and less harmless. > @@ -1445,7 +1450,7 @@ HRESULT load_mesh_data(IDirect3DRMMeshBuilder3 *iface, IDirectXFileData *pData, > } > *(faces_data_ptr + faces_data_size++) = vertex_idx; > /* Add face normal to vertex normal */ > - D3DRMVectorAdd(&mesh_builder->pNormals[vertex_idx], &mesh_builder->pNormals[vertex_idx], &face_normal); > + D3DRMVectorAdd(&mesh_builder->normals[vertex_idx], &mesh_builder->normals[vertex_idx], &face_normal); In particular, note that this line depends on the initial vertex normals being {0.0f, 0.0f, 0.0f}.

June 7, 2017

Re: [PATCH 02/12] msacm32: Add invalid parameter checks for acmFormatDetails().

by Andrew Eikum

On Tue, Jun 06, 2017 at 03:14:51PM -0500, Zebediah Figura wrote: > @@ -376,10 +377,13 @@ MMRESULT WINAPI acmFormatDetailsA(HACMDRIVER had, PACMFORMATDETAILSA pafd, > ACMFORMATDETAILSW afdw; > MMRESULT mmr; > > - memset(&afdw, 0, sizeof(afdw)); > + if (!pafd) > + return MMSYSERR_INVALPARAM; > + > afdw.cbStruct = sizeof(afdw); > afdw.dwFormatIndex = pafd->dwFormatIndex; > afdw.dwFormatTag = pafd->dwFormatTag; > + afdw.fdwSupport = pafd->fdwSupport; > afdw.pwfx = pafd->pwfx; > afdw.cbwfx = pafd->cbwfx; > Like before, is there a reason to no longer zero this struct? > @@ -430,7 +447,14 @@ MMRESULT WINAPI acmFormatDetailsW(HACMDRIVER had, PACMFORMATDETAILSW pafd, DWORD > } > break; > case ACM_FORMATDETAILSF_INDEX: > - /* should check pafd->dwFormatIndex < aftd->cStandardFormats */ > + aftd.cbStruct = sizeof(aftd); > + aftd.dwFormatTag = pafd->dwFormatTag; > + acmFormatTagDetailsW(had, &aftd, ACM_FORMATTAGDETAILSF_FORMATTAG); > + if (pafd->dwFormatIndex >= aftd.cStandardFormats) > + { > + mmr = MMSYSERR_INVALPARAM; > + break; > + } Please add error checking for acmFormatTagDetailsW. The next two patches in the sequence fail on the test bot, so I stopped reviewing here. It's common for APIs to return different error codes on different Windows versions, so you likely just need to accept either code in your ok() checks. Andrew

June 7, 2017