Trunk Foobared

[Alex Ionescu]

Hi,

I am aware that I broke trunk. This is because I've removed the code in
ObReferenceObjectByHandle which allowed GENERIC access masks to be
converted. This API does NOT support GENERIC access masks and converting
to them was incorrect, however, as always, plenty of code in ROS abused
the system and calls this API incorrectly. Although ObOpenObjectByName
should return a handle and IoCreateFile should be done with it, because
parsing is broken (so it creeps up again!), we try to re-reference it by
handle to get its pointer... however, this attempt is made with the
current AccessMode, which happens to be User, and if the AccessMask was
GENERIC... we fail. ObOpenObjectByName is made to be used with GENERIC
access masks so that's really not a problem. Hopefully the I/O File
stuff is the only place where this happens, so I will implement a simple
hack - call ObReferenceObjectByHandle with a KernelMode parameter so
that access checks are skipped (we've already done them in
ObOpenObjectByName anyways!).

Best regards,
Alex Ionescu

[Alex Ionescu]

Alex Ionescu wrote:
> Hi,
>
> I am aware that I broke trunk. This is because I've removed the code in
> ObReferenceObjectByHandle which allowed GENERIC access masks to be
> converted. This API does NOT support GENERIC access masks and converting
> to them was incorrect, however, as always, plenty of code in ROS abused
> the system and calls this API incorrectly. Although ObOpenObjectByName
> should return a handle and IoCreateFile should be done with it, because
> parsing is broken (so it creeps up again!), we try to re-reference it by
> handle to get its pointer... however, this attempt is made with the
> current AccessMode, which happens to be User, and if the AccessMask was
> GENERIC... we fail. ObOpenObjectByName is made to be used with GENERIC
> access masks so that's really not a problem. Hopefully the I/O File
> stuff is the only place where this happens, so I will implement a simple
> hack - call ObReferenceObjectByHandle with a KernelMode parameter so
> that access checks are skipped (we've already done them in
> ObOpenObjectByName anyways!).
>
> Best regards,
> Alex Ionescu
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

Unofficial test app for the skeptical:

NTSTATUS
DriverEntry(IN PDRIVER_OBJECT DriverObject,
            IN PUNICODE_STRING RegistryPath)
{
    HANDLE File;
    NTSTATUS Status;
    PVOID KmodeFile, UmodeFile;
    OBJECT_ATTRIBUTES ObjectAttributes;
    UNICODE_STRING FileName =
RTL_CONSTANT_STRING(L"\GLOBAL??\c:\test2.txt");
    IO_STATUS_BLOCK IoStatusBlock;

//
    // Make ourselves noticed
    //
    DbgPrint("I was loaded. Current Mode: %lx!\n", ExGetPreviousMode());

//
    // Create a file
    //
    InitializeObjectAttributes(&ObjectAttributes,
                               &FileName,
                               OBJ_CASE_INSENSITIVE,
                               NULL,
                               NULL);
    Status = NtCreateFile(&File,
                          GENERIC_ALL,
                          &ObjectAttributes,
                          &IoStatusBlock,
                          NULL,
                          FILE_ATTRIBUTE_NORMAL,
                          0,
                          FILE_CREATE,
                          FILE_NON_DIRECTORY_FILE |
FILE_SYNCHRONOUS_IO_NONALERT,
                          NULL,
                          0);
    DbgPrint("Status: %lx %lx!\n", Status, File);

//
    // Get a pointer to it in for kernel-mode
    //
    Status = ObReferenceObjectByHandle(File, GENERIC_READ, NULL,
KernelMode, &KmodeFile, NULL);
    DbgPrint("Status: %lx %lx!\n", Status, KmodeFile);

//
    // Get a pointer to it in for user-mode
    //
    Status = ObReferenceObjectByHandle(File, GENERIC_READ, NULL,
UserMode, &UmodeFile, NULL);
    DbgPrint("Status: %lx %lx!\n", Status, UmodeFile);

//
    // Return
    //
    return STATUS_DEVICE_CONFIGURATION_ERROR;
}

You'll see the second call failing just as it does in ReactOS now, even
though GENERIC_ALL should give access to GENERIC_READ requests.

Best regards,
Alex Ionescu