• Youzhong Yang's avatar
    nfsd: add list_head nf_gc to struct nfsd_file · 8e6e2ffa
    Youzhong Yang authored
    nfsd_file_put() in one thread can race with another thread doing
    garbage collection (running nfsd_file_gc() -> list_lru_walk() ->
    nfsd_file_lru_cb()):
    
      * In nfsd_file_put(), nf->nf_ref is 1, so it tries to do nfsd_file_lru_add().
      * nfsd_file_lru_add() returns true (with NFSD_FILE_REFERENCED bit set)
      * garbage collector kicks in, nfsd_file_lru_cb() clears REFERENCED bit and
        returns LRU_ROTATE.
      * garbage collector kicks in again, nfsd_file_lru_cb() now decrements nf->nf_ref
        to 0, runs nfsd_file_unhash(), removes it from the LRU and adds to the dispose
        list [list_lru_isolate_move(lru, &nf->nf_lru, head)]
      * nfsd_file_put() detects NFSD_FILE_HASHED bit is cleared, so it tries to remove
        the 'nf' from the LRU [if (!nfsd_file_lru_remove(nf))]. The 'nf' has been added
        to the 'dispose' list by nfsd_file_lru_cb(), so nfsd_file_lru_remove(nf) simply
        treats it as part of the LRU and removes it, which leads to its removal from
        the 'dispose' list.
      * At this moment, 'nf' is unhashed with its nf_ref being 0, and not on the LRU.
        nfsd_file_put() continues its execution [if (refcount_dec_and_test(&nf->nf_ref))],
        as nf->nf_ref is already 0, nf->nf_ref is set to REFCOUNT_SATURATED, and the 'nf'
        gets no chance of being freed.
    
    nfsd_file_put() can also race with nfsd_file_cond_queue():
      * In nfsd_file_put(), nf->nf_ref is 1, so it tries to do nfsd_file_lru_add().
      * nfsd_file_lru_add() sets REFERENCED bit and returns true.
      * Some userland application runs 'exportfs -f' or something like that, which triggers
        __nfsd_file_cache_purge() -> nfsd_file_cond_queue().
      * In nfsd_file_cond_queue(), it runs [if (!nfsd_file_unhash(nf))], unhash is done
        successfully.
      * nfsd_file_cond_queue() runs [if (!nfsd_file_get(nf))], now nf->nf_ref goes to 2.
      * nfsd_file_cond_queue() runs [if (nfsd_file_lru_remove(nf))], it succeeds.
      * nfsd_file_cond_queue() runs [if (refcount_sub_and_test(decrement, &nf->nf_ref))]
        (with "decrement" being 2), so the nf->nf_ref goes to 0, the 'nf' is added to the
        dispose list [list_add(&nf->nf_lru, dispose)]
      * nfsd_file_put() detects NFSD_FILE_HASHED bit is cleared, so it tries to remove
        the 'nf' from the LRU [if (!nfsd_file_lru_remove(nf))], although the 'nf' is not
        in the LRU, but it is linked in the 'dispose' list, nfsd_file_lru_remove() simply
        treats it as part of the LRU and removes it. This leads to its removal from
        the 'dispose' list!
      * Now nf->ref is 0, unhashed. nfsd_file_put() continues its execution and set
        nf->nf_ref to REFCOUNT_SATURATED.
    
    As shown in the above analysis, using nf_lru for both the LRU list and dispose list
    can cause the leaks. This patch adds a new list_head nf_gc in struct nfsd_file, and uses
    it for the dispose list. This does not fix the nfsd_file leaking issue completely.
    Signed-off-by: default avatarYouzhong Yang <youzhong@gmail.com>
    Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
    Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
    8e6e2ffa
filecache.h 2.65 KB