[Question] nfsd: possible reordering between nf->nf_file assignment and NFSD_FILE_PENDING clearing?

[Li Lingfeng]

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@xxxxxxxxx/

Any suggestion will be appreciated.

Thanks,
Lingfeng.