LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH] vfat: don't read garbage after last dirent
@ 2018-12-07  1:34 Matteo Croce
  2018-12-08 17:16 ` Matteo Croce
  2018-12-14  3:41 ` OGAWA Hirofumi
  0 siblings, 2 replies; 3+ messages in thread
From: Matteo Croce @ 2018-12-07  1:34 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: Timothy Redaelli, linux-kernel

The FAT32 File System Specification[1] states:

    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 notneed to be examined
    because they are all free.

This is not enforced by Linux, and is possible to read garbage if
not all the 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

Reported-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 fs/fat/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index c8366cb8eccd..0cec89271349 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -588,7 +588,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;
-- 
2.19.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vfat: don't read garbage after last dirent
  2018-12-07  1:34 [PATCH] vfat: don't read garbage after last dirent Matteo Croce
@ 2018-12-08 17:16 ` Matteo Croce
  2018-12-14  3:41 ` OGAWA Hirofumi
  1 sibling, 0 replies; 3+ messages in thread
From: Matteo Croce @ 2018-12-08 17:16 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: linux-kernel

On Fri, Dec 7, 2018 at 1:34 AM Matteo Croce <mcroce@redhat.com> wrote:
>     the rest of the entries in this directory do notneed to be examined

I made a small typo, it should be "do not need"

Regards,
-- 
Matteo Croce
per aspera ad upstream

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vfat: don't read garbage after last dirent
  2018-12-07  1:34 [PATCH] vfat: don't read garbage after last dirent Matteo Croce
  2018-12-08 17:16 ` Matteo Croce
@ 2018-12-14  3:41 ` OGAWA Hirofumi
  1 sibling, 0 replies; 3+ messages in thread
From: OGAWA Hirofumi @ 2018-12-14  3:41 UTC (permalink / raw)
  To: Matteo Croce; +Cc: Timothy Redaelli, linux-kernel

Matteo Croce <mcroce@redhat.com> writes:

> The FAT32 File System Specification[1] states:
>
>     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 notneed to be examined
>     because they are all free.
>
> This is not enforced by Linux, and is possible to read garbage if
> not all the 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
>
> Reported-by: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

I know this spec. But name[0] == 0 means - the rest of the entries in
this directory must be 0. Otherwise, overwriting zeroed entry by adding
new dir entry, then shows garbage as dir entry. So "stop at zero" is
just a optimization.

On other hand, I know there is buggy formatters don't clear that
garbage, and uses it on read-only storage area. 

So I will agree to supporting "stop at zero" though (and keep assuming
dir is initialized with zero clear. i.e. don't add tricky workaround to
support buggy formatters.), the implementation should handle it for
whole path, not only readdir().

E.g. I recall lookup, dir empty check path, and maybe there are others I
can't recall immediately.

>  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;

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07  1:34 [PATCH] vfat: don't read garbage after last dirent Matteo Croce
2018-12-08 17:16 ` Matteo Croce
2018-12-14  3:41 ` OGAWA Hirofumi

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox