| Submitter | Aaron Williams |
|---|---|
| Date | May 3, 2012, 2:17 a.m. |
| Message ID | <4FA1EAC5.8000003@cavium.com> |
| Download | mbox | patch |
| Permalink | /patch/156595/ |
| State | Superseded |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
Comments
Any comments on this patch? On 05/02/2012 07:17 PM, Aaron Williams wrote: > This patch fixes several issues where sector offsets can overflow due to > being limited to 16-bits. There are many cases which can cause an > overflow, including large FAT32 partitions and partitions that start at > a sufficiently large offset on the storage device. > > Numerous issues were observed and fixed when a 64GB FAT32 filesystem was > accessed due to truncation. > > Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com> > --- > include/fat.h | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/fat.h b/include/fat.h > index 4c92442..7215628 100644 > --- a/include/fat.h > +++ b/include/fat.h > @@ -178,12 +178,12 @@ typedef struct dir_slot { > typedef struct { > __u8 *fatbuf; /* Current FAT buffer */ > int fatsize; /* Size of FAT in bits */ > - __u16 fatlength; /* Length of FAT in sectors */ > - __u16 fat_sect; /* Starting sector of the FAT */ > - __u16 rootdir_sect; /* Start sector of root directory */ > - __u16 sect_size; /* Size of sectors in bytes */ > + __u32 fat_sect; /* Starting sector of the FAT */ > + __u32 rootdir_sect; /* Start sector of root directory */ > + __u32 fatlength; /* Length of FAT in sectors */ > __u16 clust_size; /* Size of clusters in sectors */ > - short data_begin; /* The sector of the first cluster, can be negative */ > + __u16 sect_size; /* Size of sectors in bytes */ > + int data_begin; /* The sector of the first cluster, can be negative */ > int fatbufnum; /* Used by get_fatent, init to -1 */ > } fsdata; >
Hello, On Wed, 02 May 2012 19:17:41 -0700 Aaron Williams <Aaron.Williams@cavium.com> wrote: > This patch fixes several issues where sector offsets can overflow due to > being limited to 16-bits. There are many cases which can cause an > overflow, including large FAT32 partitions and partitions that start at > a sufficiently large offset on the storage device. For large FAT32 partitions only changing of fatlength, rootdir_sect and data_begin is needed to avoid overflows. Changing of fat_sect shouldn't be needed. What do you mean exactly by "partitions starting at a sufficiently large offset on the storage device"? How do you create such partition? I've tested with a 210 GB FAT32 partition as the fourth primary partition on a 2 TB disk. This partition is the last partition on the disk, so its offset is sufficiently large. For this test only fatlength, rootdir_sect and data_begin was changed to __u32 and int and I do not see issues when listing or loading the files from this partition. > Numerous issues were observed and fixed when a 64GB FAT32 filesystem was > accessed due to truncation. > > Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com> > --- > include/fat.h | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/fat.h b/include/fat.h > index 4c92442..7215628 100644 > --- a/include/fat.h > +++ b/include/fat.h > @@ -178,12 +178,12 @@ typedef struct dir_slot { > typedef struct { > __u8 *fatbuf; /* Current FAT buffer */ > int fatsize; /* Size of FAT in bits */ > - __u16 fatlength; /* Length of FAT in sectors */ > - __u16 fat_sect; /* Starting sector of the FAT */ > - __u16 rootdir_sect; /* Start sector of root directory */ > - __u16 sect_size; /* Size of sectors in bytes */ > + __u32 fat_sect; /* Starting sector of the FAT */ > + __u32 rootdir_sect; /* Start sector of root directory */ > + __u32 fatlength; /* Length of FAT in sectors */ > __u16 clust_size; /* Size of clusters in sectors */ > - short data_begin; /* The sector of the first cluster, can be negative */ > + __u16 sect_size; /* Size of sectors in bytes */ > + int data_begin; /* The sector of the first cluster, can be negative */ > int fatbufnum; /* Used by get_fatent, init to -1 */ > } fsdata; The patch is probably corrupted by your mailer, it doesn't apply. Thanks, Anatolij
Hi Anatolij, On 05/12/2012 08:41 AM, Anatolij Gustschin wrote: > Hello, > > On Wed, 02 May 2012 19:17:41 -0700 > Aaron Williams <Aaron.Williams@cavium.com> wrote: > >> This patch fixes several issues where sector offsets can overflow due to >> being limited to 16-bits. There are many cases which can cause an >> overflow, including large FAT32 partitions and partitions that start at >> a sufficiently large offset on the storage device. > For large FAT32 partitions only changing of fatlength, rootdir_sect and > data_begin is needed to avoid overflows. Changing of fat_sect shouldn't > be needed. > > What do you mean exactly by "partitions starting at a sufficiently large > offset on the storage device"? How do you create such partition? > I've tested with a 210 GB FAT32 partition as the fourth primary partition > on a 2 TB disk. This partition is the last partition on the disk, so its > offset is sufficiently large. For this test only fatlength, rootdir_sect > and data_begin was changed to __u32 and int and I do not see issues when > listing or loading the files from this partition. You are correct about fat_sect, I misread the code and changed that one as well. I don't think it really matters since another changing it back to a __u16 won't save any space in the data structure on most platforms. It looks like the corruption problems I was seeing were due to the fact that the file I was reading was after a number of very large files in a 64GB FAT32 partition and I ran into overflows. The partition location shouldn't have any impact unless we're talking about 2+TB drives, in which case I expect there will be numerous other things that will break. >> Numerous issues were observed and fixed when a 64GB FAT32 filesystem was >> accessed due to truncation. >> >> Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com> >> --- >> include/fat.h | 10 +++++----- >> 1 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/include/fat.h b/include/fat.h >> index 4c92442..7215628 100644 >> --- a/include/fat.h >> +++ b/include/fat.h >> @@ -178,12 +178,12 @@ typedef struct dir_slot { >> typedef struct { >> __u8 *fatbuf; /* Current FAT buffer */ >> int fatsize; /* Size of FAT in bits */ >> - __u16 fatlength; /* Length of FAT in sectors */ >> - __u16 fat_sect; /* Starting sector of the FAT */ >> - __u16 rootdir_sect; /* Start sector of root directory */ >> - __u16 sect_size; /* Size of sectors in bytes */ >> + __u32 fat_sect; /* Starting sector of the FAT */ >> + __u32 rootdir_sect; /* Start sector of root directory */ >> + __u32 fatlength; /* Length of FAT in sectors */ >> __u16 clust_size; /* Size of clusters in sectors */ >> - short data_begin; /* The sector of the first cluster, can be negative */ >> + __u16 sect_size; /* Size of sectors in bytes */ >> + int data_begin; /* The sector of the first cluster, can be negative */ >> int fatbufnum; /* Used by get_fatent, init to -1 */ >> } fsdata; > The patch is probably corrupted by your mailer, it doesn't apply. > > Thanks, > Anatolij > Unfortunately our outbound Exchange mail server tends to "fix" mail formatting making patch submission rather difficult. I'll see what I can do to try and bypass it. -Aaron
Patch
diff --git a/include/fat.h b/include/fat.h index 4c92442..7215628 100644 --- a/include/fat.h +++ b/include/fat.h @@ -178,12 +178,12 @@ typedef struct dir_slot { typedef struct { __u8 *fatbuf; /* Current FAT buffer */ int fatsize; /* Size of FAT in bits */ - __u16 fatlength; /* Length of FAT in sectors */ - __u16 fat_sect; /* Starting sector of the FAT */ - __u16 rootdir_sect; /* Start sector of root directory */ - __u16 sect_size; /* Size of sectors in bytes */ + __u32 fat_sect; /* Starting sector of the FAT */ + __u32 rootdir_sect; /* Start sector of root directory */ + __u32 fatlength; /* Length of FAT in sectors */ __u16 clust_size; /* Size of clusters in sectors */ - short data_begin; /* The sector of the first cluster, can be negative */ + __u16 sect_size; /* Size of sectors in bytes */ + int data_begin; /* The sector of the first cluster, can be negative */ int fatbufnum; /* Used by get_fatent, init to -1 */ } fsdata;
This patch fixes several issues where sector offsets can overflow due to being limited to 16-bits. There are many cases which can cause an overflow, including large FAT32 partitions and partitions that start at a sufficiently large offset on the storage device. Numerous issues were observed and fixed when a 64GB FAT32 filesystem was accessed due to truncation. Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com> --- include/fat.h | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)