[U-Boot] FAT filesystems and mtools-created filesystems

Hi,
FAT file systems created by GNU mtools have a problem that mtools doesn't initialize the first cluster field of the '.' and '..' directory entries. That is, with the following script:
mkdir fattmp cd fattmp mkdir -p foo/bar/baz touch foo/bar/baz/biff truncate -s 16M ../fattest.img mkfs.vfat ../fattest.img mcopy -bpsvm -i ../fattest.img ./* ::
... `fsck.vfat ../fattest.img` outputs:
/FOO/BAR/. Start (0) does not point to parent (3) /FOO/BAR/.. Start (0) does not point to .. (4) /FOO/BAR/BAZ/. Start (0) does not point to parent (2) /FOO/BAR/BAZ/.. Start (0) does not point to .. (3)
Now that's of course a bug in mtools, but the tricky thing is that Linux is fine with that (and probably Windows as well, or they would have drowned in complaints), presumably due to both OSes resolving '.' and ''..' in their VFS layers.
I'm not sure if this problem has always been there but I've started to see "Invalid FAT entry" prints lately, presumably since the "fat/fs: convert to directory iterators" change. In my case it accidentally works anyway, since I have an entry like 'LINUX ../foo/bar' in extlinux/extlinux.conf and an invalid FAT entry somehow makes it back to the root directory.
So should we
1) Ignore the problem and call mtools broken 2) Hack around this in the FAT driver 3) Special-case '.' and '..' in the common directory traversal code?

On 09/23/2017 01:26 PM, Tuomas Tynkkynen wrote:
Hi,
FAT file systems created by GNU mtools have a problem that mtools doesn't initialize the first cluster field of the '.' and '..' directory entries. That is, with the following script:
Oh, and unrelated to the mtools problem, '..' entries leading to the root directory need fixing in the current code, as itr->is_root needs to be set.

On Sat, Sep 23, 2017 at 6:51 AM, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
On 09/23/2017 01:26 PM, Tuomas Tynkkynen wrote:
Hi,
FAT file systems created by GNU mtools have a problem that mtools doesn't initialize the first cluster field of the '.' and '..' directory entries. That is, with the following script:
Sorry for the delay, I've been travelling.
I'll have a look at this.. leaning towards moving sanitize_path() from efi_file.c into fs/fat, since we don't have an infinite stack to store parent ".." entries.
Definitely would be good if you could send a patch to add this to test/fs/fs-test.sh
Oh, and unrelated to the mtools problem, '..' entries leading to the root directory need fixing in the current code, as itr->is_root needs to be set.
Hmm, I thought I fixed this once, but I guess only for fat32..
BR, -R

On Sat, Sep 23, 2017 at 01:26:38PM +0300, Tuomas Tynkkynen wrote:
Hi,
FAT file systems created by GNU mtools have a problem that mtools doesn't initialize the first cluster field of the '.' and '..' directory entries. That is, with the following script:
mkdir fattmp cd fattmp mkdir -p foo/bar/baz touch foo/bar/baz/biff truncate -s 16M ../fattest.img mkfs.vfat ../fattest.img mcopy -bpsvm -i ../fattest.img ./* ::
... `fsck.vfat ../fattest.img` outputs:
/FOO/BAR/. Start (0) does not point to parent (3) /FOO/BAR/.. Start (0) does not point to .. (4) /FOO/BAR/BAZ/. Start (0) does not point to parent (2) /FOO/BAR/BAZ/.. Start (0) does not point to .. (3)
Now that's of course a bug in mtools, but the tricky thing is that Linux is fine with that (and probably Windows as well, or they would have drowned in complaints), presumably due to both OSes resolving '.' and ''..' in their VFS layers.
I'm not sure if this problem has always been there but I've started to see "Invalid FAT entry" prints lately, presumably since the "fat/fs: convert to directory iterators" change. In my case it accidentally works anyway, since I have an entry like 'LINUX ../foo/bar' in extlinux/extlinux.conf and an invalid FAT entry somehow makes it back to the root directory.
So should we
- Ignore the problem and call mtools broken
- Hack around this in the FAT driver
- Special-case '.' and '..' in the common directory traversal code?
Reality triumphs over specification. Please submit a patch to fix the new problem. Bonus points if you can add an fs test for this case to test/fs/, thanks!
participants (3)
-
Rob Clark
-
Tom Rini
-
Tuomas Tynkkynen