On Wed, 26 Mar 2025, Dai Ngo wrote: > On 3/25/25 5:23 PM, NeilBrown wrote: > > On Sat, 22 Mar 2025, Chuck Lever wrote: > >> On 3/21/25 10:36 AM, Benjamin Coddington wrote: > >>> On 20 Mar 2025, at 13:53, Chuck Lever wrote: > >>> > >>>> On 3/19/25 5:46 PM, NeilBrown wrote: > >>>>> On Thu, 20 Mar 2025, Dai Ngo wrote: > >>>>>> Hi, > >>>>>> > >>>>>> Currently when the local file system needs to be unmounted for maintenance > >>>>>> the admin needs to make sure all the NFS clients have stopped using any files > >>>>>> on the NFS shares before the umount(8) can succeed. > >>>>> This is easily achieved with > >>>>> echo /path/to/filesystem > /proc/fs/nfsd/unlock_filesystem > >>>>> > >>>>> Do this after unexporting and before unmounting. > >>>> Seems like administrators would expect that a filesystem can be > >>>> unmounted immediately after unexporting it. Should "exportfs" be changed > >>>> to handle this extra step under the covers? Doesn't seem like it would > >>>> be hard to do, and I can't think of a use case where it would be > >>>> harmful. > >>> No. I think that admins don't expect to lose all their NFS client's state if > >>> they're managing the exports. That would be a really big and invisible change > >>> to existing behavior. > >> To be clear, I mean that a file system should be unlocked only when it > >> is specifically unexported. IMO, unexport is usually an administrator > >> action that means "I want to stop remote access to this file system now" > >> and that's what unlock_filesystem does. > > A problem with that position is that "unexport" isn't a well defined > > operation. > > It is quite possible to edit /etc/exports then run "exportfs -r". This > > may implicit unexport things. > > > > The kernel certainly doesn't have a concept of "unexport". You can run > > "exportfs -f" at any time quite safely. That tells the kernel to forget > > all export information, but allows the kernel to ask mountd for anything > > it find that it needs. > > > >> IMO administrators would be surprised to learn that NFS clients may > >> continue to access a file system (via existing open files) after it > >> has been explicitly unexported. > > They can't access those file while it remains unexported. But if it is > > re-exported, the access they had can continue seamlessly. > > > > The origin model is NLM which is separate from NFS. Unexporting to NFS > > doesn't close the locks held by NLM. That can be done separately by the > > client with a STATMON request. In fact NLM never drops locks unless > > explicitly asked to by the client or forced by the server admin. So it > > isn't a good model, but it is what we had. > > > >> The alternative is to document unlock_filesystem in man exportfs(8). > > Another alternative is to provide new functionality in exportfs. Maybe > > a --force flag or a --close-all flag. > > It could examine /proc/fs/nfsd/clients/*/states to determine which > > filesystems had active state, then examine the export tables > > (/var/lib/nfs/etab) to see what was currently exported, then write > > something appropriate to unlock_filesystem for any active filesystems > > which are no longer exported. > > Is it possible that at the time of cache_clean/svc_export_put the kernel > makes an upcall to rpc.mountd to check if svc_export.ex_path is still > exported?. If it's not then release all the states that use that super_block. I suspect that could be done, but then you would hit Ben's concern. Temporarily unexported a filesystem would change from the client getting ESTALE if it happens to access a file while the filesystem is not exported, to the client definitely getting ADMIN_REVOKED (probably -EIO) then next time it accesses a file even if the filesystem has been exported again. I agree with Ben that there needs to be a deliberate admin action to revoke state, not just a side-effect of unexport which historically has not revoked state. NeilBrown > > -Dai > > > > > If we did that we would want to find NLM locks in /proc/locks too and > > ensure those were discarded if necessary. > > > > There is also the possibility that a filesystem is still exported to > > some clients but not to all. In that case writing something to > > unlock_ip might be appropriate - though that doesn't revoke v4 state > > yet. > > > > Thanks, > > NeilBrown > > > > > >> And perhaps we need a more surgical mechanism that can handle the case > >> where the file system is still exported but the security policy has > >> changed. Because this does feel like a real information leak. > >> > >> > >> -- > >> Chuck Lever > >> >
