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

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

[Tuomas Tynkkynen]

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?

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

[Tuomas Tynkkynen]

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.

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

[Tom Rini]

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

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170923/cb7b107b/attachment.sig>

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

[Rob Clark]

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