Bug 264166 - mount_msdosfs: Fails to mount glabel label: mount_msdosfs: /dev/label/MACOME_CONFIG: Invalid argument
Summary: mount_msdosfs: Fails to mount glabel label: mount_msdosfs: /dev/label/MACOME_...
Status: Closed Works As Intended
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-fs (Nobody)
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks: 264030
  Show dependency treegraph
 
Reported: 2022-05-23 01:36 UTC by t_uemura
Modified: 2023-10-04 13:44 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback? (kib)
koobs: mfc-stable13?
koobs: mfc-stable12-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description t_uemura 2022-05-23 01:36:38 UTC
There is a regression caused by the commit c5c1537dc7ffd27066d035b8e236d3dcb548a3c8; msdosfs: sanity check sector count from BPB, by Mr. Konstantin Belousov, which made mount_msdosfs to not accept glabel label, even though the bare device node (such as da0s1) of the same disk can be mounted and accessed correctly.

With the mentioned commit applied, as shown below, label couldn't be mounted though the corresponding raw device da0s1 was handled as expected.

root@da320:~# glabel status | grep label/
                       label/MACOME_CONFIG     N/A  da0s1
root@da320:~# ls -la /dev/label/MACOME_CONFIG
crw-r-----  1 root  operator  0x73 May 22 20:54 /dev/label/MACOME_CONFIG
root@da320:~# gpart show da0
=>     63  3919809  da0  MBR  (1.9G)
       63  3919809    1  fat32lba  (1.9G)
root@da320:~# mount -rt msdosfs /dev/da0s1 /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt
root@da320:~# mount -rt msdosfs /dev/label/MACOME_CONFIG /mnt
mount_msdosfs: /dev/label/MACOME_CONFIG: Invalid argument
root@da320:~# echo $?
1

Backed out the commit, and the issue was fixed.

root@da320:~# mount -rt msdosfs /dev/da0s1 /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt
root@da320:~# mount -rt msdosfs /dev/label/MACOME_CONFIG /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt

With another USB stick (with the same label), the same problem happened if the commit was applied.

root@da320:~# glabel status | grep label/
                       label/MACOME_CONFIG     N/A  da0
root@da320:~# ls -la /dev/label/MACOME_CONFIG
crw-r-----  1 root  operator  0x7b May 23 09:56 /dev/label/MACOME_CONFIG
root@da320:~# gpart show da0
gpart: No such geom: da0.
root@da320:~# mount -rt msdosfs /dev/da0 /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt
root@da320:~# mount -rt msdosfs /dev/label/MACOME_CONFIG /mnt
mount_msdosfs: /dev/label/MACOME_CONFIG: Invalid argument
root@da320:~# echo $?
1

And fixed when backed out.

root@da320:~# mount -rt msdosfs /dev/da0s1 /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt
root@da320:~# mount -rt msdosfs /dev/label/MACOME_CONFIG /mnt
root@da320:~# echo $?
0
root@da320:~# umount /mnt
Comment 1 t_uemura 2022-05-23 01:39:55 UTC
(In reply to t_uemura from comment #0)

> And fixed when backed out.

> root@da320:~# mount -rt msdosfs /dev/da0s1 /mnt
> root@da320:~# echo $?
> 0

Update to myself. This is the correct one.

And fixed when backed out.

root@da320:~# mount -rt msdosfs /dev/da0 /mnt
root@da320:~# echo $?
0
Comment 2 t_uemura 2022-05-24 00:45:32 UTC
(In reply to t_uemura from comment #1)

Specifically, the following condition is the culprit.

(off_t)pmp->pm_HugeSectors * pmp->pm_BytesPerSec >
cp->provider->mediasize /* past end of vol */
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-24 03:02:39 UTC
^Triage: Request feedback from base c5c1537dc7ff
Comment 4 Mark Johnston freebsd_committer freebsd_triage 2022-05-24 16:10:24 UTC
(In reply to t_uemura from comment #2)
How did you create the filesystem and the label?  It looks like you added the label to the partition after creating the filesystem.  But the label occupies some space on the partition, and I suspect that both glabel and the filesystem "own" that space.
Comment 5 t_uemura 2022-05-24 22:26:05 UTC
(In reply to Mark Johnston from comment #4)

> It looks like you added the label to the partition after creating the filesystem.

Correct. I'd put a label on an out-of-the-box USB stick.

So, will the problem be resolved by re-formatting the disk via the label by newfs_msdos? Or maybe reformatting with Windows won't fix the problem, as Windows will ignore the label? I'll try later today.
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2022-05-24 22:30:50 UTC
(In reply to t_uemura from comment #5)
Formatting with Windows probably won't work.  But you could perhaps use the embedded volume name instead?  The FreeBSD kernel will find it automatically and create a device under /dev/msdosfs/.  Though, the length of FAT volume names is quite limited.
Comment 7 t_uemura 2022-05-25 00:41:17 UTC
(In reply to Mark Johnston from comment #6)

Re-formatting with newfs_msdos does resolve the problem. This really is not a regression but my improper usage, but I appreciate if I could have a notice in glabel(8) or somewhere.

Should I close this issue?

> Formatting with Windows probably won't work.

Formatting with Windows doesn't work.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-25 23:49:51 UTC
^Triage: Docs improvement or is there something we can/should do to improve mount_msdosfs label handling?
Comment 9 t_uemura 2022-05-26 02:42:45 UTC
(In reply to Kubilay Kocak from comment #8)

Appreciated if glabel warns users when they try to label on existing filesystems, that they need to re-format the filesystem via the label or inconsistent state may arise.
Comment 10 Mark Johnston freebsd_committer freebsd_triage 2022-05-26 13:57:26 UTC
(In reply to t_uemura from comment #7)
Here is my attempt at improving the documentation: https://reviews.freebsd.org/D35326
Please let me know if it is clear or not.

(In reply to t_uemura from comment #9)
This is a good idea.  We have fstyp(8), which can "taste" devices to see if they contain a known filesystem.  However, that functionality would have to be available in a library in order for glabel(8) to use it, and that would require some effort...
Comment 11 t_uemura 2022-05-27 00:15:53 UTC
(In reply to Mark Johnston from comment #10)

> Please let me know if it is clear or not.

Personally, the second paragraph (Note also that generic, automatic labels occupy ...) is more important than the first paragraph, so I think these two shall be swapped.

> This is a good idea.  We have fstyp(8), which can "taste" devices to see if they

That looks promising, must be the best solution.
Comment 12 commit-hook freebsd_committer freebsd_triage 2023-09-27 12:41:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=81f36fbc98dd74ca923938e0329919d426811b0c

commit 81f36fbc98dd74ca923938e0329919d426811b0c
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-05-26 13:48:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-09-27 12:39:31 +0000

    glabel.8: Warn against using generic labels on a shared device

    Also suggest against creating a generic label on a device which already
    contains a filesystem.

    PR:             264166
    Reviewed by:    imp, delphij, Pau Amma <pauamma@gundo.com>
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D35326

 lib/geom/label/glabel.8 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
Comment 13 commit-hook freebsd_committer freebsd_triage 2023-10-04 13:44:12 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2464d8c5e296a02de882310398e73077225cac00

commit 2464d8c5e296a02de882310398e73077225cac00
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-05-26 13:48:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-10-04 13:43:22 +0000

    glabel.8: Warn against using generic labels on a shared device

    Also suggest against creating a generic label on a device which already
    contains a filesystem.

    PR:             264166
    Reviewed by:    imp, delphij, Pau Amma <pauamma@gundo.com>
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D35326

    (cherry picked from commit 81f36fbc98dd74ca923938e0329919d426811b0c)

 lib/geom/label/glabel.8 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
Comment 14 commit-hook freebsd_committer freebsd_triage 2023-10-04 13:44:17 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b11568083f09f48caf11d78d25ab58fc65793bcf

commit b11568083f09f48caf11d78d25ab58fc65793bcf
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-05-26 13:48:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-10-04 13:41:29 +0000

    glabel.8: Warn against using generic labels on a shared device

    Also suggest against creating a generic label on a device which already
    contains a filesystem.

    PR:             264166
    Reviewed by:    imp, delphij, Pau Amma <pauamma@gundo.com>
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D35326

    (cherry picked from commit 81f36fbc98dd74ca923938e0329919d426811b0c)

 lib/geom/label/glabel.8 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)