Re: ntdll: Do not accept device control requests with invalid and/or incompatible handles

[Alexandre Julliard]

Peter Dons Tychsen  writes:
> +static void test_device_control(void)
> +{
> - IO_STATUS_BLOCK io;
> - NTSTATUS status;
> -
> - /* Try using a bugus handle for I/O control */
> - HANDLE handle = (HANDLE)0xdeadbeef;
> - status = pNtDeviceIoControlFile(handle, NULL, NULL, NULL, &io, IOCTL_SERIAL_SET_BAUD_RATE, NULL, 0, NULL, 0);
> - ok(status == STATUS_INVALID_HANDLE, "Expected %08X, got %08X\n", STATUS_INVALID_HANDLE, status);
> -
> - /* Try using an valid (but wrong) handle for I/O control */
> - handle = GetStdHandle(STD_INPUT_HANDLE);
> - status = pNtDeviceIoControlFile(handle, NULL, NULL, NULL, &io, IOCTL_SERIAL_SET_BAUD_RATE, NULL, 0, NULL, 0);
> - ok(status == STATUS_INVALID_HANDLE, "Expected %08X, got %08X\n", STATUS_INVALID_HANDLE, status);

GetStdHandle returns a console handle which is not a valid ntdll handle,
so you are not testing what you think.

ntdll: Do not accept device control requests with invalid and/or incompa tible handles

[Peter Dons Tychsen]

Thanks for the comments.
Not sure i understand them though.

The whole point of the test was to test what happens when you use a wrong
handle. The actual bug i fixed was that it was possible to for programs
to send invalid handles to IoControl. Cygwin was infact sending handles
to ntdll.dll which originated from GetStdHandle().

So i think using this API for testing is relevant, as the point of the
test is to verify that handles of the completely wrong class are
rejected, and not just "bad handles".

Thanks,

/pedro

----- Original meddelelse -----
> Fra: Alexandre Julliard 
> Til: Peter Dons Tychsen 
> Cc: [e-mail]
> Dato: Tir, 08. sep 2009 22:25
> Emne: Re: ntdll: Do not accept device control requests with invalid
> and/or incompatible handles
>
> Peter Dons Tychsen  writes:
> > +static void test_device_control(void)
> > +{
> > - IO_STATUS_BLOCK io;
> > - NTSTATUS status;
> > -
> > - /* Try using a bugus handle for I/O control */
> > - HANDLE handle = (HANDLE)0xdeadbeef;
> > - status = pNtDeviceIoControlFile(handle, NULL, NULL, NULL, &io,
>
> IOCTL_SERIAL_SET_BAUD_RATE, NULL, 0, NULL, 0);
> > - ok(status == STATUS_INVALID_HANDLE, "Expected %08X, got %08X\n",
>
> STATUS_INVALID_HANDLE, status);
> > -
> > - /* Try using an valid (but wrong) handle for I/O control */
> > - handle = GetStdHandle(STD_INPUT_HANDLE);
> > - status = pNtDeviceIoControlFile(handle, NULL, NULL, NULL, &io,
>
> IOCTL_SERIAL_SET_BAUD_RATE, NULL, 0, NULL, 0);
> > - ok(status == STATUS_INVALID_HANDLE, "Expected %08X, got %08X\n",
>
> STATUS_INVALID_HANDLE, status);
>
> GetStdHandle returns a console handle which is not a valid ntdll
> handle,
> so you are not testing what you think.
>
> --
> Alexandre Julliard
> [e-mail]

ntdll: Do not accept device control requests with invalid and/or incompa tible handles

[Alexandre Julliard]

Peter Dons Tychsen  writes:
> Thanks for the comments.
> Not sure i understand them though.
>
> The whole point of the test was to test what happens when you use a wrong
> handle. The actual bug i fixed was that it was possible to for programs
> to send invalid handles to IoControl. Cygwin was infact sending handles
> to ntdll.dll which originated from GetStdHandle().
>
> So i think using this API for testing is relevant, as the point of the
> test is to verify that handles of the completely wrong class are
> rejected, and not just "bad handles".

Yes, but then you should explicitly construct and test both kinds of
handles. Using GetStdHandle will either be a wrong class or a bad handle
depending on whether stdio is to a file or to a console, so you don't
really know which case you are testing. The remapping of console handles
happens in kernel32, not in ntdll, so it doesn't really make sense to
pass the result of GetStdHandle straight to ntdll.

ntdll: Do not accept device control requests with invalid and/or incompa tible handles

[Peter Dons Tychsen]

OK. The "bad handle" is tested by the "deadbeef" handle.
Where is the best place to get a really good "wrong class" handle?

Thanks,

/pedro

----- Original meddelelse -----
> Fra: Alexandre Julliard 
> Til: Peter Dons Tychsen 
> Cc: [e-mail]
> Dato: Tor, 10. sep 2009 22:20
> Emne: Re: ntdll: Do not accept device control requests with invalid
> and/or incompa tible handles
>
> Peter Dons Tychsen  writes:
> > Thanks for the comments.
> > Not sure i understand them though.
> >
> > The whole point of the test was to test what happens when you use a
>
> wrong
> > handle. The actual bug i fixed was that it was possible to for
>
> programs
> > to send invalid handles to IoControl. Cygwin was infact sending
>
> handles
> > to ntdll.dll which originated from GetStdHandle().
> >
> > So i think using this API for testing is relevant, as the point of
>
> the
> > test is to verify that handles of the completely wrong class are
> > rejected, and not just "bad handles".
>
> Yes, but then you should explicitly construct and test both kinds of
> handles. Using GetStdHandle will either be a wrong class or a bad
> handle
> depending on whether stdio is to a file or to a console, so you don't
> really know which case you are testing. The remapping of console
> handles
> happens in kernel32, not in ntdll, so it doesn't really make sense to
> pass the result of GetStdHandle straight to ntdll.
>
> --
> Alexandre Julliard
> [e-mail]

ntdll: Do not accept device control requests with invalid and/or incompa tible handles

[Vitaliy Margolen]

Peter Dons Tychsen wrote:
> OK. The "bad handle" is tested by the "deadbeef" handle.
> Where is the best place to get a really good "wrong class" handle?

Create it. Event handle will do for example.

Vitaliy.

ntdll: Do not accept device control requests with invalid and/or incompa tible handles

[Peter Dons Tychsen]

Thanks V.

However, it would seem that AJ already has commited another fix in ntdlls
serial module (which fixes the same bug), so i guess my fix and tests are
not valid any more.

Thanks,

/pedro

----- Original meddelelse -----
> Fra: Vitaliy Margolen 
> Til: Peter Dons Tychsen 
> Cc: [e-mail]
> Dato: Søn, 13. sep 2009 23:42
> Emne: Re: ntdll: Do not accept device control requests with invalid
> and/or incompa tible handles
>
> Peter Dons Tychsen wrote:
> > OK. The "bad handle" is tested by the "deadbeef" handle.
> > Where is the best place to get a really good "wrong class" handle?
>
> Create it. Event handle will do for example.
>
> Vitaliy.