On 3/25/25 11:41 PM, NeilBrown wrote: > 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. I'm not religiously attached to expunging open/lock state on a simple unexport operation. But I think it is critical to document the fact that NFSv4 state remains and that will prevent an unmount (I'm not sure we've identified any possible security exposures). Neil, do you happen to know if unlock_filesystem and unlock_ip are mentioned in man pages? If so, then exportfs(8) should refer to them. > 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 >>>> >> > -- Chuck Lever
