Matteo Croce <mcr...@redhat.com> writes:

> The FAT32 File System Specification[1] states that:
>
>     If DIR_Name[0] == 0x00, then the directory entry is free, and there
>     are no allocated directory entries after this one.
>
>     The special 0 value, indicates to FAT file system driver code that
>     the rest of the entries in this directory do not need to be examined
>     because they are all free.
>
> This is not enforced by Linux, and is possible to read garbage if not
> all dirents after the last one are filled with zeroes.
>
> [1] 
> http://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/fatgen103.doc

Tested some, and this patch seems to be not working.  First
fat_readdir() exits by zero dir entry, but dir pos is updated to next
position, then second fat_readdir() continue with previous dir pos and
will read beyond zero dir entry.

So, maybe, we are better move down the zero dir entry handling into
fat_get_entry(), instead of copying several places. But like you know,
we would like to skip the zero dir entry only if read/search operation.

Maybe, adding flag to fat_get_entry() and use it, or adding small
wrapper? And in read/search case, we should not update *pos if hit the
zero dir entry.

I'm not testing the above actually though, we would need such.

Thanks.

> Reported-by: Timothy Redaelli <tredae...@redhat.com>
> Signed-off-by: Matteo Croce <mcr...@redhat.com>
> ---
>  fs/fat/dir.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 9d01db37183f..d919a1ee519c 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -314,7 +314,7 @@ static int fat_parse_long(struct inode *dir, loff_t *pos,
>  
>               if (ds->id & 0x40)
>                       (*unicode)[offset + 13] = 0;
> -             if (fat_get_entry(dir, pos, bh, de) < 0)
> +             if (fat_get_entry(dir, pos, bh, de) < 0 || !(*de)->name[0])
>                       return PARSE_EOF;
>               if (slot == 0)
>                       break;
> @@ -476,7 +476,8 @@ int fat_search_long(struct inode *inode, const unsigned 
> char *name,
>  
>       err = -ENOENT;
>       while (1) {
> -             if (fat_get_entry(inode, &cpos, &bh, &de) == -1)
> +             if (fat_get_entry(inode, &cpos, &bh, &de) == -1 ||
> +                 !de->name[0])
>                       goto end_of_dir;
>  parse_record:
>               nr_slots = 0;
> @@ -588,7 +589,7 @@ static int __fat_readdir(struct inode *inode, struct file 
> *file,
>  
>       bh = NULL;
>  get_new:
> -     if (fat_get_entry(inode, &cpos, &bh, &de) == -1)
> +     if (fat_get_entry(inode, &cpos, &bh, &de) == -1 || !de->name[0])
>               goto end_of_dir;
>  parse_record:
>       nr_slots = 0;
> @@ -898,7 +899,8 @@ int fat_get_dotdot_entry(struct inode *dir, struct 
> buffer_head **bh,
>       loff_t offset = 0;
>  
>       *de = NULL;
> -     while (fat_get_short_entry(dir, &offset, bh, de) >= 0) {
> +     while (fat_get_short_entry(dir, &offset, bh, de) >= 0 &&
> +            (*de)->name[0]) {
>               if (!strncmp((*de)->name, MSDOS_DOTDOT, MSDOS_NAME))
>                       return 0;
>       }
> @@ -916,7 +918,8 @@ int fat_dir_empty(struct inode *dir)
>  
>       bh = NULL;
>       cpos = 0;
> -     while (fat_get_short_entry(dir, &cpos, &bh, &de) >= 0) {
> +     while (fat_get_short_entry(dir, &cpos, &bh, &de) >= 0 &&
> +            de->name[0]) {
>               if (strncmp(de->name, MSDOS_DOT   , MSDOS_NAME) &&
>                   strncmp(de->name, MSDOS_DOTDOT, MSDOS_NAME)) {
>                       result = -ENOTEMPTY;
> @@ -941,7 +944,8 @@ int fat_subdirs(struct inode *dir)
>  
>       bh = NULL;
>       cpos = 0;
> -     while (fat_get_short_entry(dir, &cpos, &bh, &de) >= 0) {
> +     while (fat_get_short_entry(dir, &cpos, &bh, &de) >= 0 &&
> +            de->name[0]) {
>               if (de->attr & ATTR_DIR)
>                       count++;
>       }
> @@ -961,7 +965,7 @@ int fat_scan(struct inode *dir, const unsigned char *name,
>       sinfo->slot_off = 0;
>       sinfo->bh = NULL;
>       while (fat_get_short_entry(dir, &sinfo->slot_off, &sinfo->bh,
> -                                &sinfo->de) >= 0) {
> +                                &sinfo->de) >= 0 && sinfo->de->name[0]) {
>               if (!strncmp(sinfo->de->name, name, MSDOS_NAME)) {
>                       sinfo->slot_off -= sizeof(*sinfo->de);
>                       sinfo->nr_slots = 1;
> @@ -985,7 +989,7 @@ int fat_scan_logstart(struct inode *dir, int i_logstart,
>       sinfo->slot_off = 0;
>       sinfo->bh = NULL;
>       while (fat_get_short_entry(dir, &sinfo->slot_off, &sinfo->bh,
> -                                &sinfo->de) >= 0) {
> +                                &sinfo->de) >= 0 && sinfo->de->name[0]) {
>               if (fat_get_start(MSDOS_SB(sb), sinfo->de) == i_logstart) {
>                       sinfo->slot_off -= sizeof(*sinfo->de);
>                       sinfo->nr_slots = 1;

-- 
OGAWA Hirofumi <hirof...@mail.parknet.co.jp>

Reply via email to