Bug 199152 - msdosfs writes on umount to readonly mounted fs
Summary: msdosfs writes on umount to readonly mounted fs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-04-03 21:49 UTC by longwitz
Modified: 2018-02-12 09:53 UTC (History)
2 users (show)

See Also:


Attachments
Patch for msdosfs (2.26 KB, patch)
2015-04-03 21:49 UTC, longwitz
no flags Details | Diff
Let kernel compile with MSDOSFS_DEBUG (2.12 KB, patch)
2017-10-18 14:32 UTC, longwitz
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description longwitz 2015-04-03 21:49:25 UTC
Created attachment 155169 [details]
Patch for msdosfs

In FreeBSD 10.1-STABLE #10 r279940 I see

mount -r /mydosfs
umount /mydosfs
umount: unmount of /mydosfs failed: Operation not permitted
On console: g_vfs_done():ada0p1[WRITE(offset=512, length=512)]error = 1

The next umount works.

This problem was introduced by rev 275638 to head, because in the case of a readonly mounted msdosfs a call of msdosfs_sync() was introduced which failes to write in msdos_fsiflush(). This happens because the flag MSDOSFS_FSIMOD is set on the first call of msdosfs_sync().

The attached patch solves the problem for me and includes also some minor changes necessary to build the kernel with MSDOS_DEBUG.
Comment 1 commit-hook freebsd_committer freebsd_triage 2015-04-05 21:08:18 UTC
A commit references this bug:

Author: kib
Date: Sun Apr  5 21:08:05 UTC 2015
New revision: 281120
URL: https://svnweb.freebsd.org/changeset/base/281120

Log:
  Assert that an msdosfs mount is not read-only when FAT modifications
  are requested.

  PR:	199152
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Changes:
  head/sys/fs/msdosfs/msdosfs_fat.c
Comment 2 commit-hook freebsd_committer freebsd_triage 2015-04-05 21:11:20 UTC
A commit references this bug:

Author: kib
Date: Sun Apr  5 21:10:39 UTC 2015
New revision: 281121
URL: https://svnweb.freebsd.org/changeset/base/281121

Log:
  Do not call msdosfs_sync() on the read-only msdosfs mounts.  In fact,
  it should be a nop for ro.

  PR:	199152
  Reviewed by:	bde (PR version of the patch)
  Submitted by:	longwitz@incore.de
  MFC after:	1 week

Changes:
  head/sys/fs/msdosfs/msdosfs_vfsops.c
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2015-04-05 21:14:19 UTC
(In reply to longwitz from comment #0)
The msdosfs_unmount() chunk is committed, with minor editing around.

I also committed the assertions to verify your claim that MSDOSFS_FSIMOD flag was set.  Please re-test with only r281120 applied.  I cannot reproduce the issue locally, using ro file-backed md volume.  I want to see the backtrace for the moment where FSIMOD flag is set.
Comment 4 longwitz 2015-04-07 22:17:54 UTC
Your assertions from r281120 did not trigger. I found the flag FSIMOD is set in usemap_free() called from fillinusemap(), which is called from mountmsdosfs() but before the MSDOSFSMNT_RONLY flag is set. Thats the reason why your assertion is missed. At the begin of the "for (cn = CLUST_FIRST .." loop in fillinusemap() I see pm_flags=0x20000000 and pm_maxcluster=98305. After the loop I have pm_flags=0x21000000 and usemap_free() was called 79356 times, enough to set the FSIMOD bit.
The first install of my test maschine was Windows 7 mit UEFI, I boot FreeBSD 10 Stable with PXE, because my motherboard (Gigabyte hGA-A75M-UD2H) has the problem described in PR/193646 and FreeBSD hangs on EFI boot. My test msdosfs partition is the EFI partition installed by Windows 7 and is 100 MB in size. For clarification the output of "gpart show":

=>       34  234441581  ada0  GPT  (112G)
         34       2014        - free -  (1.0M)
       2048     204800     1  efi  (100M)
     206848     262144     2  ms-reserved  (128M)
     468992  182771712     3  ms-basic-data  (87G)
  183240704       1024     4  freebsd-boot  (512K)
  183241728    2097152     5  freebsd-ufs  (1.0G)
  185338880    8388608     6  freebsd-swap  (4.0G)
  193727488    8388608     7  freebsd-ufs  (4.0G)
  202116096   16777216     8  freebsd-ufs  (8.0G)
  218893312   15548302     9  freebsd-ufs  (7.4G)
  234441614          1        - free -  (512B)

If you need more information let me know.
Comment 5 commit-hook freebsd_committer freebsd_triage 2015-04-12 07:05:31 UTC
A commit references this bug:

Author: kib
Date: Sun Apr 12 07:05:21 UTC 2015
New revision: 281457
URL: https://svnweb.freebsd.org/changeset/base/281457

Log:
  MFC r281121:
  Do not call msdosfs_sync() on the read-only msdosfs mounts.

  PR:    199152

Changes:
_U  stable/10/
  stable/10/sys/fs/msdosfs/msdosfs_vfsops.c
Comment 6 Ed Maste freebsd_committer freebsd_triage 2017-10-13 17:47:24 UTC
Is this now resolved?
Comment 7 longwitz 2017-10-18 14:32:44 UTC
Created attachment 187282 [details]
Let kernel compile with MSDOSFS_DEBUG
Comment 8 longwitz 2017-10-18 14:39:55 UTC
Seen in FreeBSD 10.3-STABLE r317782: 

1. The problem with the first failing umount on a readonly mounted partition is solved by commit 281457.

2. There is probably in issue with mount -r /mydosfs, it is a little bit suprising that the bit FSIMOD is set on a readonly mount. That is the reason for the failing first umount in absence of the patch mentioned above. The backtrace for this situation is included in the output of a DTrace script:

  1   4751   msdosfs_mount:entry       called with mnt_flag=0x1
  1  18090   mountmsdosfs:entry        called with mnt_flag=0x1
  1  15625   fillinusemap:entry        called with pm_flags=0x20000000, pm_maxcluster=98305, pm_inusemap=fffffe0003e5b000
  1  18456   usemap_free:entry         called with pm_flags=0x20000000, pm_maxcluster=98305 and cn=67

              kernel`fillinusemap+0x16d
              kernel`mountmsdosfs+0x9be
              kernel`msdosfs_mount+0x66a
              kernel`vfs_donmount+0xf50
              kernel`sys_nmount+0x71
              kernel`amd64_syscall+0x452
              kernel`0xffffffff807f364b

  1  18457   usemap_free:return       returned with pm_flags=0x21000000
  1  15626   fillinusemap:return      returncode=0, pm_flags=0x21000000, pm_maxcluster=98305, pm_inusemap=fffffe0003e5b000
  1  18091   mountmsdosfs:return      returncode=0, mnt_flag=0x1001, pm_flags=0xa1000000, pm_inusemap=fffffe0003e5b000
  1   4752   msdosfs_mount:return     returncode=0

FSIMOD (=0x1000000) is set on call number 67 in usemap_free().

3. To compile the kernel with MSDOSFS_DEBUG some patches are nesessary.
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-10-19 12:55:45 UTC
A commit references this bug:

Author: emaste
Date: Thu Oct 19 12:55:11 UTC 2017
New revision: 324753
URL: https://svnweb.freebsd.org/changeset/base/324753

Log:
  msdosfs: fix build with MSDOSFS_DEBUG

  Inspired by a patch submission by longwitz@incore.de with many changes
  for ino64 in HEAD.

  PR:		199152
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/sys/fs/msdosfs/msdosfs_conv.c
  head/sys/fs/msdosfs/msdosfs_denode.c
  head/sys/fs/msdosfs/msdosfs_vfsops.c
  head/sys/fs/msdosfs/msdosfs_vnops.c