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

[Li Lingfeng <lilingfeng3@huawei.com>]

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.