Skip to content

Commit

Permalink
[FREELDR] fs.c: Minor refactoring in ArcOpen(); fix a memory leak (re…
Browse files Browse the repository at this point in the history
…actos#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()`.
  • Loading branch information
HBelusca committed Oct 1, 2024
1 parent 29dad35 commit 465aaa1
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 67 deletions.
6 changes: 5 additions & 1 deletion boot/freeldr/freeldr/include/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
138 changes: 72 additions & 66 deletions boot/freeldr/freeldr/lib/fs/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ typedef struct tagDEVICE
{
LIST_ENTRY ListEntry;
const DEVVTBL* FuncTable;
CHAR* Prefix;
PSTR DeviceName;
ULONG DeviceId;
ULONG ReferenceCount;
} DEVICE;
Expand Down Expand Up @@ -167,78 +167,81 @@ 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)
{
/* OK, device found. Is it already opened? */
if (pDevice->ReferenceCount == 0)
{
/* Find some room for the device */
for (DeviceId = 0; ; ++DeviceId)
{
if (DeviceId >= _countof(FileData))
return EMFILE;
if (!FileData[DeviceId].FuncTable)
break;
}

/* Try to open the device */
FileData[DeviceId].FuncTable = pDevice->FuncTable;
Status = pDevice->FuncTable->Open(pDevice->Prefix, DeviceOpenMode, &DeviceId);
if (Status != ESUCCESS)
{
FileData[DeviceId].FuncTable = NULL;
return Status;
}
else if (!*FileName)
{
/* Done, caller wanted to open the raw device */
*FileId = DeviceId;
pDevice->ReferenceCount++;
return ESUCCESS;
}

/* Try to detect the file system */
{
ULONG fs;
for (fs = 0; fs < _countof(FileSystems); ++fs)
{
FileData[DeviceId].FileFuncTable = FileSystems[fs](DeviceId);
if (FileData[DeviceId].FileFuncTable)
break;
}
}
if (!FileData[DeviceId].FileFuncTable)
{
/* Error, unable to detect the file system */
pDevice->FuncTable->Close(DeviceId);
FileData[DeviceId].FuncTable = NULL;
return ENODEV;
}

pDevice->DeviceId = DeviceId;
}
else
{
DeviceId = pDevice->DeviceId;
}
pDevice->ReferenceCount++;
if (strncmp(pDevice->DeviceName, DeviceName, Length) == 0)
break;
}
pEntry = pEntry->Flink;
}

/* 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)
{
/* Find some room for the device */
for (DeviceId = 0; ; ++DeviceId)
{
if (DeviceId >= _countof(FileData))
return EMFILE;
if (!FileData[DeviceId].FuncTable)
break;
}

/* Try to open the device */
FileData[DeviceId].FuncTable = pDevice->FuncTable;
Status = pDevice->FuncTable->Open(pDevice->DeviceName, DeviceOpenMode, &DeviceId);
if (Status != ESUCCESS)
{
FileData[DeviceId].FuncTable = NULL;
return Status;
}
else if (!*FileName)
{
/* Done, caller wanted to open the raw device */
*FileId = DeviceId;
pDevice->ReferenceCount++;
return ESUCCESS;
}

/* Try to detect the file system */
for (ULONG fs = 0; fs < _countof(FileSystems); ++fs)
{
FileData[DeviceId].FileFuncTable = FileSystems[fs](DeviceId);
if (FileData[DeviceId].FileFuncTable)
break;
}
if (!FileData[DeviceId].FileFuncTable)
{
/* Error, unable to detect the file system */
pDevice->FuncTable->Close(DeviceId);
FileData[DeviceId].FuncTable = NULL;
return ENODEV;
}

pDevice->DeviceId = DeviceId;
}
else
{
DeviceId = pDevice->DeviceId;
}
pDevice->ReferenceCount++;

/* At this point, device is found and opened. Its file id is stored
* in DeviceId, and FileData[DeviceId].FileFuncTable contains what
* needs to be called to open the file */
Expand Down Expand Up @@ -435,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);
}
Expand Down

0 comments on commit 465aaa1

Please sign in to comment.