• Jeff Layton's avatar
    nfsd: close potential race between delegation break and laundromat · dff1399f
    Jeff Layton authored
    Bruce says:
    
        There's also a preexisting expire_client/laundromat vs break race:
    
        - expire_client/laundromat adds a delegation to its local
          reaplist using the same dl_recall_lru field that a delegation
          uses to track its position on the recall lru and drops the
          state lock.
    
        - a concurrent break_lease adds the delegation to the lru.
    
        - expire/client/laundromat then walks it reaplist and sees the
          lru head as just another delegation on the list....
    
    Fix this race by checking the dl_time under the state_lock. If we find
    that it's not 0, then we know that it has already been queued to the LRU
    list and that we shouldn't queue it again.
    
    In the case of destroy_client, we must also ensure that we don't hit
    similar races by ensuring that we don't move any delegations to the
    reaplist with a dl_time of 0. Just bump the dl_time by one before we
    drop the state_lock. We're destroying the delegations anyway, so a 1s
    difference there won't matter.
    
    The fault injection code also requires a bit of surgery here:
    
    First, in the case of nfsd_forget_client_delegations, we must prevent
    the same sort of race vs. the delegation break callback. For that, we
    just increment the dl_time to ensure that a delegation callback can't
    race in while we're working on it.
    
    We can't do that for nfsd_recall_client_delegations, as we need to have
    it actually queue the delegation, and that won't happen if we increment
    the dl_time. The state lock is held over that function, so we don't need
    to worry about these sorts of races there.
    
    There is one other potential bug nfsd_recall_client_delegations though.
    Entries on the victims list are not dequeued before calling
    nfsd_break_one_deleg. That's a potential list corruptor, so ensure that
    we do that there.
    Reported-by: default avatar"J. Bruce Fields" <bfields@fieldses.org>
    Signed-off-by: default avatarJeff Layton <jlayton@primarydata.com>
    Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
    dff1399f
nfs4state.c 141 KB