[FREELDR] fs.c: Minor refactoring in ArcOpen(); fix a memory leak (#7385)

- `ArcOpen()`: flatten the registered-device search for-loop.
  Limit it to just the device search, and exit early.
  The rest of the initialization is now done outside the loop.

- The `DeviceName` string pointer may have been allocated from
  the heap, for path normalization (see `NormalizeArcDeviceName()`).
  This pointer is then only used in the device search loop and not
  nused anymore afterwards. The old code didn't free this pointer
  and memory could leak. This is now fixed easily, thanks to the
  loop flattening.

- Rename `DEVICE` member `Prefix` to `DeviceName`; SAL-annotate
  `FsRegisterDevice()`.
This commit is contained in:
Hermès Bélusca-Maïto 2024-09-26 21:32:42 +02:00
parent 689b9e0475
commit 3dfbe52699
No known key found for this signature in database
GPG key ID: 3B2539C65E7B93D0
2 changed files with 77 additions and 64 deletions

View file

@ -52,7 +52,11 @@ FsOpenFile(
ULONG FsGetNumPathParts(PCSTR Path);
VOID FsGetFirstNameFromPath(PCHAR Buffer, PCSTR Path);
VOID FsRegisterDevice(CHAR* Prefix, const DEVVTBL* FuncTable);
VOID
FsRegisterDevice(
_In_ PCSTR DeviceName,
_In_ const DEVVTBL* FuncTable);
PCWSTR FsGetServiceName(ULONG FileId);
VOID FsSetDeviceSpecific(ULONG FileId, VOID* Specific);
VOID* FsGetDeviceSpecific(ULONG FileId);

View file

@ -43,7 +43,7 @@ typedef struct tagDEVICE
{
LIST_ENTRY ListEntry;
const DEVVTBL* FuncTable;
CHAR* Prefix;
PSTR DeviceName;
ULONG DeviceId;
ULONG ReferenceCount;
} DEVICE;
@ -167,18 +167,30 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
if (!DeviceName)
return ENOMEM;
/* Search for the device */
if (OpenMode == OpenReadOnly || OpenMode == OpenWriteOnly)
DeviceOpenMode = OpenMode;
else
DeviceOpenMode = OpenReadWrite;
pEntry = DeviceListHead.Flink;
while (pEntry != &DeviceListHead)
/* Search for the registered device */
pDevice = NULL;
for (pEntry = DeviceListHead.Flink;
pEntry != &DeviceListHead;
pEntry = pEntry->Flink)
{
pDevice = CONTAINING_RECORD(pEntry, DEVICE, ListEntry);
if (strncmp(pDevice->Prefix, DeviceName, Length) == 0)
{
if (strncmp(pDevice->DeviceName, DeviceName, Length) == 0)
break;
}
/* Cleanup */
if (DeviceName != Path)
FrLdrTempFree(DeviceName, TAG_DEVICE_NAME);
DeviceName = NULL;
if (pEntry == &DeviceListHead)
return ENODEV;
/* OK, device found. Is it already opened? */
if (pDevice->ReferenceCount == 0)
{
@ -193,7 +205,7 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
/* Try to open the device */
FileData[DeviceId].FuncTable = pDevice->FuncTable;
Status = pDevice->FuncTable->Open(pDevice->Prefix, DeviceOpenMode, &DeviceId);
Status = pDevice->FuncTable->Open(pDevice->DeviceName, DeviceOpenMode, &DeviceId);
if (Status != ESUCCESS)
{
FileData[DeviceId].FuncTable = NULL;
@ -229,12 +241,6 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
DeviceId = pDevice->DeviceId;
}
pDevice->ReferenceCount++;
break;
}
pEntry = pEntry->Flink;
}
if (pEntry == &DeviceListHead)
return ENODEV;
/* At this point, device is found and opened. Its file id is stored
* in DeviceId, and FileData[DeviceId].FileFuncTable contains what
@ -432,22 +438,25 @@ VOID FsGetFirstNameFromPath(PCHAR Buffer, PCSTR Path)
TRACE("FsGetFirstNameFromPath() Path = %s FirstName = %s\n", Path, Buffer);
}
VOID FsRegisterDevice(CHAR* Prefix, const DEVVTBL* FuncTable)
VOID
FsRegisterDevice(
_In_ PCSTR DeviceName,
_In_ const DEVVTBL* FuncTable)
{
DEVICE* pNewEntry;
SIZE_T Length;
TRACE("FsRegisterDevice() Prefix = %s\n", Prefix);
TRACE("FsRegisterDevice(%s)\n", DeviceName);
Length = strlen(Prefix) + 1;
Length = strlen(DeviceName) + 1;
pNewEntry = FrLdrTempAlloc(sizeof(DEVICE) + Length, TAG_DEVICE);
if (!pNewEntry)
return;
pNewEntry->FuncTable = FuncTable;
pNewEntry->DeviceId = INVALID_FILE_ID;
pNewEntry->ReferenceCount = 0;
pNewEntry->Prefix = (CHAR*)(pNewEntry + 1);
RtlCopyMemory(pNewEntry->Prefix, Prefix, Length);
pNewEntry->DeviceName = (PSTR)(pNewEntry + 1);
RtlCopyMemory(pNewEntry->DeviceName, DeviceName, Length);
InsertHeadList(&DeviceListHead, &pNewEntry->ListEntry);
}