linux-kernel - [Question] nfsd: possible reordering between nf->nf_file assignment and NFSD_FILE

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>] [day] [month] [year] [list]

Message-ID: <34bd5595-8f3f-4c52-a1d5-d782fc99efb9@huawei.com>
Date: Thu, 18 Sep 2025 21:57:59 +0800
From: Li Lingfeng <lilingfeng3@...wei.com>
To: Dai Ngo <Dai.Ngo@...cle.com>, Chuck Lever <chuck.lever@...cle.com>, Jeff
 Layton <jlayton@...nel.org>, NeilBrown <neil@...wn.name>, Olga Kornievskaia
	<okorniev@...hat.com>, Tom Talpey <tom@...pey.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>, yangerkun
	<yangerkun@...wei.com>, "zhangyi (F)" <yi.zhang@...wei.com>, Hou Tao
	<houtao1@...wei.com>, "chengzhihao1@...wei.com" <chengzhihao1@...wei.com>,
	"yukuai (C)" <yukuai3@...wei.com>, <leo.lilong@...wei.com>,
	<wutengda2@...wei.com>
Subject: [Question] nfsd: possible reordering between nf->nf_file assignment
 and NFSD_FILE_PENDING clearing?

Recently, we encountered a null pointer dereference on a relatively old
5.10 kernel that does not include commit c4c649ab413ba ("NFSD: Convert
filecache to rhltable"), which exhibited the same behavior as described
in [1]. I was wondering if it might be caused by the reordering between
the assignment of nf->nf_file and the clearing of NFSD_FILE_PENDING.

Just to mention, I don't believe the analysis in [1] is entirely accurate,
since hlist_add_head_rcu includes a write barrier.

We haven't encountered this issue on newer kernel versions, but the
assignment of nf->nf_file and the clearing of NFSD_FILE_PENDING appear
consistent across different versions.

Our expected outcome should be like this:
                 T1                                    T2
nfsd_read
  nfsd_file_acquire_gc
   nfsd_file_do_acquire
    nfsd_file_lookup_locked
     // get nfsd_file from nfsd_file_rhltable
                                         nfsd_read
                                          nfsd_file_acquire_gc
                                           nfsd_file_do_acquire
                                            nfsd_file_alloc
                                             nf->nf_flags // set 
NFSD_FILE_PENDING
                                            rhltable_insert // insert to 
nfsd_file_rhltable
                                            nf->nf_file = file // set 
nf_file
    wait_on_bit
    // wait NFSD_FILE_PENDING to be cleared
                                            clear_and_wake_up_bit // 
clear NFSD_FILE_PENDING
    // get file after being awakened
  file = nf->nf_file

Or like this:
                 T1                                    T2
nfsd_read
  nfsd_file_acquire_gc
   nfsd_file_do_acquire
    nfsd_file_lookup_locked
     // get nfsd_file from nfsd_file_rhltable
                                         nfsd_read
                                          nfsd_file_acquire_gc
                                           nfsd_file_do_acquire
                                            nfsd_file_alloc
                                             nf->nf_flags // set 
NFSD_FILE_PENDING
                                            rhltable_insert // insert to 
nfsd_file_rhltable
                                            nf->nf_file = file // set 
nf_file
                                            clear_and_wake_up_bit // 
clear NFSD_FILE_PENDING
    // get file directly
  file = nf->nf_file

But is it possible that due to reordering, it ends up like this:
                 T1                                    T2
nfsd_read
  nfsd_file_acquire_gc
   nfsd_file_do_acquire
    nfsd_file_lookup_locked
     // get nfsd_file from nfsd_file_rhltable
                                         nfsd_read
                                          nfsd_file_acquire_gc
                                           nfsd_file_do_acquire
                                            nfsd_file_alloc
                                             nf->nf_flags // set 
NFSD_FILE_PENDING
                                            rhltable_insert // insert to 
nfsd_file_rhltable
                                            clear_and_wake_up_bit // 
clear NFSD_FILE_PENDING
    // get file directly
  file = nf->nf_file
                                            nf->nf_file = file // set 
nf_file
  // Null dereference due to uninitialized file pointer.

[1]: 
https://lore.kernel.org/all/20230818065507.1280625-1-haydenw.kernel@gmail.com/

Any suggestion will be appreciated.

Thanks,
Lingfeng.