At creation of a new file on FAT12 and FAT32 (and it is possible also FAT16) if the name of a file finished on one or several space or dots and contains before it some other symbols which number is multiple one LFN-record - the name is created incorrect: the counter of the first LFN-record < 0x40. How-To-Repeat: root@home > mount_msdosfs -l /dev/fd0 /mnt root@home > mkdir /mnt/test root@home > cd /mnt/test root@home > echo > 0123456789012\ root@home > echo > 0123456789012- root@home > echo > 01234567890123\ root@home > ls 0123456789012- 01234567890123 012345~1 root@home > hd -n 512 . 00000000 2e 20 20 20 20 20 20 20 20 20 20 10 00 7b 37 b0 |. ..{7╟| 00000010 92 36 92 36 00 00 37 b0 92 36 61 06 00 00 00 00 |66..7╟6a.....| 00000020 2e 2e 20 20 20 20 20 20 20 20 20 10 00 7b 37 b0 |.. ..{7╟| 00000030 92 36 92 36 00 00 37 b0 92 36 00 00 00 00 00 00 |66..7╟6......| 00000040 01 30 00 31 00 32 00 33 00 34 00 0f 00 dd 35 00 |.0.1.2.3.4...щ5.| 00000050 36 00 37 00 38 00 39 00 30 00 00 00 31 00 32 00 |6.7.8.9.0...1.2.| 00000060 30 31 32 33 34 35 7e 31 20 20 20 20 00 1f 43 b0 |012345~1 ..C╟| 00000070 92 36 92 36 00 00 43 b0 92 36 ad 00 01 00 00 00 |66..C╟6╜.....| 00000080 42 2d 00 00 00 ff ff ff ff ff ff 0f 00 fd ff ff |B-...ЪЪЪЪЪЪ..ЩЪЪ| 00000090 ff ff ff ff ff ff ff ff ff ff 00 00 ff ff ff ff |ЪЪЪЪЪЪЪЪЪЪ..ЪЪЪЪ| 000000a0 01 30 00 31 00 32 00 33 00 34 00 0f 00 fd 35 00 |.0.1.2.3.4...Щ5.| 000000b0 36 00 37 00 38 00 39 00 30 00 00 00 31 00 32 00 |6.7.8.9.0...1.2.| 000000c0 30 31 32 33 34 35 7e 32 20 20 20 20 00 3f 45 b0 |012345~2 .?E╟| 000000d0 92 36 92 36 00 00 45 b0 92 36 b7 00 01 00 00 00 |66..E╟6╥.....| 000000e0 42 33 00 00 00 ff ff ff ff ff ff 0f 00 1d ff ff |B3...ЪЪЪЪЪЪ...ЪЪ| 000000f0 ff ff ff ff ff ff ff ff ff ff 00 00 ff ff ff ff |ЪЪЪЪЪЪЪЪЪЪ..ЪЪЪЪ| 00000100 01 30 00 31 00 32 00 33 00 34 00 0f 00 1d 35 00 |.0.1.2.3.4....5.| 00000110 36 00 37 00 38 00 39 00 30 00 00 00 31 00 32 00 |6.7.8.9.0...1.2.| 00000120 30 31 32 33 34 35 7e 33 20 20 20 20 00 2b 49 b0 |012345~3 .+I╟| 00000130 92 36 92 36 00 00 49 b0 92 36 0e 01 01 00 00 00 |66..I╟6......| 00000140 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000200 Byte by offset 0x40 == 0x01, but not 0x41 !
Responsible Changed From-To: freebsd-bugs->freebsd-fs Over to maintainer(s).
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped
Still happens on CURRENT. Can be reproduced with any file where, for an integer N > 0, the filename begins with 13*N valid characters, the last of those not being a space or dot, and extra spaces and/or dots follow those characters. For example: $ touch "1234567890123 " $ ls 123456~1 Why? In sys/fs/msdosfs/msdosfs_lookup.c, ddep->de_fndcnt gets set in the msdosfs_lookup_() function, and it's supposed to count the number of LFN entries we will need to store the long filename. This is obtained from winSlotCnt() which trims trailing spaces and dots with winLenFixup(), and divides the resulting length by WIN_CHARS (13), which is the maximum number of chars in an LFN entry. createde() in sys/fs/msdosfs/msdosfs_lookup.c generates the long file name from this, by calling unix2winfn() ddep->de_fndcnt times, each one generating one LFN entry for that consecutive non-overlapping 13 character substring of the long filename. The bug is in this unix2winfn() function in sys/fs/msdosfs/msdosfs_conv.c. The function trims trailing spaces and/or dots, and then tries to write up to 13 of the remaining characters into the LFN entry. The last LFN entry must have WIN_LAST (0x40) OR-ed into its sequence number. If the filename ends before all 13 characters are populated, WIN_LAST is set. But if the filename is an exact multiple of 13 characters, the code relies on the original string to terminate in "\0" in order to set WIN_LAST: if (*un == '\0') end = WIN_LAST; The problem is that if the string ends in spaces and/or dots, it doesn't end in "\0" yet, so that test fails and WIN_LAST isn't set, producing a corrupt long filename. With the long filename corrupted, "ls" only shows the short name.
Created attachment 192919 [details] Fix WIN_LAST detection Here is a patch for this bug. Instead of detecting the LFN entry that should have WIN_LAST set via its filename segment ending in "\0", explicitly tell unix2winfn() which LFN entry is last from the function calling it, and set WIN_LAST for it. With this patch, files like "1234567890123 " get successfully created.
(In reply to Damjan Jovanovic from comment #4) Comparing the change with NetBSD, they also add another parameter to unix2winfn() but in this case it is tied to UTF: https://github.com/NetBSD/src/commit/de24c3c793dac1eece58245874fe2605d3bb6685#diff-97787ab37def0c52ffd1c0785bbd28b9 We like to keep changes with other BSDs as similar as possible but no idea if that is enough for this case: can you look at it? The NetBSD approach seems a bit cleaner but also touches makefs, which I prefer we avoid for now.
That wasn't the commit in which NetBSD first fixed this bug. Their code was correct from the start - the 1995 commit that added long filenames uses decrementing unlen to detect end of filename instead of equality with '\0': https://github.com/NetBSD/src/commit/e0c5bef42599b40c241b8a09e6ac9c56b6246b3a But yes, the new code in unix2winfn() in the commit you found would also fix this bug, though yet another approach, that detects end of the filename by comparing this LFN entry's offset into the full filename against its space/dot-trimmed length. What now?
(In reply to Damjan Jovanovic from comment #6) Interesting ... This code originated in NetBSD but we have diverged quite a bit over the years. For maintainance purposes we still would like to keep some similarities. If we have to diverge further, it is OK, but please try to reduce somewhat the differences: rename islastentry to utf8, adopt enough of their patches so that it fixes the issue. Thanks for working on this!
This bug is a regression that happened in https://github.com/freebsd/freebsd/commit/571ef024e3f3a472116a55a8489d77eb4f5f933e where we went from checking unlen to checking for '\0'. NetBSD's patch is mostly for UTF-8 support, and I think is best handled in a separate issue. For this bug, we should fix our code to use the old method of decrementing and checking unlen, like we did and NetBSD did prior to their UTF-8 patch, which will also make applying NetBSD's patch on top of it easier later on.
(In reply to Damjan Jovanovic from comment #8) Well the commit introduced libkiconv and reports 75 changed files, and while some of them (ntfs) don't exist anymore it is not a realistic revert. It seems better to keep consistency by keeping libkiconv as in other filesystem code so if comes to that, I prefer your existing patch.
Created attachment 193037 [details] Fix
Created attachment 193038 [details] Fix WIN_LAST detection using unlen == 0 Here is a much simpler patch. Just check unlen has been decremented to 0, instead of requiring the caller to tell us this is the last LFN entry. This is what was being done before our commit https://github.com/freebsd/freebsd/commit/571ef024e3f3a472116a55a8489d77eb4f5f933e and it still works.
A commit references this bug: Author: pfg Date: Fri May 4 03:44:13 UTC 2018 New revision: 333239 URL: https://svnweb.freebsd.org/changeset/base/333239 Log: msdosfs: long names of files are created incorrectly. This fixes a regression that happened in r120492 (2003) where libkiconv was introduced and we went from checking unlen to checking for '\0'. PR: 111843 Patch by: Damjan Jovanovic MFC after: 1 week Changes: head/sys/fs/msdosfs/msdosfs_conv.c
A commit references this bug: Author: pfg Date: Mon May 14 19:20:38 UTC 2018 New revision: 333610 URL: https://svnweb.freebsd.org/changeset/base/333610 Log: MFC r333239: msdosfs: long names of files are created incorrectly. This fixes a regression that happened in r120492 (2003) where libkiconv was introduced and we went from checking unlen to checking for '\0'. PR: 111843 Patch by: Damjan Jovanovic Approved by: re (gjb) Changes: _U stable/11/ stable/11/sys/fs/msdosfs/msdosfs_conv.c
A commit references this bug: Author: pfg Date: Mon May 14 19:21:57 UTC 2018 New revision: 333611 URL: https://svnweb.freebsd.org/changeset/base/333611 Log: MFC r333239: msdosfs: long names of files are created incorrectly. This fixes a regression that happened in r120492 (2003) where libkiconv was introduced and we went from checking unlen to checking for '\0'. PR: 111843 Patch by: Damjan Jovanovic Changes: _U stable/10/ stable/10/sys/fs/msdosfs/msdosfs_conv.c
Thanks! Committed in all supported versions. The bug is old enough we should apply it to older versions but I don't keep those trees around and can't really test.