Re: [PATCH 0/6] SUNRPC: Address remaining cache_check_rcu() UAF in cache content files

[yangerkun <yangerkun@huawei.com>]

在 2026/5/8 11:08, yangerkun 写道:
> 
> 
> 在 2026/5/8 10:45, yangerkun 写道:
>> Hello  Chuck,
>>
>> 在 2026/5/8 0:12, Chuck Lever 写道:
>>> Hello Erkun -
>>>
>>> On Thu, May 7, 2026, at 11:09 AM, yangerkun wrote:
>>>> Hi,
>>>>
>>>> 在 2026/5/1 22:51, Chuck Lever 写道:
>>>>> Misbah Anjum reported a use-after-free in cache_check_rcu()
>>>>> reached through e_show() while sosreport was reading
>>>>> /proc/fs/nfsd/exports on ppc64le.  Two fixes for that report
>>>>> landed in v7.0:
>>>>>
>>>>>     48db892356d6 ("NFSD: Defer sub-object cleanup in export put 
>>>>> callbacks")
>>>>>     e7fcf179b82d ("NFSD: Hold net reference for the lifetime of / 
>>>>> proc/fs/nfs/exports fd")
>>>>
>>>> Back to the problem fixed by this patches, I'm a little confused why
>>>> this UAF can be trigged.
>>>>
>>>> Before this patches, svc_export_put show as follow:
>>>>
>>>>    368 static void svc_export_put(struct kref *ref)
>>>>    369 {
>>>>    370         struct svc_export *exp = container_of(ref, struct
>>>> svc_export, h.ref);
>>>>    371
>>>>    372         path_put(&exp->ex_path);
>>>>    373         auth_domain_put(exp->ex_client);
>>>>    374         call_rcu(&exp->ex_rcu, svc_export_release);
>>>>    375 }
>>>>
>>>> The auth_domain_put function releases ->name using call_rcu, and
>>>> path_put may release the dentry also via call_rcu. All of this seems to
>>>> prevent e_show from causing a UAF. Could you point out which line in
>>>> d_path triggers the issue?
>>>
>>> The dentry, the mount, and the auth_domain ->name buffer all
>>> end up RCU-freed (dentry_free() and delayed_free_vfsmnt in
>>> fs/, svcauth_unix_domain_release_rcu() in svcauth_unix.c).
>>> The eventual kfree isn't the problem.
>>>
>>> The problem is the synchronous teardown inside path_put(),
>>> which runs before svc_export_put() ever reaches its own
>>> call_rcu():
>>>
>>>    path_put(&exp->ex_path)
>>>      -> dput(dentry)
>>>         -> __dentry_kill()              [if last ref]
>>>            -> __d_drop()                /* unhashes */
>>>            -> dentry_unlink_inode()     /* d_inode = NULL */
>>>            -> d_op->d_release() if set
>>>            -> drops parent d_lockref    /* may cascade up */
>>>            -> dentry_free()             /* call_rcu deferred */
>>>      -> mntput(mnt)                     /* deferred via task_work */
>>>
>>> The dentry pointer itself is RCU-safe, so prepend_path()'s walk
>>> of d_parent and d_name doesn't read freed memory.  But by the
>>> time the reader gets there, __d_clear_type_and_inode() has
>>> already stored NULL into d_inode, __d_drop() has broken the
>>> hash linkage, and the parent's d_lockref has been decremented
>>> -- which can in turn fire __dentry_kill() on the parent, and
>>> on up the tree.  An e_show() that's still inside its cache RCU
>>> read section walks into that half-dismantled state through
>>> seq_path(), and that's the NULL deref Misbah reported.
>>
>> Thank you for your detailed explanation! Yes, e_show might be called 
>> when the state is partially dismantled, but after carefully reviewing 
>> the code with dput up to __dentry_kill, I still cannot find anything 
>> that could cause this issue. Additionally, the comments for 
>> prepend_path indicate that they have already taken into account that 
>> the dentry can be removed concurrently. I have also run some tests on 
>> my arm64 QEMU, but I couldn't reproduce the problem either. Could you 
>> please help me identify the specific line or pointer in the dentry 
>> that triggers this use-after-free or null pointer issue?
>>
>> Maybe I am not be very familiar with the code, which caused me to fail 
>> to identify the real root cause. I'm so sorry for that.
>>
>>
>> 265 char *d_path(const struct path *path, char *buf, int buflen)
>> 266 {
>> 267         DECLARE_BUFFER(b, buf, buflen);
>> 268         struct path root;
>> 269
>> 270         /*
>> 271          * We have various synthetic filesystems that never get 
>> mounted.  On
>> 272          * these filesystems dentries are never used for lookup 
>> purposes, and
>> 273          * thus don't need to be hashed.  They also don't need a 
>> name until a
>> 274          * user wants to identify the object in /proc/pid/fd/.  
>> The little hack
>> 275          * below allows us to generate a name for these objects on 
>> demand:
>> 276          *
>> 277          * Some pseudo inodes are mountable.  When they are mounted
>> 278          * path->dentry == path->mnt->mnt_root.  In that case 
>> don't call d_dname
>> 279          * and instead have d_path return the mounted path.
>> 280          */
>> 281         if (path->dentry->d_op && path->dentry->d_op->d_dname &&
>> 282             (!IS_ROOT(path->dentry) || path->dentry != path->mnt- 
>>  >mnt_root))
>> 283                 return path->dentry->d_op->d_dname(path->dentry, 
>> buf, buflen);
>> 284
>> 285         rcu_read_lock();
>> 286         get_fs_root_rcu(current->fs, &root);
>> 287         if (unlikely(d_unlinked(path->dentry)))
>> 288                 prepend(&b, " (deleted)", 11);
>> 289         else
>> 290                 prepend_char(&b, 0);
>> 291         prepend_path(path, &root, &b);
>> 292         rcu_read_unlock();
>> 293
>> 294         return extract_string(&b);
>> 295 }
>>
>>
>>>
>>> The earlier fix (2530766492ec, "nfsd: fix UAF when access
>>> ex_uuid or ex_stats") moved the kfree of ex_uuid and ex_stats
>>> into svc_export_release() so those are RCU-safe now.
>>> path_put() and auth_domain_put() couldn't go in there because
>>> both may sleep, and call_rcu callbacks run in softirq context.
>>> This series uses queue_rcu_work() instead: it defers past the
>>> grace period AND runs the callback in process context, so the
>>> sleeping puts move into the deferred path and the window
>>> closes.
>>
>> Yeah, I can get this! Thanks again for your detail explanation!
> 
> Also, could the scenario described in this commit be triggered again?
> 
> commit 69d803c40edeaf94089fbc8751c9b746cdc35044
> Author: Yang Erkun <yangerkun@huawei.com>
> Date:   Mon Dec 16 22:21:52 2024 +0800
> 
>      nfsd: Revert "nfsd: release svc_expkey/svc_export with rcu_work"
> 
>      This reverts commit f8c989a0c89a75d30f899a7cabdc14d72522bb8d.
> 
>      Before this commit, svc_export_put or expkey_put will call path_put 
> with
>      sync mode. After this commit, path_put will be called with async mode.
>      And this can lead the unexpected results show as follow.
> 
>      mkfs.xfs -f /dev/sda
>      echo "/ *(rw,no_root_squash,fsid=0)" > /etc/exports
>      echo "/mnt *(rw,no_root_squash,fsid=1)" >> /etc/exports
>      exportfs -ra
>      service nfs-server start
>      mount -t nfs -o vers=4.0 127.0.0.1:/mnt /mnt1
>      mount /dev/sda /mnt/sda
>      touch /mnt1/sda/file
>      exportfs -r
>      umount /mnt/sda # failed unexcepted
> 
>      The touch will finally call nfsd_cross_mnt, add refcount to mount, and
>      then add cache_head. Before this commit, exportfs -r will call
>      cache_flush to cleanup all cache_head, and path_put in
>      svc_export_put/expkey_put will be finished with sync mode. So, the
>      latter umount will always success. However, after this commit, 
> path_put
>      will be called with async mode, the latter umount may failed, and if
>      we add some delay, umount will success too. Personally I think this 
> bug
>      and should be fixed. We first revert before bugfix patch, and then fix
>      the original bug with a different way.
> 
>      Fixes: f8c989a0c89a ("nfsd: release svc_expkey/svc_export with 
> rcu_work")
>      Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>      Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> 
> 

After reviewing these two commits:

e7fcf179b82d NFSD: Hold net reference for the lifetime of 
/proc/fs/nfs/exports fd
48db892356d6 NFSD: Defer sub-object cleanup in export put callbacks

I believe that the issue described in commit e7fcf179b82d might be the
root cause of the null pointer dereferences mentioned in [1]. This is
because we do not call get_net when opening /proc/fs/nfs/exports. As a
result, when the network namespace exits, nfsd_net_exit is triggered.
If, at the same time, the contents of /proc/fs/nfs/exports are being
read, a use-after-free (UAF) can occur on the struct cache_detail. I
think all three bugs referenced in [1] stem from this issue. Therefore,
commit e7fcf179b82d has already addressed the problem. To prevent the
issue described in commit 69d803c40ede, should we consider reverting
commit 48db892356d6 first? Please let me know if I have misunderstood
any aspect of this problem.

>>
>> Thanks,
>> Erkun.
>>
>>>
>>>
>>
> 
> 
>