From f0166aaf6e893cf830ace9019ec713f26eea8a33 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Thu, 27 Jul 2006 00:13:14 +0000 Subject: [PATCH] - Fix multiple bugs in VfatGetNameInformation: * Return the file name length even if the buffer is too small, that's the whole point of the "Query length before comitting a buffer" principle. * FSDs are not supposed to null-terminate the buffer, nor expect the caller to send a buffer large enough for null-termination. * Added a hack in IopQueryFile to handle another VFAT bug which makes it return the total number of bytes written in IoStatus.Information instead of the total number of bytes *left untouched*.There are probably many other broken things due to this. - Fix some length calculation bugs in IopQueryFile. svn path=/trunk/; revision=23302 --- reactos/drivers/filesystems/vfat/finfo.c | 7 +++---- reactos/ntoskrnl/io/iomgr/file.c | 17 ++++++++++++++--- reactos/ntoskrnl/ob/obname.c | 2 +- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/reactos/drivers/filesystems/vfat/finfo.c b/reactos/drivers/filesystems/vfat/finfo.c index 055e097d6ec..36ad1ee9a74 100644 --- a/reactos/drivers/filesystems/vfat/finfo.c +++ b/reactos/drivers/filesystems/vfat/finfo.c @@ -341,14 +341,13 @@ VfatGetNameInformation(PFILE_OBJECT FileObject, ASSERT(NameInfo != NULL); ASSERT(FCB != NULL); - if (*BufferLength < sizeof(FILE_NAME_INFORMATION) + FCB->PathNameU.Length + sizeof(WCHAR)) + NameInfo->FileNameLength = FCB->PathNameU.Length; + if (*BufferLength < FIELD_OFFSET(FILE_NAME_INFORMATION, FileName[0]) + FCB->PathNameU.Length) return STATUS_BUFFER_OVERFLOW; - NameInfo->FileNameLength = FCB->PathNameU.Length; RtlCopyMemory(NameInfo->FileName, FCB->PathNameU.Buffer, FCB->PathNameU.Length); - NameInfo->FileName[FCB->PathNameU.Length / sizeof(WCHAR)] = 0; - *BufferLength -= (sizeof(FILE_NAME_INFORMATION) + FCB->PathNameU.Length + sizeof(WCHAR)); + *BufferLength -= (FIELD_OFFSET(FILE_NAME_INFORMATION, FileName[0]) + FCB->PathNameU.Length); return STATUS_SUCCESS; } diff --git a/reactos/ntoskrnl/io/iomgr/file.c b/reactos/ntoskrnl/io/iomgr/file.c index 19ca48d2fa2..b228bf21ffd 100644 --- a/reactos/ntoskrnl/io/iomgr/file.c +++ b/reactos/ntoskrnl/io/iomgr/file.c @@ -915,7 +915,7 @@ IopQueryNameFile(IN PVOID ObjectBody, LocalInfo, Length, &LocalReturnLength); - if (!NT_SUCCESS (Status)) + if (!NT_SUCCESS(Status)) { /* Free the buffer and fail */ ExFreePool(LocalInfo); @@ -935,6 +935,14 @@ IopQueryNameFile(IN PVOID ObjectBody, /* Advance in buffer */ p += (LocalInfo->Name.Length / sizeof(WCHAR)); + /* Check if this already filled our buffer */ + if (LocalReturnLength > Length) + { + /* Free the buffer and fail */ + ExFreePool(LocalInfo); + return STATUS_BUFFER_OVERFLOW; + } + /* Now get the file name buffer and check the length needed */ LocalFileInfo = (PFILE_NAME_INFORMATION)LocalInfo; FileLength = Length - @@ -944,7 +952,7 @@ IopQueryNameFile(IN PVOID ObjectBody, /* Query the File name */ Status = IoQueryFileInformation(FileObject, FileNameInformation, - Length, + FileLength, LocalFileInfo, &LocalReturnLength); if (NT_ERROR(Status)) @@ -954,6 +962,9 @@ IopQueryNameFile(IN PVOID ObjectBody, return Status; } + /* ROS HACK. VFAT SUCKS */ + if (NT_WARNING(Status)) LocalReturnLength = FileLength; + /* Now calculate the new lenghts left */ FileLength = LocalReturnLength - FIELD_OFFSET(FILE_NAME_INFORMATION, FileName); @@ -972,7 +983,7 @@ IopQueryNameFile(IN PVOID ObjectBody, /* Setup the length and maximum length */ FileLength = (ULONG_PTR)p - (ULONG_PTR)ObjectNameInfo; - ObjectNameInfo->Name.Length = Length - sizeof(OBJECT_NAME_INFORMATION); + ObjectNameInfo->Name.Length = FileLength - sizeof(OBJECT_NAME_INFORMATION); ObjectNameInfo->Name.MaximumLength = ObjectNameInfo->Name.Length + sizeof(UNICODE_NULL); diff --git a/reactos/ntoskrnl/ob/obname.c b/reactos/ntoskrnl/ob/obname.c index 6ab686164af..fe15162fc0b 100644 --- a/reactos/ntoskrnl/ob/obname.c +++ b/reactos/ntoskrnl/ob/obname.c @@ -620,7 +620,7 @@ ObQueryNameString(IN PVOID Object, } /* Check if the object doesn't even have a name */ - if (!LocalInfo || !LocalInfo->Name.Buffer) + if (!(LocalInfo) || !(LocalInfo->Name.Buffer)) { /* We're returning the name structure */ *ReturnLength = sizeof(OBJECT_NAME_INFORMATION);