Re: [PATCH 2/3] NFS/localio: nfs_uuid_put() fix the wait for file unlink events

[Trond Myklebust <trondmy@kernel.org>]

On Wed, 2025-07-16 at 11:22 +1000, NeilBrown wrote:
> On Wed, 16 Jul 2025, Trond Myklebust wrote:
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > No reference to nfl is held when waiting in nfs_uuid_put(), so not
> > only
> > must the event condition check if the first entry in the list has
> > changed, it must also check if the nfl->nfs_uuid field is still
> > NULL,
> > in case the old entry was replaced.
> 
> As no reference is held to nfl, it cannot be safe to check if
> nfl->nfs_uuid is still NULL.  It could be freed and the memory reused
> for anything.

It is quite safe.

The test first checks if the nfs_uuid->files list first entry still
points to 'nfl'. Then it checks the value of nfl->nfs_uuid.

All this happens while the nfs_uuid->lock is held.

> 
> At this point, with nfs_uuid->net set to NULL, nothing can be added
> to
> nfs_uuid->files.  Things can only be removed.

There is nothing in either nfs_open_local_fh() or nfs_uuid_add_file()
that stops anyone from adding a new entry to nfs_uuid->files in the
case where nfs_uuid->net is NULL.

If that was the intention, then I agree that this patch is wrong, but
not for the reasons you listed.


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com