Help diagnosing a problem with cygwin/samba on ROS

All development related issues welcome

Moderator: Moderator Team

hbelusca
Developer
Posts: 1204
Joined: Sat Dec 26, 2009 10:36 pm
Location: Zagreb, Croatia

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by hbelusca »

Hi, can you retest again with the newest build of ReactOS? Fixes for the cache manager, the memory manager, and the usage of the new open-sourced Microsoft FastFat driver have been done yesterday.
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

That sounds very interesting and potentially a big move for ReactOS. I am still tinkering with this because it looks like there may be small issues in the object manager as well (for this oddball edge case usage anyway). I don't know why it doesn't happen on BtrFs, but the traces look like ObOjectLookupByName may return the incorrect object handle in this case.

Using the MS driver sounds like it may improve some stability issues as well which is always a good thing.

bugdude
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

I wonder if someone could help me build a small workaround for this problem. Basically what happens is this.. The library(cygwin) calls NtOpenFile using a path and the call works completely and the application gets back a File Handle.

The library then it tries to reopen that same file by calling NtOpenFile() again after using InitObjectAttributes to set Attributes->RootDirectory to the file handle returned by teh first call combined with a blank path. NtOpenFile() builds an OpenPacket and calls ObOpenObjectByName which calls ObpLookupObjectName which calls IopParseDevice but ultimately ObpLookupObjectName does not find the object and returns a handle to the FAT volume the file is on. I suspect this happens because it gets as far as locating the device and returns the handle for that. It is not surprising that the operations fail because they are based on the name having a value and it doesn't have one in this case.

What I am looking to find is a direct way to duplicate the handle using the internal API, so I can detect this condition on entry to NtOpenFile() and use that alternate mechanism to duplicate the handle. Detecting this condition is easy enough, but I have had a series of problems trying to duplicate the handle. I started by looking at NtDuplicateObject but could not get it to work, it failed giving an access denied error 0xc0000005 when probing the target handle to see if it could be written. I looked at other functions in the object manager and io manager and couldn't find any that seem to duplicate file handles easily. I tried calling ObReferenceObjectByHandle() intending to follow that by calling ObOpenObjectByPointer(), but ObReferenceObjectByHandle() failed with error 0xc0000008 (invalid_handle). It does that whether I use an ObjectType of NULL or of IoFileObject which I thought would be the correct type. All told I have tried about five or six combinations to no avail. MS Docs say there is a function called DuplicateHandle() but I couldn't find it anywhere so I assume it's not part of the internal API or possibly is from a later windows version.

Does anyone know how I can take the FileHandle returned by NtOpenFile() and duplicate it (create another handle to same object with same access rights/attributes etc. ?

bugdude
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

bugdude wrote: Thu Feb 11, 2021 4:40 am I wonder if someone could help me build a small workaround for this problem. Basically what happens is this.. The library(cygwin) calls NtOpenFile using a path and the call works completely and the application gets back a File Handle.

The library then it tries to reopen that same file by calling NtOpenFile() again after using InitObjectAttributes to set Attributes->RootDirectory to the file handle returned by teh first call combined with a blank path. NtOpenFile() builds an OpenPacket and calls ObOpenObjectByName which calls ObpLookupObjectName which calls IopParseDevice but ultimately ObpLookupObjectName does not find the object and returns a handle to the FAT volume the file is on. I suspect this happens because it gets as far as locating the device and returns the handle for that. It is not surprising that the operations fail because they are based on the name having a value and it doesn't have one in this case.

What I am looking to find is a direct way to duplicate the handle using the internal API, so I can detect this condition on entry to NtOpenFile() and use that alternate mechanism to duplicate the handle. Detecting this condition is easy enough, but I have had a series of problems trying to duplicate the handle. I started by looking at NtDuplicateObject but could not get it to work, it failed giving an access denied error 0xc0000005 when probing the target handle to see if it could be written. I looked at other functions in the object manager and io manager and couldn't find any that seem to duplicate file handles easily. I tried calling ObReferenceObjectByHandle() intending to follow that by calling ObOpenObjectByPointer(), but ObReferenceObjectByHandle() failed with error 0xc0000008 (invalid_handle). It does that whether I use an ObjectType of NULL or of IoFileObject which I thought would be the correct type. All told I have tried about five or six combinations to no avail. MS Docs say there is a function called DuplicateHandle() but I couldn't find it anywhere so I assume it's not part of the internal API or possibly is from a later windows version.

Does anyone know how I can take the FileHandle returned by NtOpenFile() and duplicate it (create another handle to same object with same access rights/attributes etc. ?

bugdude
Does cygwin directly call NtOpenFile, or does it call some higher level function?
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

Cygwin directly calls NtOpenFile() in both cases, it targets the internal API for most of its calls rather than the main winapi.

I looked on MSDN and did not find anything in the MS Docs that indicates that making a call to NtOpenFile in this fashion is incorrect or invalid, or that precludes leaving the path empty or blank when initializing the Object Attributes, however the NtOpenFile source code in ReactOS definitely seems to 'presume' that the pathname will be populated. It seems to begin with a search based on the pathname. What I hoped to do was basically check for an empty path with rootdir defined and duplicate the handle when that condition occurs regardless of underlying filesystem. In theory the entire call could be handled in io manager without actually performing any driver or physical IO activity - a short circuit evaluation of sorts.

bigdude
hbelusca
Developer
Posts: 1204
Joined: Sat Dec 26, 2009 10:36 pm
Location: Zagreb, Croatia

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by hbelusca »

You may try to write a minimal test with NtOpenFile() that tries to do what you think cygwin & co does. That test should of course work on Windows. And then try it on ReactOS.
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

hbelusca wrote: Fri Feb 12, 2021 1:23 am You may try to write a minimal test with NtOpenFile() that tries to do what you think cygwin & co does. That test should of course work on Windows. And then try it on ReactOS.
or find a test like that in the existing test suite
CoryColbert
Posts: 1
Joined: Tue Feb 16, 2021 4:20 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by CoryColbert »

bugdude wrote: Fri Jan 15, 2021 1:15 am Hello everyone,

First off Kudos, I have been working on 4.13-dev for some time and recently jumped to 4.15-dev and the greater stability really shows everywhere.

I have been working on porting Samba4 (including the servers) to ROS as a pet project for some time, Because Samba has a long dependency list I decided to use cygwin. In order to be compatible with Win2003 API levels an older cygwin was the only choice so I used the 2.5.2 version which was the last version compatible with W2K3 that uses LGPL. I finally got everything to build and I have a test candidate that works with alternate ports on Win 10, but unfortunately it died on first run on ROS 4.13-dev so I shifted to 4.15-dev hoping for better and found it did the same thing there. I instrumented Samba, and then cygwin, and recently ROS a bit, but after putting things together I ran into a new problem because I cannot get ROS file debug logging to work on ROS 4.15-dev..

I edited Freeeldr.ini and configured the LogFile boot item and tested it using both ArcName and Device paths but could not get the logging to work. I added some DPRINTS, commented out the NDEBUG define, and set the environment variables to enable all channels then edited freeldr.ini. It seems to slow down execution some, but no logfile seems to be getting created anywhere. I also tried it with no path specified but nothing works leaving me a bit stalled leading to my first question:
Any idea how I can get debug logging to work under 4.15-dev since the methods in the wiki page don't seem to work ?

The reason I am trying to enable logging is to track down a problem that occurs in ROS that does not happen when the same binary is executed under Windows 10. The application is basically opening a file and doing a random read of a small (4-byte) header data element using the NT Native API NtOpenFile, NtCreateFile, NtReadFile routines. The Failure actually happens in a call to NtReadFile which ends up returning STATUS_INVALID_PARAMETER. After reviewing the code in io/iomgr/iofunc.c and related files I decided to instrument the exit points to see where it exits because there are quite a few conditions that set that status. Most of them seem to be parameter checks and failures that apply only for non-cached access, but the file is not being opened specifying direct access or no caching, so those should not apply in this case. In any event that leads to my second question:
Are there any parameter validations in NtReadFile() under ROS that are more strict than in WIN 10 that could cause this ?/

Based on tracing the call coming from cygwin looks like this:
NtOpenFile(<handle>, 0xC0100000, \??\C:\ReactOS\samba\var\lock\names.tdb, io, 0x7, 0x4020 ); -- this call succeeds
NtReadFile(<handle>, NULL NULL, NULL, &io, buf, count, &off, NULL); /* where count=4 and off=44 */ -- this call fails

I also tried rebuilding after replacing the first NULL in teh NtReadFile() call with a zero because some docs indicate it should be a UINT type and got the same results.

Help !!

Bugdude
Using the correct Cygwin mailing list is absolutely the proper way to report problems or make observations. The mailing lists were created for this express purpose. Reporting problems to a public forum means that the workload is shared and, since your report will be read by many people, it may get more attention than one person would be able to provide.

So, there is generally no need to "Cc" a package maintainer with your observations. All maintainers should be reading the appropriate mailing list so Ccing them will result in sending them two copies of your email.

This is particularly true if you notice that someone has specifically set the Reply-To of a message to go only to the cygwin mailing list. Some contributors even take this a step further and actually set their return address to the mailing list in an attempt to make it very obvious that Cygwin-related email should go to the mailing list.

Nevertheless, some people still seem to insist on either Cc'ing Cygwin contributors or sending private email. This is inconsiderate. Please do not do this unless specifically requested.
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

I actually did write a test for this, but since I do not really do much programming in C I did it under a tool I'm more familiar with which was Delphi. The test binary produced different results under Win10 than ROS, but in the end it showed that was not an issue with validation.

The returned error seems to occur because under ROS the second call to NtOpenFile() opens the incorrect file object. This is not a problem with Cygwin, but I believe this is a bug in the ROS implementation of IopCreateFile(). It is admittedly an edge case that doesn't occur very often rather than a common usage, but as far as I can tell the usage is not incorrect or invalid.

Cygwin first calls NtOpenFile() in the traditional fashion using a full path, but on its second call it makes a call to NtOpenFile() using only the handle from the first call with a blank path. Under Windows 10 the two opens return different handles to the same underlying file and the two handles have the same properties (essentially it duplicates the file first handle). Within ROS the results are very different, ROS returns success for the second call but the handle it returns points to the volume hosting the file rather than the file. Looking at the ROS source it seems this takes place because the code assumes that a path is always present and rootdir will contain a parent object rather than an actual leaf node (file) and ROS searches for the file object based on that assumption while Windows does not seem to make that assumption giving different results.

The failure is not detected in the open because it returns success and a handle.. Cygwin finally fails when the first unaligned read operation is undertaken on the second handle because the handle returned by ROS points at a volume which is non-cached and requires reads to be aligned. The validation fails the read, but this is actually the correct thing to do because the handle requires alignment.

The testing rig I made in Delphi confirmed my suspicion by returning a very different file size for the second handle and then failing when running the unaligned read on the second handle (it succeeeds on the first file handle because the correct file is opened). I was trying to program a simple workaround by detecting the condition and calling ObLookupObjectByHandle() rather than ObLookupObjectByName() as the IoCreateFile() code currently does, but I couldn't get things to work..

the pseudocode for a test in C would look like this: (any data file over 50 bytes long would work for testing)

Code: Select all

#include <ntoskrnl.h>

InitializeObjectAttributes(Oa, "C:\FILE.TXT", OBJ_CASE_INSENSITIVE, 0, NULL);
Status = NtOpenFile(ourFD1, SYNCHRONIZE || GENERIC_READ || GENERIC_WRITE, Oa, FILE_SHARE_ALL , 
                               FILE_OPEN_FOR_BACKUP_INTENT || FILE_SYNCHRONOUS_IO_NONALERT);
-- check status bomb if not successful
Status = NtQueryInformationFile(ourFD1, iosb, &fsi, Sizeof(TFileStandardInformation), FileStandardInformation);
-- check status bomb if not successful
FileSize1 = fsi.EndofFile;
Status = NtReadFile(ourFD1 , 0, NULL, NULL, iosb, &bufr[0], 4, NULL, NULL); /* aligned read */
-- check status bomb if not successful
Status = NtReadFile(ourFD1 , 0, NULL, NULL, iosb, &bufr[0], 4, 44, NULL); /* non-aligned read */
-- check status bomb if not successful

InitializeObjectAttributes(Oa2, "", OBJ_CASE_INSENSITIVE, ourFD1, NULL); /* attributes do not include a path, only existing open handle */

Status = NtOpenFile(ourFD2, SYNCHRONIZE || GENERIC_READ || GENERIC_WRITE, Oa2, FILE_SHARE_ALL , 
                               FILE_OPEN_FOR_BACKUP_INTENT || FILE_SYNCHRONOUS_IO_NONALERT);
-- check status bomb if not successful
Status = NtQueryInformationFile(ourFD2, iosb, &fsi, Sizeof(TFileStandardInformation), FileStandardInformation);
-- check status bomb if not successful
FileSize2 = fsi.EndofFile;
if (FileSize1 <> FileSize2)  printf("ERROR  Filesizes of the two handles do not match !\n");
Status = NtReadFile(ourFD2 , 0, NULL, NULL, iosb, &bufr[0], 4, NULL, NULL); /* aligned read */
-- check status bomb if not successful
Status = NtReadFile(ourFD2 , 0, NULL, NULL, iosb, &bufr[0], 4, 44, NULL); /* non-aligned read */
-- check status bomb if not successful
The test outcome should match what I saw in the Delphi test app, the filesize message will show different filesizes for the two handles under ROS but not under Windows, and the last read operation on the second file handle will fail under ROS while the entire test succeeds under Windows. The test doesn't even go so far as to validate the actual data from the reads because it doesn't need to go that far to fail.. Looking through the ROS code I see NtOpenFile call works it's way to IopCreateFile() and IopCreateFile() runs all calls through ObLookupObjectByName() and since the second call has no name(path) and ObLookupByName() doesn't expect that, the results are not what we get in Windows..

If someone more comfortable with C wants to build this out and test it independently I'm sure you'll get the same results I did in Delphi.

BugDude
Last edited by bugdude on Wed Feb 17, 2021 9:47 pm, edited 4 times in total.
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

bugdude wrote: Wed Feb 17, 2021 6:26 am I actually did write a test for this, but since I do not really do much programming in C I did it under a tool I'm more familiar with which was Delphi. The test binary produced different results under Win10 than ROS, but in the end it showed that was not an issue with validation.

--blub--
ps: You used file handle 1 for first and second test, this is not correct in the delphi snippet ;)


Here is the c equivalent:

Code: Select all

/*
 * PROJECT:     ReactOS API Tests
 * LICENSE:     LGPL-2.1-or-later (https://spdx.org/licenses/LGPL-2.1-or-later)
 * PURPOSE:     Test for NtOpenFile
 * COPYRIGHT:   Copyright 2021 Your name here <your.name@here>
 *              Copyright 2021 Mark Jansen <mark.jansen@reactos.org>
 */

#include "precomp.h"
#include <ntstrsafe.h>


static BOOL create_temp_file(WCHAR* DosName, PUNICODE_STRING NtName)
{
    WCHAR path[MAX_PATH];
    HANDLE hFile;

    GetTempPathW(RTL_NUMBER_OF(path), path);
    GetTempFileNameW(path, L"NtO", 0, DosName);
    hFile = CreateFileW(DosName, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0);
    ok(hFile != INVALID_HANDLE_VALUE, "failed to create temp file\n");
    if (hFile != INVALID_HANDLE_VALUE)
    {
        char Buf[0xf];
        int n;
        for (n = 0; n < 10; ++n)
        {
            DWORD dwWritten;
            memset(Buf, n + '0', sizeof(Buf));
            WriteFile(hFile, Buf, sizeof(Buf), &dwWritten, NULL);
            ok_int(dwWritten, sizeof(Buf));
        }

        CloseHandle(hFile);
        RtlDosPathNameToNtPathName_U(DosName, NtName, NULL, NULL);

        return TRUE;
    }

    return FALSE;
}

static void run_test(UNICODE_STRING* FileName)
{
    OBJECT_ATTRIBUTES ObjectAttributes;
    FILE_STANDARD_INFORMATION FileStandardInfo;
    IO_STATUS_BLOCK IoStatusBlock;
    LARGE_INTEGER ByteOffset, File1Size;
    UNICODE_STRING EmptyName = RTL_CONSTANT_STRING(L"");

    NTSTATUS Status;
    HANDLE hFile1, hFile2;
    char bufr[5] = { 0 };

    InitializeObjectAttributes(&ObjectAttributes, FileName, OBJ_CASE_INSENSITIVE, 0, NULL);

    Status = NtOpenFile(&hFile1, SYNCHRONIZE | GENERIC_READ | GENERIC_WRITE, &ObjectAttributes, &IoStatusBlock, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_OPEN_FOR_BACKUP_INTENT | FILE_SYNCHRONOUS_IO_NONALERT);
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
        return;

    Status = NtQueryInformationFile(hFile1, &IoStatusBlock, &FileStandardInfo, sizeof(FILE_STANDARD_INFORMATION), FileStandardInformation);
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        return;
    }

    File1Size = FileStandardInfo.EndOfFile;

    Status = NtReadFile(hFile1, 0, NULL, NULL, &IoStatusBlock, bufr, 4, NULL, NULL); /* aligned read */
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        return;
    }

    ByteOffset.QuadPart = 44;
    Status = NtReadFile(hFile1, 0, NULL, NULL, &IoStatusBlock, &bufr[0], 4, &ByteOffset, NULL); /* non-aligned read */
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        return;
    }


    /* attributes do not include a path, only existing open handle */
    InitializeObjectAttributes(&ObjectAttributes, &EmptyName, OBJ_CASE_INSENSITIVE, hFile1, NULL);
    Status = NtOpenFile(&hFile2, SYNCHRONIZE | GENERIC_READ | GENERIC_WRITE, &ObjectAttributes, &IoStatusBlock, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_OPEN_FOR_BACKUP_INTENT | FILE_SYNCHRONOUS_IO_NONALERT);
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        return;
    }


    Status = NtQueryInformationFile(hFile2, &IoStatusBlock, &FileStandardInfo, sizeof(FILE_STANDARD_INFORMATION), FileStandardInformation);
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        NtClose(hFile2);
        return;
    }

    ok_int(FileStandardInfo.EndOfFile.LowPart, File1Size.LowPart);
    ok_int(FileStandardInfo.EndOfFile.HighPart, File1Size.HighPart);


    Status = NtReadFile(hFile2, 0, NULL, NULL, &IoStatusBlock, bufr, 4, NULL, NULL); /* aligned read */
    ok_hex(Status, STATUS_SUCCESS);
    if (!NT_SUCCESS(Status))
    {
        NtClose(hFile1);
        NtClose(hFile2);
        return;
    }

    ByteOffset.QuadPart = 44;
    Status = NtReadFile(hFile2, 0, NULL, NULL, &IoStatusBlock, &bufr[0], 4, &ByteOffset, NULL); /* non-aligned read */
    ok_hex(Status, STATUS_SUCCESS);

    NtClose(hFile1);
    NtClose(hFile2);
}

START_TEST(NtOpenFile)
{
    WCHAR DosName[MAX_PATH + 1] = { 0 };
    UNICODE_STRING FileName;


    if (!create_temp_file(DosName, &FileName))
        return;

    run_test(&FileName);
    DeleteFileW(DosName);
    RtlFreeUnicodeString(&FileName);
}


[code]
Last edited by learn_more on Wed Feb 17, 2021 10:10 pm, edited 1 time in total.
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

Learn_more,

You are correct, that was a typo in the pseudocode (which I'll fix momentarily). The Delphi test app did not have that error in it.

I have been looking through more of the Cygwin code and they actually use this method pretty heavily in their code and refer to it as a 'reopen' of a file. Even though so far I have only seen it in cygwin they do use it frequently, it appears in several common system calls (fstat, pread and pwrite so far). Cygwin also has a workaround in their code to avoid using this 'reopen' method which they implemented for buggy network filesystems, I'm currently looking at detecting ROS and activating that workaround in a custom cygwin1.dll since I'm more familiar with that code. Now I just need a clean and accurate way to identify ROS using API calls that exist in Windows and ROS. That would allow me to trigger the workaround for ROS and still have a DLL that works in windows so I can continue testing the Samba port I am working on. You never know I might even end up with a ROS compatible Cygwin 2.5.2 build as well. It would be a bit outdated since Cygwin is many versions along since then, but it's still potentially useful.

Were you able to compile and run your test binary to confirm you see failure on ROS and do not see failure on Windows ?

I have only tested this on Windows 10, so I am curious if this method actually works under the exact API versions that ROS targets (200/2003/XP) which would mean this is already a compatibility failure bug in those versions rather than just a feature that exists in newer windows versions not present in those API targets.

bugdude
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

bugdude wrote: Wed Feb 17, 2021 9:44 pm I'm currently looking at detecting ROS and activating that workaround in a custom cygwin1.dll since I'm more familiar with that code. Now I just need a clean and accurate way to identify ROS using API calls that exist in Windows and ROS.
We don't have an api to detect ROS, precisely to not have people do what you are trying to do:
Detect ROS and change behavior.

The code works fine on Win10, 2k3 and ReactOS btw,
so either something got lost in translation or you are looking at the wrong code?
Can you link to the offending code?
bugdude
Posts: 28
Joined: Wed Nov 27, 2019 11:41 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by bugdude »

That test failing on ROS would essentially confirm that a bug exists in ROS in the API call NtOpenFile(). How should it be formally reported if confirmed ?

There must be something lost if your test worked in ROS. The only obvious difference between the pseudocode and my test in Delphi would be the path and file. What I did is create a folder ReactOS\samba\var and the test file was placed in that folder. It is a TDB database file which is why random access is required, but it only has a header and no data and the random read is reading a header element. Maybe if you use the same path you'll get a failure ?

Since NtOpenFile is basically a forwarder, the code flaw actually lies in the iomanager IopCreateFile() function.

As far as detection goes, I understand that and read that in other forum posts. The rationale is sound in that it avoids people 'blocking' software from running on ROS, but the reverse is also true because this also becomes a blocker for porting useful software onto ROS when the process hits bugs or requires APIs that are not yet implemented. I will see if I can add a config element for it somewhere in a custom Cygwin DLL so a user can selectively enable the workaround, but an automatic detection would have been a far more user-friendly way to do this.

In any event below are a few a failed attempts I made to have IopCreateFile detect this condition and call ObLookupObjectByHandle() the code snippets do not work (because I am not much of a C programmer and can't get all the details right). If you can see how to arm-twist any of these attempts into working it could form the start for a patch to fix this problem in iomanager. The code would be inserted in IopCreateFile() right after the parameter validations are carried out, and would provide a code branch for these 'reopen' type calls.

I hope you won't laugh too hard at my mistakes, I've never studied C so it's coded by plain and simple 'osmosis' learning and I do apologize for that !

Code: Select all

    /* hack to handle Cygwin edge case of re-opening file by passing in existing Handle by deflecting call to ObOpenObjectByHandle() early
        Done early to avoid generating OpenPacket or other structures because we should have most of what we need to duplicate handle already. 
        first check if the pathname is NULL or blank and if so call our hack, if not skip it and fall through to existing code for IopCreateFile()   */
    if (  (ObjectAttributes->ObjectName->Length <= 1 || (!ObjectAttributes->ObjectName->Length) ||  /* detect empty path */
            (!ObjectAttributes->ObjectName) || (!ObjectAttributes->ObjectName->Buffer)) && 
            (ObjectAttributes->RootDirectory != 0) && (ObjectAttributes->RootDirectory != NULL) ) {
	
	DPRINT1("(PID %lx) IopCreateFile called with ObjectAttributes->RootDirectory set and  without FileName in ObjectAttributes->ObjectName, 
                                     calling DuplicateHandle(). \n", PsGetCurrentProcessId() );

/* won't compile, where is DuplicateHandle API function found in ROS ??
        if (DuplicateHandle(PsGetCurrentProcessId(), // according to MS docs DuplicateHandle works with pseudoHandle from GetCurrentProcess()
                            ObjectAttributes->RootDirectory, 
                            PsGetCurrentProcessId(),
                            &LocalHandle, 
                            0,
                            FALSE,
                            DUPLICATE_SAME_ACCESS) == 0)
        {
		// got our duplicated handle so we are done with handle work and can set status and return
		Status = STATUS_SUCCESS;
       		if (LocalHandle != 0) {
          		DPRINT("(PID %lx) IopCreateFile Completed successfully - returning status: 0x%lx handle: %p FileName: %wZ \n", 
                                                   PsGetCurrentProcessId(), Status, LocalHandle, ObjectAttributes->ObjectName );
       		} else {
          		DPRINT("(PID %lx) IopCreateFile Completed successfully - returning status: 0x%lx handle: 0x0 FileName: %wZ \n", 
                                                  PsGetCurrentProcessId(), Status, ObjectAttributes->ObjectName );
       		}
		return Status;
        } else { // failed to duplicate handle so try this the hard way
		DPRINT1("(PID %lx) IopCreateFile call to DuplicateHandle failed, running NtDuplicateObject() logic. \n", PsGetCurrentProcessId() );
*/

/*    Status = NtDuplicateObject(PsGetCurrentProcessId(), //PsGetCurrentProcess(), // SourceProcessHandle this one must be set
					ObjectAttributes->RootDirectory, // SourceHandle 
					NULL, // TargetProcessHandle code shows NULL is valid so try it 
					&LocalHandle, // TargetHandle 
					DesiredAccess, // DesiredAccess 
					ObjectAttributes->Attributes, // HandleAttributes parameter not used in OpenObjByName check if its elsewhere 
					CreateOptions ); // Options, if this doesn't work try NULL here or zero, only used by ObDuplicateObject call  
       if (!NT_SUCCESS(Status)) {
		DPRINT1("(PID %lx) IopCreateFile call to NtDuplicateObject() failed status - 0x%lx. \n", PsGetCurrentProcessId(), Status );
		return Status;
	}
*/

    /* Get File Object - this was my last attempt before moving back to addressing it in Cygwin */
    Status = ObReferenceObjectByHandle(FileHandle,
                                       			     0,
                                       			     NULL, /* IoFileObjectType, */
                                       			     PreviousMode,
                                       			     (PVOID*)&FileObject,
                                       			     &HandleInformation);  /* fails c0000008 invalid handle */
    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObReferenceObjectByHandle for File Handle failed - status: %lx\n", PsGetCurrentProcessId(), 			 
 						Status); 
		return Status;
    }

    Status = ObOpenObjectByPointer(FileObject,
						      HandleInformation.HandleAttributes, /* Duplicate attributes */
						      NULL, /* &HandleInformation.GrantedAccess, wrong type, ACCESS_MASK not PACCESS_STATE */
						      DesiredAccess,
						      NULL, /* Object Type */
						      PreviousMode,
						      &LocalHandle); /* our goal ReturnedHandle OUT */

    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObOpenObjectByPointer using File Handle HandleInformation failed - status: %lx\n", 
        					PsGetCurrentProcessId(), Status); 
        	ObDereferenceObject(FileObject);
		return Status;
    }

    ObDereferenceObject(FileObject);

/* this would have been more to the point, but the ObpCreateHandle() function is not public 
    Status = ObpCreateHandle(ObCreateHandle,
					    FileObject,
					    NULL,
					    &HandleInformation.AccessState,
					    0, // AdditionalReferences ObpOpenObjectByPointer uses zero (0) 
					    HandleInformation.HandleAttributes, // Duplicate attributes 
					    NULL, // Context ObpOpenObjectByPointer uses NULL 
					    PreviousMode,
					    NULL, // ^ReturnedObject ObpOpenObjectByPointer uses NULL
					    &LocalHandle); // our goal ReturnedHandle OUT 

    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObpCreateHandle using File Handle HandleInformation failed - status: %lx\n",
        				    PsGetCurrentProcessId(), Status); 
		return Status;
    }

    ObDereferenceObject(FileObject);
*/

/*  THIS IS THE CODE FOR NtDuplicateObject, currently it fails to reference ObjectByHandle, how do we turn pseudo handle into a real handle ?? */
/*  NOTE: NtDuplicateObject docs indicate IPC is required to xfer handle, but ROS NtDuplicateObject writes to it directly. could that be why it 
gets 0xc0000005 error ? (I stopped using it due to 0xc0000005 error and tried placing it here which is inproc and local to handle but still failed) */
/*

    DPRINT1("(PID %lx) IopCreateFile calling ObReferenceObjectByHandle to reference Source Process Object.\n", PsGetCurrentProcessId());

    Status = ObReferenceObjectByHandle(PsGetCurrentProcessId(), // PsGetCurrentProcess(),
                                       PROCESS_DUP_HANDLE,
                                       PsProcessType,
                                       PreviousMode,
                                       (PVOID*)&SourceProcess,
                                       NULL);

    if (!NT_SUCCESS(Status)) {
        DPRINT1("(PID %lx) IopCreateFile call to ObReferenceObjectByHandle for Source Process failed - status: %lx\n", PsGetCurrentProcessId(), 
        				Status); 
	return Status;
    }

    DPRINT1("(PID %lx) IopCreateFile calling ObDuplicateObject for handle ObjectAttributes->RootDirectory: %p\n", PsGetCurrentProcessId(), 
    				ObjectAttributes->RootDirectory); 

    Status = ObDuplicateObject(SourceProcess,
                               ObjectAttributes->RootDirectory,
                               NULL,
                               &LocalHandle,
                               DesiredAccess,
                               ObjectAttributes->Attributes,
                               CreateOptions,
                               PreviousMode);

    if (!NT_SUCCESS(Status)) {
        DPRINT1("(PID %lx) IopCreateFile call to ObDuplicateObject failed - status: %lx\n", PsGetCurrentProcessId(), Status); 
        ObDereferenceObject(SourceProcess);
	return Status;
    }

    DPRINT1("(PID %lx) IopCreateFile call ObDuplicateObject completed succesfully, Duplicated handle: %p into %p ,releasing Source Process reference.\n", PsGetCurrentProcessId(), ObjectAttributes->RootDirectory, LocalHandle);
    ObDereferenceObject(SourceProcess);
*/
/*  END OF NTDUPLICATE OBJECT CLONE */
//    } // end of DuplicateHandle failure else case
Thanks for your help along the way, I will keep looking for a work around in cygwin code for now..

BugDude
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

bugdude wrote: Wed Feb 17, 2021 11:00 pm That test failing on ROS would essentially confirm that a bug exists in ROS in the API call NtOpenFile(). How should it be formally reported if confirmed ?

There must be something lost if your test worked in ROS. The only obvious difference between the pseudocode and my test in Delphi would be the path and file. What I did is create a folder ReactOS\samba\var and the test file was placed in that folder. It is a TDB database file which is why random access is required, but it only has a header and no data and the random read is reading a header element. Maybe if you use the same path you'll get a failure ?

Since NtOpenFile is basically a forwarder, the code flaw actually lies in the iomanager IopCreateFile() function.

As far as detection goes, I understand that and read that in other forum posts. The rationale is sound in that it avoids people 'blocking' software from running on ROS, but the reverse is also true because this also becomes a blocker for porting useful software onto ROS when the process hits bugs or requires APIs that are not yet implemented. I will see if I can add a config element for it somewhere in a custom Cygwin DLL so a user can selectively enable the workaround, but an automatic detection would have been a far more user-friendly way to do this.

In any event below are a few a failed attempts I made to have IopCreateFile detect this condition and call ObLookupObjectByHandle() the code snippets do not work (because I am not much of a C programmer and can't get all the details right). If you can see how to arm-twist any of these attempts into working it could form the start for a patch to fix this problem in iomanager. The code would be inserted in IopCreateFile() right after the parameter validations are carried out, and would provide a code branch for these 'reopen' type calls.

I hope you won't laugh too hard at my mistakes, I've never studied C so it's coded by plain and simple 'osmosis' learning and I do apologize for that !

Code: Select all

    /* hack to handle Cygwin edge case of re-opening file by passing in existing Handle by deflecting call to ObOpenObjectByHandle() early
        Done early to avoid generating OpenPacket or other structures because we should have most of what we need to duplicate handle already. 
        first check if the pathname is NULL or blank and if so call our hack, if not skip it and fall through to existing code for IopCreateFile()   */
    if (  (ObjectAttributes->ObjectName->Length <= 1 || (!ObjectAttributes->ObjectName->Length) ||  /* detect empty path */
            (!ObjectAttributes->ObjectName) || (!ObjectAttributes->ObjectName->Buffer)) && 
            (ObjectAttributes->RootDirectory != 0) && (ObjectAttributes->RootDirectory != NULL) ) {
	
	DPRINT1("(PID %lx) IopCreateFile called with ObjectAttributes->RootDirectory set and  without FileName in ObjectAttributes->ObjectName, 
                                     calling DuplicateHandle(). \n", PsGetCurrentProcessId() );

/* won't compile, where is DuplicateHandle API function found in ROS ??
        if (DuplicateHandle(PsGetCurrentProcessId(), // according to MS docs DuplicateHandle works with pseudoHandle from GetCurrentProcess()
                            ObjectAttributes->RootDirectory, 
                            PsGetCurrentProcessId(),
                            &LocalHandle, 
                            0,
                            FALSE,
                            DUPLICATE_SAME_ACCESS) == 0)
        {
		// got our duplicated handle so we are done with handle work and can set status and return
		Status = STATUS_SUCCESS;
       		if (LocalHandle != 0) {
          		DPRINT("(PID %lx) IopCreateFile Completed successfully - returning status: 0x%lx handle: %p FileName: %wZ \n", 
                                                   PsGetCurrentProcessId(), Status, LocalHandle, ObjectAttributes->ObjectName );
       		} else {
          		DPRINT("(PID %lx) IopCreateFile Completed successfully - returning status: 0x%lx handle: 0x0 FileName: %wZ \n", 
                                                  PsGetCurrentProcessId(), Status, ObjectAttributes->ObjectName );
       		}
		return Status;
        } else { // failed to duplicate handle so try this the hard way
		DPRINT1("(PID %lx) IopCreateFile call to DuplicateHandle failed, running NtDuplicateObject() logic. \n", PsGetCurrentProcessId() );
*/

/*    Status = NtDuplicateObject(PsGetCurrentProcessId(), //PsGetCurrentProcess(), // SourceProcessHandle this one must be set
					ObjectAttributes->RootDirectory, // SourceHandle 
					NULL, // TargetProcessHandle code shows NULL is valid so try it 
					&LocalHandle, // TargetHandle 
					DesiredAccess, // DesiredAccess 
					ObjectAttributes->Attributes, // HandleAttributes parameter not used in OpenObjByName check if its elsewhere 
					CreateOptions ); // Options, if this doesn't work try NULL here or zero, only used by ObDuplicateObject call  
       if (!NT_SUCCESS(Status)) {
		DPRINT1("(PID %lx) IopCreateFile call to NtDuplicateObject() failed status - 0x%lx. \n", PsGetCurrentProcessId(), Status );
		return Status;
	}
*/

    /* Get File Object - this was my last attempt before moving back to addressing it in Cygwin */
    Status = ObReferenceObjectByHandle(FileHandle,
                                       			     0,
                                       			     NULL, /* IoFileObjectType, */
                                       			     PreviousMode,
                                       			     (PVOID*)&FileObject,
                                       			     &HandleInformation);  /* fails c0000008 invalid handle */
    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObReferenceObjectByHandle for File Handle failed - status: %lx\n", PsGetCurrentProcessId(), 			 
 						Status); 
		return Status;
    }

    Status = ObOpenObjectByPointer(FileObject,
						      HandleInformation.HandleAttributes, /* Duplicate attributes */
						      NULL, /* &HandleInformation.GrantedAccess, wrong type, ACCESS_MASK not PACCESS_STATE */
						      DesiredAccess,
						      NULL, /* Object Type */
						      PreviousMode,
						      &LocalHandle); /* our goal ReturnedHandle OUT */

    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObOpenObjectByPointer using File Handle HandleInformation failed - status: %lx\n", 
        					PsGetCurrentProcessId(), Status); 
        	ObDereferenceObject(FileObject);
		return Status;
    }

    ObDereferenceObject(FileObject);

/* this would have been more to the point, but the ObpCreateHandle() function is not public 
    Status = ObpCreateHandle(ObCreateHandle,
					    FileObject,
					    NULL,
					    &HandleInformation.AccessState,
					    0, // AdditionalReferences ObpOpenObjectByPointer uses zero (0) 
					    HandleInformation.HandleAttributes, // Duplicate attributes 
					    NULL, // Context ObpOpenObjectByPointer uses NULL 
					    PreviousMode,
					    NULL, // ^ReturnedObject ObpOpenObjectByPointer uses NULL
					    &LocalHandle); // our goal ReturnedHandle OUT 

    if (!NT_SUCCESS(Status)) {
        	DPRINT1("(PID %lx) IopCreateFile call to ObpCreateHandle using File Handle HandleInformation failed - status: %lx\n",
        				    PsGetCurrentProcessId(), Status); 
		return Status;
    }

    ObDereferenceObject(FileObject);
*/

/*  THIS IS THE CODE FOR NtDuplicateObject, currently it fails to reference ObjectByHandle, how do we turn pseudo handle into a real handle ?? */
/*  NOTE: NtDuplicateObject docs indicate IPC is required to xfer handle, but ROS NtDuplicateObject writes to it directly. could that be why it 
gets 0xc0000005 error ? (I stopped using it due to 0xc0000005 error and tried placing it here which is inproc and local to handle but still failed) */
/*

    DPRINT1("(PID %lx) IopCreateFile calling ObReferenceObjectByHandle to reference Source Process Object.\n", PsGetCurrentProcessId());

    Status = ObReferenceObjectByHandle(PsGetCurrentProcessId(), // PsGetCurrentProcess(),
                                       PROCESS_DUP_HANDLE,
                                       PsProcessType,
                                       PreviousMode,
                                       (PVOID*)&SourceProcess,
                                       NULL);

    if (!NT_SUCCESS(Status)) {
        DPRINT1("(PID %lx) IopCreateFile call to ObReferenceObjectByHandle for Source Process failed - status: %lx\n", PsGetCurrentProcessId(), 
        				Status); 
	return Status;
    }

    DPRINT1("(PID %lx) IopCreateFile calling ObDuplicateObject for handle ObjectAttributes->RootDirectory: %p\n", PsGetCurrentProcessId(), 
    				ObjectAttributes->RootDirectory); 

    Status = ObDuplicateObject(SourceProcess,
                               ObjectAttributes->RootDirectory,
                               NULL,
                               &LocalHandle,
                               DesiredAccess,
                               ObjectAttributes->Attributes,
                               CreateOptions,
                               PreviousMode);

    if (!NT_SUCCESS(Status)) {
        DPRINT1("(PID %lx) IopCreateFile call to ObDuplicateObject failed - status: %lx\n", PsGetCurrentProcessId(), Status); 
        ObDereferenceObject(SourceProcess);
	return Status;
    }

    DPRINT1("(PID %lx) IopCreateFile call ObDuplicateObject completed succesfully, Duplicated handle: %p into %p ,releasing Source Process reference.\n", PsGetCurrentProcessId(), ObjectAttributes->RootDirectory, LocalHandle);
    ObDereferenceObject(SourceProcess);
*/
/*  END OF NTDUPLICATE OBJECT CLONE */
//    } // end of DuplicateHandle failure else case
Thanks for your help along the way, I will keep looking for a work around in cygwin code for now..

BugDude
Can you link the exact code in cygwin so I can compare it?
learn_more
Developer
Posts: 246
Joined: Fri Dec 19, 2014 10:00 pm

Re: Help diagnosing a problem with cygwin/samba on ROS

Post by learn_more »

Looking at the logging and the snippet you pasted, the open flags do not match at all.
Good luck on your endeavors finding the problem, I am not going to play a game of 'find the actual code'.
Post Reply

Who is online

Users browsing this forum: No registered users and 10 guests