linux-kernel - Re: CVE-2024-50106: nfsd: fix race between laundromat and free

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: <2024121713-reproduce-rippling-73cc@gregkh>
Date: Tue, 17 Dec 2024 16:59:58 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Li Lingfeng <lilingfeng3@...wei.com>
Cc: cve@...nel.org, linux-kernel@...r.kernel.org,
	linux-cve-announce@...r.kernel.org,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
	Olga Kornievskaia <okorniev@...hat.com>,
	Chuck Lever <chuck.lever@...cle.com>,
	Jeff Layton <jlayton@...nel.org>, NeilBrown <neilb@...e.de>,
	yangerkun <yangerkun@...wei.com>,
	"zhangyi (F)" <yi.zhang@...wei.com>, Hou Tao <houtao1@...wei.com>,
	"yukuai (C)" <yukuai3@...wei.com>,
	"chengzhihao1@...wei.com" <chengzhihao1@...wei.com>,
	ZhangXiaoxu <zhangxiaoxu5@...wei.com>
Subject: Re: CVE-2024-50106: nfsd: fix race between laundromat and
 free_stateid

On Tue, Dec 17, 2024 at 11:30:41PM +0800, Li Lingfeng wrote:
> Hi,
> after analysis, we think that this issue is not introduced by commit
> 2d4a532d385f ("nfsd: ensure that clp->cl_revoked list is protected by
> clp->cl_lock") but by commit 83e733161fde ("nfsd: avoid race after
> unhash_delegation_locked()").
> Therefore, kernel versions earlier than 6.9 do not involve this issue.
> 
> // normal case 1 -- free deleg by delegreturn
> 1) OP_DELEGRETURN
> nfsd4_delegreturn
>  nfsd4_lookup_stateid
>  destroy_delegation
>   destroy_unhashed_deleg
>    nfs4_unlock_deleg_lease
>     vfs_setlease // unlock
>  nfs4_put_stid // put last refcount
>   idr_remove // remove from cl_stateids
>   s->sc_free // free deleg
> 
> 2) OP_FREE_STATEID
> nfsd4_free_stateid
>  find_stateid_locked // can not find the deleg in cl_stateids
> 
> 
> // normal case 2 -- free deleg by laundromat
> nfs4_laundromat
>  state_expired
>  unhash_delegation_locked // set NFS4_REVOKED_DELEG_STID
>  list_add // add the deleg to reaplist
>  list_first_entry // get the deleg from reaplist
>  revoke_delegation
>   destroy_unhashed_deleg
>    nfs4_unlock_deleg_lease
>    nfs4_put_stid
> 
> 
> // abnormal case
> nfs4_laundromat
>  state_expired
>  unhash_delegation_locked
>   // set NFS4_REVOKED_DELEG_STID
>  list_add
>   // add the deleg to reaplist
>                                 1) OP_DELEGRETURN
>                                 nfsd4_delegreturn
>                                  nfsd4_lookup_stateid
> nfsd4_stid_check_stateid_generation
>                                   nfsd4_verify_open_stid
>                                    // check NFS4_REVOKED_DELEG_STID
>                                    // and return nfserr_deleg_revoked
>                                  // skip destroy_delegation
> 
>                                 2) OP_FREE_STATEID
>                                 nfsd4_free_stateid
>                                  // check NFS4_REVOKED_DELEG_STID
>                                  list_del_init
>                                   // remove deleg from reaplist
>                                  nfs4_put_stid
>                                   // free deleg
>  list_first_entry
>   // cant not get the deleg from reaplist
> 
> 
> Before commit 83e733161fde ("nfsd: avoid race after
> unhash_delegation_locked()"), nfs4_laundromat --> unhash_delegation_locked
> would not set NFS4_REVOKED_DELEG_STID for the deleg.
> So the description "it marks the delegation stid revoked" in the CVE fix
> patch does not hold true. And the OP_FREE_STATEID operation will not
> release the deleg.

Thanks for the research.  If the maintainers involved agree, we'll be
glad to add a .vulnerable file to our git repo and regenerate the json
entry to reflect this starting point for the issue.

thanks,

greg k-h