Re: [PATCH v3 1/1] NFSD: Track SCSI Persistent Registration Fencing per Client with xarray

[Chuck Lever <cel@kernel.org>]

On 12/24/25 12:52 PM, Dai Ngo wrote:
> 
> On 12/24/25 6:57 AM, Chuck Lever wrote:
>>
>> On Tue, Dec 23, 2025, at 5:34 PM, Dai Ngo wrote:
>>> On 12/23/25 11:47 AM, Chuck Lever wrote:
>>>> On Tue, Dec 23, 2025, at 1:54 PM, Dai Ngo wrote:

>>>>>> Another question is: Can cl_fenced_devs grow without bounds?
>>>>> I think it has the same limitation for any xarray. The hard limit
>>>>> is the availability of memory in the system.
>>>> My question isn't about how much can any xarray hold, it's how
>>>> much will NFSD ask /cl_fenced_devs/ to hold. IIUC, the upper
>>>> bound for each nfs4_client's cl_fenced_devs will be the number
>>>> of exported block devices, and no more than that.
>>>>
>>>> I want to avoid a potential denial of service vector -- NFSD
>>>> should not be able to create an unlimited number of items
>>>> in cl_fenced_devs... but sounds like there is a natural limit.
>>> Oh I see. I did not even think about this DOS since I think this
>>> is under the control of the admin on NFSD and a sane admin would
>>> not configure a massive amount of exported block devices.
>> Ultimately, the upper bound on the number entries in cl_fenced_devs
>> is indeed under the control of the NFS server administrator,
>> indirectly. But looking only at the code in the patch:
>>
>>   - New entries are created in cl_fenced_devs via GETDEVICEINFO,
>>     a client (remote host) action
>>   - There's nothing that removes these entries over time
> 
> I don't understand why these entries need to be removed over time.

They don't need to be removed over time. I'm saying you should document
the assumptions better -- that NFSD indirectly enforces a limit on the
number of items in this xarray. That isn't clear if a reviewer is
looking only at the code in nfsd4_block_get_device_info_scsi.


> These entries are created only when the clients accessing NFS exports
> that use pNFS SCSI layout. So long as the clients still mount these
> exports, don't we need to keep these entries around?
> 
>>
>> The duplicate checking logic needs to ensure that client actions
>> cannot create more entries than that upper bound.
> 
> How can this happens? repeated GETDEVICEINFO ops of the same device
> still use the same entry so how do the clients can indirectly create
> more entries than the number of pNFS exports?

It can happen if a race occurs, for example. A broken or malicious
client could send two concurrent GETDEVICEINFO operations for the same
device. If the new code isn't careful, it could unintentionally add two
copies of the device to that array due to that race.

What I'm saying is that this needs to be documented: why does the
addition of the device to cl_fenced_devs need to be atomic? Because
if it isn't, a race could allow an errant client to add extra devices
to cl_fenced_devs over time. That could cause cl_fenced_devs to grow
without bound.


-- 
Chuck Lever