[PATCH] kernel32: Check dwFlags parameter in QueryFullProcessImageNameW().

[Serge Gautherie]

Signed-off-by: Serge Gautherie 
---
 dlls/kernel32/process.c       | 6 ++++++
 dlls/kernel32/tests/process.c | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c
index 23e2316..9f4b313 100644
--- a/dlls/kernel32/process.c
+++ b/dlls/kernel32/process.c
@@ -4030,6 +4030,12 @@ BOOL WINAPI QueryFullProcessImageNameW(HANDLE hProcess, DWORD dwFlags, LPWSTR lp
     NTSTATUS status;
     DWORD needed;

+    if (dwFlags >= 2)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
     /* FIXME: On Windows, ProcessImageFileName return an NT path. In Wine it
      * is a DOS path and we depend on this. */
     status = NtQueryInformationProcess(hProcess, ProcessImageFileName, buffer,
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c
index cba0a3f..9fae67f 100644
--- a/dlls/kernel32/tests/process.c
+++ b/dlls/kernel32/tests/process.c
@@ -2038,26 +2038,20 @@ static void test_QueryFullProcessImageNameW(void)
         size = ARRAY_SIZE(buf);
         SetLastError(0xdeadbeef);
         *(DWORD*)buf = 0x13579acf;
-        todo_wine
-        {
         expect_eq_d(FALSE, pQueryFullProcessImageNameW(hSelf, flags, buf, &size));
         expect_eq_d(ARRAY_SIZE(buf), size);  /* size not changed */
         expect_eq_d(ERROR_INVALID_PARAMETER, GetLastError());
         expect_eq_d(0x13579acf, *(DWORD*)buf);  /* buffer not changed */
-        }
     }
     for (flags = 16; flags != 0; flags <<= 1)
     {
         size = ARRAY_SIZE(buf);
         SetLastError(0xdeadbeef);
         *(DWORD*)buf = 0x13579acf;
-        todo_wine
-        {
         expect_eq_d(FALSE, pQueryFullProcessImageNameW(hSelf, flags, buf, &size));
         expect_eq_d(ARRAY_SIZE(buf), size);  /* size not changed */
         expect_eq_d(ERROR_INVALID_PARAMETER, GetLastError());
         expect_eq_d(0x13579acf, *(DWORD*)buf);  /* buffer not changed */
-        }
     }

/* native path */

[Dmitry Timoshkov]

Serge Gautherie  wrote:
> @@ -4030,6 +4030,12 @@ BOOL WINAPI QueryFullProcessImageNameW(HANDLE hProcess, DWORD dwFlags, LPWSTR lp
>      NTSTATUS status;
>      DWORD needed;
> - if (dwFlags >= 2)
> - {
> -
>    SetLastError(ERROR_INVALID_PARAMETER);
> -
>    return FALSE;
> - }

What does that mean? Is there an application that depends on this? If you
don't know, then it's better to not add anything like this.

[Serge Gautherie]

On 01/08/2019 13:17, Dmitry Timoshkov wrote:
> Serge Gautherie  wrote:
> > @@ -4030,6 +4030,12 @@ BOOL WINAPI QueryFullProcessImageNameW(HANDLE hProcess, DWORD dwFlags, LPWSTR lp
> >       NTSTATUS status;
> >       DWORD needed;
> > - if (dwFlags >= 2)
> > - {
> > -
> >    SetLastError(ERROR_INVALID_PARAMETER);
> > -
> >    return FALSE;
> > - }
>
> What does that mean?

I am not sure what you are asking for.
To succeed tests!?
> Is there an application that depends on this?

I don't know indeed.
> If you don't know, then it's better to not add anything like this.

Is Wine not interested in validating parameters (values), then?

[Dmitry Timoshkov]

Serge Gautherie  wrote:
> > > @@ -4030,6 +4030,12 @@ BOOL WINAPI QueryFullProcessImageNameW(HANDLE hProcess, DWORD dwFlags, LPWSTR lp
> > >       NTSTATUS status;
> > >       DWORD needed;
> > > - if (dwFlags >= 2)
> > > - {
> > > -
> > >    SetLastError(ERROR_INVALID_PARAMETER);
> > > -
> > >    return FALSE;
> > > - }
> >
> > What does that mean?
>
> I am not sure what you are asking for.

I'm asking what that magic check is supposed to do.
> To succeed tests!?

Why?
> > Is there an application that depends on this?
>
> I don't know indeed.
> > If you don't know, then it's better to not add anything like this.
>
> Is Wine not interested in validating parameters (values), then?

Validating parameters makes sense when it makes an application work.

[Marvin]

Hi,

While running your changed tests, 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=55007

Your paranoid android.

=== wvistau64_he (32 bit report) ===

kernel32:
process.c:2525: Test failed: WaitForSingleObject returned 258
process.c:2529: Test failed: AssignProcessToJobObject unexpectedly succeeded
process.c:2530: Test failed: Expected GetLastError() to be 5 (ERROR_ACCESS_DENIED) is -559038737
process.c:2557: Test failed: WaitForSingleObject returned 258
process.c:2854: Test failed: WaitForSingleObject returned 258
process.c:2904: Test failed: WaitForSingleObject returned 258
process.c:2921: Test failed: WaitForSingleObject returned 258

=== debian10 (build log) ===

error: patch failed: dlls/kernel32/tests/process.c:2038
Task: Patch failed to apply

=== debian10 (build log) ===

error: patch failed: dlls/kernel32/tests/process.c:2038
Task: Patch failed to apply

[Serge Gautherie]

Not caused by my patch.

On 01/08/2019 13:36, Marvin wrote:
> Hi,
>
> While running your changed tests, 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=55007
>
> Your paranoid android.
>
> === wvistau64_he (32 bit report) ===
>
> kernel32:
> process.c:2525: Test failed: WaitForSingleObject returned 258
> process.c:2529: Test failed: AssignProcessToJobObject unexpectedly succeeded
> process.c:2530: Test failed: Expected GetLastError() to be 5 (ERROR_ACCESS_DENIED) is -559038737
> process.c:2557: Test failed: WaitForSingleObject returned 258
> process.c:2854: Test failed: WaitForSingleObject returned 258
> process.c:2904: Test failed: WaitForSingleObject returned 258
> process.c:2921: Test failed: WaitForSingleObject returned 258
>
> === debian10 (build log) ===
>
> error: patch failed: dlls/kernel32/tests/process.c:2038
> Task: Patch failed to apply
>
> === debian10 (build log) ===
>
> error: patch failed: dlls/kernel32/tests/process.c:2038
> Task: Patch failed to apply