linux-kernel - Re: [PATCH] nfsd: handle get_client_locked() failure in nfsd4_setclientid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEBF3_Z_J6b4xJwM6k_SX4EjEGvOAmRTH1_KLd+1fk_027LWVQ@mail.gmail.com>
Date: Thu, 5 Jun 2025 10:50:25 +0800
From: lei lu <llfamsec@...il.com>
To: Jeff Layton <jlayton@...nel.org>
Cc: Chuck Lever <chuck.lever@...cle.com>, NeilBrown <neil@...wn.name>, 
	Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>, 
	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfsd: handle get_client_locked() failure in nfsd4_setclientid_confirm()

On Thu, Jun 5, 2025 at 12:01 AM Jeff Layton <jlayton@...nel.org> wrote:
>
> Lei Lu recently reported that nfsd4_setclientid_confirm() did not check
> the return value from get_client_locked(). a SETCLIENTID_CONFIRM could
> race with a confirmed client expiring and fail to get a reference. That
> could later lead to a UAF.
>
> Fix this by getting a reference early in the case where there is an
> extant confirmed client. If that fails then treat it as if there were no
> confirmed client found at all.
>
> In the case where the unconfirmed client is expiring, just fail and
> return the result from get_client_locked().
>
> Reported-by: lei lu <llfamsec@...il.com>
> Closes: https://lore.kernel.org/linux-nfs/CAEBF3_b=UvqzNKdnfD_52L05Mqrqui9vZ2eFamgAbV0WG+FNWQ@mail.gmail.com/
> Fixes: d20c11d86d8f ("nfsd: Protect session creation and client confirm using client_lock")
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
> ---
> I ran this vs. pynfs and it seemed to do OK. lei lu, can you test this
> patch vs. your reproducer and tell us whether it fixes it?

Patch works for me, the issue is fixed.

Thanks,
LL

> ---
>  fs/nfsd/nfs4state.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d5694987f86fadab985e55cce6261ad680e83b69..d61a7910dde3b8536b8715c2eebd1f1faec95f8f 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4697,10 +4697,16 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>         }
>         status = nfs_ok;
>         if (conf) {
> -               old = unconf;
> -               unhash_client_locked(old);
> -               nfsd4_change_callback(conf, &unconf->cl_cb_conn);
> -       } else {
> +               if (get_client_locked(conf) == nfs_ok) {
> +                       old = unconf;
> +                       unhash_client_locked(old);
> +                       nfsd4_change_callback(conf, &unconf->cl_cb_conn);
> +               } else {
> +                       conf = NULL;
> +               }
> +       }
> +
> +       if (!conf) {
>                 old = find_confirmed_client_by_name(&unconf->cl_name, nn);
>                 if (old) {
>                         status = nfserr_clid_inuse;
> @@ -4717,10 +4723,14 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>                         }
>                         trace_nfsd_clid_replaced(&old->cl_clientid);
>                 }
> +               status = get_client_locked(unconf);
> +               if (status != nfs_ok) {
> +                       old = NULL;
> +                       goto out;
> +               }
>                 move_to_confirmed(unconf);
>                 conf = unconf;
>         }
> -       get_client_locked(conf);
>         spin_unlock(&nn->client_lock);
>         if (conf == unconf)
>                 fsnotify_dentry(conf->cl_nfsd_info_dentry, FS_MODIFY);
>
> ---
> base-commit: 5abc7438f1e9d62e91ad775cc83c9594c48d2282
> change-id: 20250604-testing-8d988ff48076
>
> Best regards,
> --
> Jeff Layton <jlayton@...nel.org>
>