Re: NFSD automatically releases all states when underlying file system is unmounted

[Dai Ngo <dai.ngo@oracle.com>]

On 3/25/25 8: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.

Is it useful to add an option, 'R', to exportfs to also revoke state when
user doing '-au':

# exportfs -Rau

The main purpose of this option is to allow all the underlying file systems
to be unmounted without requiring all clients to unmount the NFS exports first.

Thanks,
-Dai

>
> 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
>>>>