Browse Source

lib: fatlib: Avoid unaligned access to LFN name fields

GCC 9 now starts emitting warnings (turned errors) because we grab the
address of the Long File Name FAT directory entries, which are PUSHORTs,
but are not aligned. We then access those pointers as an array. We do
(at least in all cases that mattered) use the READ_UNALIGNED16() macro,
but GCC isn't smart enough to see that we're careful with how we use the
pointer.

To fix, convert to a char pointer, and access with care, as before.

Also, FAT_DIRECTORY_ENTRY structures are always aligned to 32-byte
boundaries, so mark their structs as such if it helps with other fields.
Evan Green 3 years ago
parent
commit
f62dcb5da0
4 changed files with 37 additions and 35 deletions
  1. 11 9
      boot/fatboot/fatboot.c
  2. 2 2
      include/minoca/lib/fat/fatlib.h
  3. 16 16
      lib/fatlib/fatsup.c
  4. 8 8
      uefi/plat/panda/init/fatboot.c

+ 11 - 9
boot/fatboot/fatboot.c

@@ -863,7 +863,7 @@ Return Value:
     PFAT_LONG_DIRECTORY_ENTRY LongEntry;
     UCHAR LongEntryChecksum;
     ULONG NameIndex;
-    PUSHORT Region;
+    PUCHAR Region;
     ULONG RegionIndex;
     ULONG RegionSize;
     UCHAR Sequence;
@@ -896,27 +896,29 @@ Return Value:
             NameIndex = *State;
             for (RegionIndex = 0; RegionIndex < 3; RegionIndex += 1) {
                 if (RegionIndex == 0) {
-                    Region = LongEntry->Name1;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME1_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name1);
+                    RegionSize = sizeof(LongEntry->Name1);
 
                 } else if (RegionIndex == 1) {
-                    Region = LongEntry->Name2;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME2_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name2);
+                    RegionSize = sizeof(LongEntry->Name2);
 
                 } else {
-                    Region = LongEntry->Name3;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME3_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name3);
+                    RegionSize = sizeof(LongEntry->Name3);
                 }
 
                 for (CharacterIndex = 0;
                      CharacterIndex < RegionSize;
-                     CharacterIndex += 1) {
+                     CharacterIndex += sizeof(USHORT)) {
 
                     if (Name[NameIndex] == '\0') {
                         break;
                     }
 
-                    if (Region[CharacterIndex] != Name[NameIndex]) {
+                    if (READ_UNALIGNED16(&(Region[CharacterIndex])) !=
+                        Name[NameIndex]) {
+
                         return FALSE;
                     }
 

+ 2 - 2
include/minoca/lib/fat/fatlib.h

@@ -551,7 +551,7 @@ typedef struct _FAT_DIRECTORY_ENTRY {
     USHORT LastModifiedDate;
     USHORT ClusterLow;
     ULONG FileSizeInBytes;
-} PACKED FAT_DIRECTORY_ENTRY, *PFAT_DIRECTORY_ENTRY;
+} PACKED ALIGNED32 FAT_DIRECTORY_ENTRY, *PFAT_DIRECTORY_ENTRY;
 
 /*++
 
@@ -595,7 +595,7 @@ typedef struct _FAT_LONG_DIRECTORY_ENTRY {
     USHORT Name2[FAT_LONG_DIRECTORY_ENTRY_NAME2_SIZE];
     USHORT Cluster;
     USHORT Name3[FAT_LONG_DIRECTORY_ENTRY_NAME3_SIZE];
-} PACKED FAT_LONG_DIRECTORY_ENTRY, *PFAT_LONG_DIRECTORY_ENTRY;
+} PACKED ALIGNED32 FAT_LONG_DIRECTORY_ENTRY, *PFAT_LONG_DIRECTORY_ENTRY;
 
 #pragma pack(pop)
 

+ 16 - 16
lib/fatlib/fatsup.c

@@ -872,7 +872,7 @@ Return Value:
     ULONG RegionIndex;
     UCHAR Sequence;
     UCHAR ShortNameChecksum;
-    PUSHORT Source;
+    PUCHAR Source;
     ULONG SourceCount;
     ULONG SourceIndex;
     ULONG SourceSize;
@@ -981,21 +981,21 @@ Return Value:
 
             for (RegionIndex = 0; RegionIndex < 3; RegionIndex += 1) {
                 if (RegionIndex == 0) {
-                    Source = LongEntry->Name1;
-                    SourceSize = FAT_LONG_DIRECTORY_ENTRY_NAME1_SIZE;
+                    Source = (PUCHAR)&(LongEntry->Name1);
+                    SourceSize = sizeof(LongEntry->Name1);
 
                 } else if (RegionIndex == 1) {
-                    Source = LongEntry->Name2;
-                    SourceSize = FAT_LONG_DIRECTORY_ENTRY_NAME2_SIZE;
+                    Source = (PUCHAR)&(LongEntry->Name2);
+                    SourceSize = sizeof(LongEntry->Name2);
 
                 } else {
-                    Source = LongEntry->Name3;
-                    SourceSize = FAT_LONG_DIRECTORY_ENTRY_NAME3_SIZE;
+                    Source = (PUCHAR)&(LongEntry->Name3);
+                    SourceSize = sizeof(LongEntry->Name3);
                 }
 
                 for (SourceIndex = 0;
                      SourceIndex < SourceSize;
-                     SourceIndex += 1) {
+                     SourceIndex += sizeof(USHORT)) {
 
                     FileName[CharacterIndex] =
                                   (CHAR)FAT_READ_INT16(&(Source[SourceIndex]));
@@ -3695,7 +3695,7 @@ Return Value:
 
     USHORT Character;
     ULONG CharacterIndex;
-    PUSHORT Destination;
+    PUCHAR Destination;
     ULONG DestinationIndex;
     ULONG DestinationSize;
     PFAT_DIRECTORY_ENTRY Entries;
@@ -4010,21 +4010,21 @@ Return Value:
 
         for (RegionIndex = 0; RegionIndex < 3; RegionIndex += 1) {
             if (RegionIndex == 0) {
-                Destination = LongEntry->Name1;
-                DestinationSize = FAT_LONG_DIRECTORY_ENTRY_NAME1_SIZE;
+                Destination = (PUCHAR)&(LongEntry->Name1);
+                DestinationSize = sizeof(LongEntry->Name1);
 
             } else if (RegionIndex == 1) {
-                Destination = LongEntry->Name2;
-                DestinationSize = FAT_LONG_DIRECTORY_ENTRY_NAME2_SIZE;
+                Destination = (PUCHAR)&(LongEntry->Name2);
+                DestinationSize = sizeof(LongEntry->Name2);
 
             } else {
-                Destination = LongEntry->Name3;
-                DestinationSize = FAT_LONG_DIRECTORY_ENTRY_NAME3_SIZE;
+                Destination = (PUCHAR)&(LongEntry->Name3);
+                DestinationSize = sizeof(LongEntry->Name3);
             }
 
             for (DestinationIndex = 0;
                  DestinationIndex < DestinationSize;
-                 DestinationIndex += 1) {
+                 DestinationIndex += sizeof(USHORT)) {
 
                 if (CharacterIndex < FileNameLength) {
                     Character = FileName[CharacterIndex];

+ 8 - 8
uefi/plat/panda/init/fatboot.c

@@ -731,7 +731,7 @@ Return Value:
     PFAT_LONG_DIRECTORY_ENTRY LongEntry;
     UCHAR LongEntryChecksum;
     ULONG NameIndex;
-    PUSHORT Region;
+    PUCHAR Region;
     ULONG RegionIndex;
     ULONG RegionSize;
     UCHAR Sequence;
@@ -764,21 +764,21 @@ Return Value:
             NameIndex = *State;
             for (RegionIndex = 0; RegionIndex < 3; RegionIndex += 1) {
                 if (RegionIndex == 0) {
-                    Region = LongEntry->Name1;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME1_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name1);
+                    RegionSize = sizeof(LongEntry->Name1);
 
                 } else if (RegionIndex == 1) {
-                    Region = LongEntry->Name2;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME2_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name2);
+                    RegionSize = sizeof(LongEntry->Name2);
 
                 } else {
-                    Region = LongEntry->Name3;
-                    RegionSize = FAT_LONG_DIRECTORY_ENTRY_NAME3_SIZE;
+                    Region = (PUCHAR)&(LongEntry->Name3);
+                    RegionSize = sizeof(LongEntry->Name3);
                 }
 
                 for (CharacterIndex = 0;
                      CharacterIndex < RegionSize;
-                     CharacterIndex += 1) {
+                     CharacterIndex += sizeof(USHORT)) {
 
                     if (Name[NameIndex] == '\0') {
                         break;